Skip to content

Commit a8f21ee

Browse files
SamTV12345claude
andcommitted
fix(react-table): treat React elements as atomic values in mergeProps
lodash's deep merge recursed into React elements passed through cell props and transform results. With React 19 an element's owner chain is made of plain objects instead of FiberNode instances, so the merge walked the entire rendered tree per cell and server-side rendering of the deprecated Table hung or ran out of memory (this also stalled the docs prerender). Elements are now kept by reference: never spread into plain objects and never merged into. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a52804a commit a8f21ee

2 files changed

Lines changed: 77 additions & 24 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { isValidElement } from 'react';
2+
import { mergeProps } from '../merge-props';
3+
4+
test('merges plain props deeply', () => {
5+
const result = mergeProps({ style: { color: 'red' }, id: 'a' }, { style: { background: 'blue' } });
6+
7+
expect(result).toEqual({ style: { color: 'red', background: 'blue' }, id: 'a' });
8+
});
9+
10+
test('merges class names into a single string', () => {
11+
expect(mergeProps({ className: 'foo' }, { className: 'bar' })).toEqual({ className: 'foo bar' });
12+
});
13+
14+
test('composes children when both sides provide them', () => {
15+
const result = mergeProps({ children: <div id="outer" /> }, { children: <span id="inner" /> });
16+
17+
expect(isValidElement(result.children)).toBe(true);
18+
expect(result.children.type).toBe('div');
19+
expect(result.children.props.children.type).toBe('span');
20+
});
21+
22+
test('keeps a lone React element child by reference instead of spreading it', () => {
23+
const child = <button type="button">toggle</button>;
24+
const result = mergeProps({ className: 'cell' }, { children: child });
25+
26+
// Spreading the element into a plain object would expose React internals
27+
// (e.g. the React 19 owner chain) to lodash's deep merge and hang SSR.
28+
expect(result.children).toBe(child);
29+
});
30+
31+
test('treats React elements under arbitrary keys as atomic values', () => {
32+
const icon = <svg data-id="first" />;
33+
const replacement = <svg data-id="second" />;
34+
const result = mergeProps({ title: icon }, { title: replacement });
35+
36+
expect(result.title).toBe(replacement);
37+
expect(mergeProps({ title: icon }, { other: 1 }).title).toBe(icon);
38+
});

packages/react-table/src/components/Table/base/merge-props.ts

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,46 @@
44
* Forked from reactabular-table version 8.14.0
55
* https://github.com/reactabular/reactabular/tree/v8.14.0/packages/reactabular-table/src
66
*/
7-
import { cloneElement } from 'react';
7+
import { cloneElement, isValidElement } from 'react';
88
import mergeWith from 'lodash/mergeWith';
99
import { css } from '@patternfly/react-styles';
1010

11+
const customizer = (a: any, b: any, key: any) => {
12+
if (key === 'children') {
13+
if (a && b) {
14+
// compose the two
15+
return cloneElement(a, {
16+
children: b
17+
});
18+
}
19+
20+
// React elements must be kept as-is; spreading them into plain objects
21+
// exposes their internals (e.g. the React 19 owner chain) to lodash's
22+
// deep merge, which then walks the entire element graph.
23+
if (isValidElement(a) || isValidElement(b) || a === undefined || b === undefined) {
24+
return b === undefined ? a : b;
25+
}
26+
27+
// Children have to be merged in reverse order for Reactabular
28+
// logic to work.
29+
return { ...b, ...a };
30+
}
31+
32+
if (key === 'className') {
33+
// Process class names through classNames to merge properly
34+
// as a string.
35+
return css(a, b);
36+
}
37+
38+
// React elements are atomic values; letting lodash recurse into their
39+
// internals hangs the merge under React 19.
40+
if (isValidElement(a) || isValidElement(b)) {
41+
return b === undefined ? a : b;
42+
}
43+
44+
return undefined;
45+
};
46+
1147
/**
1248
* @param {any} props - Props
1349
*/
@@ -16,30 +52,9 @@ export function mergeProps(...props: any) {
1652
const restProps = props.slice(1);
1753

1854
if (!restProps.length) {
19-
return mergeWith({}, firstProps);
55+
return mergeWith({}, firstProps, customizer);
2056
}
2157

2258
// Avoid mutating the first prop collection
23-
return mergeWith(mergeWith({}, firstProps), ...restProps, (a: any, b: any, key: any) => {
24-
if (key === 'children') {
25-
if (a && b) {
26-
// compose the two
27-
return cloneElement(a, {
28-
children: b
29-
});
30-
}
31-
32-
// Children have to be merged in reverse order for Reactabular
33-
// logic to work.
34-
return { ...b, ...a };
35-
}
36-
37-
if (key === 'className') {
38-
// Process class names through classNames to merge properly
39-
// as a string.
40-
return css(a, b);
41-
}
42-
43-
return undefined;
44-
});
59+
return mergeWith(mergeWith({}, firstProps, customizer), ...restProps, customizer);
4560
}

0 commit comments

Comments
 (0)