Skip to content

Commit 3c7c664

Browse files
committed
fix: handle undefined sorts/customColumns from lazy+StrictMode defaultProps bug
When IrisGrid is loaded via LazyIrisGrid (the public export), it goes through a forwardRef -> lazy() -> Suspense chain. In React 18 StrictMode, there is a known bug (facebook/react#28505) where defaultProps are not correctly merged into this.props during the second componentDidMount call of the StrictMode double-mount cycle. The root cause is in React's reconciler: during the simulated unmount, componentWillUnmount sets instance.props to fiber.memoizedProps (the unresolved props without defaultProps merged). When React re-mounts the component, componentDidMount sees these unresolved props, so props like sorts and customColumns are undefined instead of their defaultProps values (EMPTY_ARRAY). loadTableState() then sets these undefined values into state, and IrisGridModelUpdater crashes with 'sorts is not iterable' when it tries to spread them: `[...sorts]`. Add defensive ?? EMPTY_ARRAY fallbacks in: - IrisGrid.loadTableState(): when setting sorts/customColumns into state - IrisGridModelUpdater: when spreading sorts and assigning customColumns Add tests that reproduce the exact bug by combining async lazy loading with StrictMode, and unit tests for IrisGridModelUpdater with undefined sorts/customColumns.
1 parent e47ed60 commit 3c7c664

4 files changed

Lines changed: 128 additions & 6 deletions

File tree

packages/iris-grid/src/IrisGrid.test.tsx

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import React, { useRef } from 'react';
2-
import { act, fireEvent, render } from '@testing-library/react';
1+
import React, { lazy, Suspense, useRef } from 'react';
2+
import { act, fireEvent, render, waitFor } from '@testing-library/react';
33
import dh from '@deephaven/jsapi-shim';
44
import { DateUtils, type Settings } from '@deephaven/jsapi-utils';
55
import { TestUtils } from '@deephaven/test-utils';
@@ -435,6 +435,65 @@ describe('handleResizeAllColumns', () => {
435435
});
436436
});
437437

438+
describe('LazyIrisGrid with StrictMode', () => {
439+
it('renders without error when sorts and customColumns are not provided', async () => {
440+
// This reproduces the real-world bug:
441+
// 1. External consumers import IrisGrid from @deephaven/iris-grid, which is LazyIrisGrid
442+
// 2. LazyIrisGrid = forwardRef wrapping lazy(() => import('./IrisGrid'))
443+
// 3. The lazy wrapper means defaultProps are NOT resolved at element creation time
444+
// (both jsx() and createElement() produce props={} for lazy components)
445+
// 4. React's reconciler resolves defaultProps during rendering for lazy components
446+
// 5. In StrictMode, React double-fires effects: componentDidMount → componentWillUnmount → componentDidMount
447+
// 6. On the second componentDidMount, loadTableState() reads this.props.sorts
448+
// which may be undefined if defaultProps resolution isn't applied consistently
449+
// across the StrictMode re-mount cycle
450+
//
451+
// To simulate true async lazy loading (vs Jest's synchronous import()),
452+
// we use a deferred promise that we manually resolve.
453+
454+
let resolveLazy!: (value: { default: typeof IrisGrid }) => void;
455+
const LazyIrisGridTest = React.forwardRef<
456+
IrisGrid,
457+
React.ComponentPropsWithoutRef<typeof IrisGrid>
458+
>((props, ref) => {
459+
const IrisGridLazy = React.useMemo(
460+
() =>
461+
lazy(
462+
() =>
463+
new Promise<{ default: typeof IrisGrid }>(resolve => {
464+
resolveLazy = resolve;
465+
})
466+
),
467+
[]
468+
);
469+
return (
470+
<Suspense fallback={<div data-testid="loading" />}>
471+
<IrisGridLazy ref={ref} {...props} />
472+
</Suspense>
473+
);
474+
});
475+
LazyIrisGridTest.displayName = 'LazyIrisGridTest';
476+
477+
const model = irisGridTestUtils.makeModel();
478+
479+
// Render in StrictMode — just like a real app typically does
480+
const { queryByTestId } = render(
481+
<React.StrictMode>
482+
<LazyIrisGridTest model={model} settings={DEFAULT_SETTINGS} />
483+
</React.StrictMode>
484+
);
485+
486+
// Should initially show loading fallback
487+
expect(queryByTestId('loading')).not.toBeNull();
488+
489+
// Now resolve the lazy import — this triggers the actual IrisGrid mount
490+
// and the StrictMode double-mount cycle
491+
await act(async () => {
492+
resolveLazy(await import('./IrisGrid'));
493+
});
494+
});
495+
});
496+
438497
describe('Advanced Filter', () => {
439498
it.each([
440499
{ columnIndex: -1, expectedVisibility: false },

packages/iris-grid/src/IrisGrid.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2178,9 +2178,9 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
21782178
}
21792179

21802180
this.setState({
2181-
sorts,
2181+
sorts: sorts ?? EMPTY_ARRAY,
21822182
reverse: reverse || reverseType === TableUtils.REVERSE_TYPE.POST_SORT,
2183-
customColumns,
2183+
customColumns: customColumns ?? EMPTY_ARRAY,
21842184
isReady: true,
21852185
searchFilter,
21862186
});
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import React from 'react';
2+
import { render } from '@testing-library/react';
3+
import dh from '@deephaven/jsapi-shim';
4+
import { Formatter } from '@deephaven/jsapi-utils';
5+
import { EMPTY_ARRAY } from '@deephaven/utils';
6+
import IrisGridModelUpdater from './IrisGridModelUpdater';
7+
import IrisGridTestUtils from './IrisGridTestUtils';
8+
9+
const irisGridTestUtils = new IrisGridTestUtils(dh);
10+
11+
function makeModel() {
12+
return irisGridTestUtils.makeModel();
13+
}
14+
15+
function renderUpdater(propsOverride = {}) {
16+
const model = makeModel();
17+
const defaultProps = {
18+
model,
19+
top: 0,
20+
bottom: 100,
21+
left: 0,
22+
right: 10,
23+
filter: EMPTY_ARRAY as readonly dh.FilterCondition[],
24+
sorts: EMPTY_ARRAY as readonly never[],
25+
customColumns: EMPTY_ARRAY as readonly string[],
26+
movedColumns: EMPTY_ARRAY as readonly never[],
27+
hiddenColumns: EMPTY_ARRAY as readonly number[],
28+
alwaysFetchColumns: EMPTY_ARRAY as readonly string[],
29+
formatColumns: EMPTY_ARRAY as readonly dh.CustomColumn[],
30+
columnHeaderGroups: EMPTY_ARRAY as readonly never[],
31+
formatter: new Formatter(dh),
32+
columnAlignmentMap: new Map<string, CanvasTextAlign>(),
33+
};
34+
return render(<IrisGridModelUpdater {...defaultProps} {...propsOverride} />);
35+
}
36+
37+
describe('IrisGridModelUpdater', () => {
38+
it('renders without error when sorts and customColumns are provided', () => {
39+
expect(() => renderUpdater()).not.toThrow();
40+
});
41+
42+
it('does not throw when sorts is undefined', () => {
43+
// This simulates what happens when LazyIrisGrid passes through props
44+
// without defaultProps being applied — sorts ends up undefined in state,
45+
// and IrisGridModelUpdater receives undefined for sorts
46+
expect(() => renderUpdater({ sorts: undefined as unknown })).not.toThrow();
47+
});
48+
49+
it('does not throw when customColumns is undefined', () => {
50+
expect(() =>
51+
renderUpdater({ customColumns: undefined as unknown })
52+
).not.toThrow();
53+
});
54+
55+
it('does not throw when both sorts and customColumns are undefined', () => {
56+
expect(() =>
57+
renderUpdater({
58+
sorts: undefined as unknown,
59+
customColumns: undefined as unknown,
60+
})
61+
).not.toThrow();
62+
});
63+
});

packages/iris-grid/src/IrisGridModelUpdater.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ function IrisGridModelUpdater({
114114
);
115115
useOnChange(
116116
function updateSorts() {
117-
const sortsForModel = [...sorts];
117+
const sortsForModel = [...(sorts ?? EMPTY_ARRAY)];
118118
if (reverse) {
119119
sortsForModel.push(model.dh.Table.reverse());
120120
}
@@ -137,7 +137,7 @@ function IrisGridModelUpdater({
137137
useOnChange(
138138
function updateCustomColumns() {
139139
if (model.isCustomColumnsAvailable) {
140-
model.customColumns = customColumns;
140+
model.customColumns = customColumns ?? EMPTY_ARRAY;
141141
}
142142
},
143143
[model, customColumns]

0 commit comments

Comments
 (0)