[DataGrid] Avoid `<GridRoot />` double-render pass on mount in SPA mode #15648

lauri865 · 2024-11-28T11:17:38Z

I have followed (at least) the PR section of the contributing guide.

Currently, GridRoot double-renders in all cases on mount to prevent SSR. In SPA-mode this is irrelevant, and can be avoided with the help of useSyncExternalStore, since there's no server snapshot in SPA-mode, the gird will be mounted directly. As a result, the rootRef becomes immediately available on first mount in SPAs, as one would expect.

Adds a new dependency use-sync-external-store to make it backwards compatible with React 17. Charts and treeview have the same dependency.

Changelog

Breaking changes

The filteredRowsLookup object of the filter state does not contain true values anymore. If the row is filtered out, the value is false. Otherwise, the row id is not present in the object.
This change only impacts you if you relied on filteredRowsLookup to get ids of filtered rows. In this case,use gridDataRowIdsSelector selector to get row ids and check filteredRowsLookup for false values:
```
 const filteredRowsLookup = gridFilteredRowsLookupSelector(apiRef);
-const filteredRowIds = Object.keys(filteredRowsLookup).filter((rowId) => filteredRowsLookup[rowId] === true);
+const rowIds = gridDataRowIdsSelector(apiRef);
+const filteredRowIds = rowIds.filter((rowId) => filteredRowsLookup[rowId] !== false);
```

The visibleRowsLookup state does not contain true values anymore. If the row is not visible, the value is false. Otherwise, the row id is not present in the object:

 const visibleRowsLookup = gridVisibleRowsLookupSelector(apiRef);
-const isRowVisible = visibleRowsLookup[rowId] === true;
+const isRowVisible = visibleRowsLookup[rowId] !== false;

… pass in SPAs

mui-bot · 2024-11-28T11:22:13Z

Deploy preview: https://deploy-preview-15648--material-ui-x.netlify.app/

Updated pages:

docs/data/migration/migration-data-grid-v7/migration-data-grid-v7.md

Generated by 🚫 dangerJS against fa21f18

lauri865 · 2024-11-28T17:21:34Z

Side-benefit is also probably more robust tests if the Datagrid is mounted on the first pass.

packages/x-data-grid/src/hooks/features/pagination/useGridPagination.ts

romgrk

Code LGTM, we can merge once the tests are passing.

lauri865 · 2024-12-04T00:54:48Z

I have no idea how to fix the last failing test. Either it's a symptom of column headers updated more times now or just a bad test. But I lack understanding why it was previously expected that the warning message will be output exactly twice.

Edit: discovered some areas to improve around mounting / state initialization

lauri865 · 2024-12-04T12:54:24Z

Also fixes an annoying flicker coming out of loading state. It's probably the root cause of a layout shift I once reported with autoHeight grid coming out of loading state.

Before:
https://github.com/user-attachments/assets/1a68b2de-f2fd-4ef4-a93d-215d044e35b7

After:
https://github.com/user-attachments/assets/cfe55012-092a-429d-a79d-75e243124d10

…on resize event

lauri865 · 2024-12-04T14:45:03Z

Tests should be passing now. 4 was the right amount of warnings for this approach, 1 extra pass (+1 warning for header/row each = 2+2 = 4) that doesn't even flush to the dom, so I think we're good there.

Curiously, after this optimisation:

mui-x/packages/x-data-grid/src/hooks/features/columns/useGridColumns.tsx

Lines 400 to 412 in 5ca918f

    
           const hasFlexColumns = gridVisibleColumnDefinitionsSelector(apiRef).some( 
        
             (col) => col.flex && col.flex > 0, 
        
           ); 
        
           if (!hasFlexColumns) { 
        
             return; 
        
           } 
        
           setGridColumnsState( 
        
             hydrateColumnsWidth( 
        
               gridColumnsStateSelector(apiRef.current.state), 
        
               apiRef.current.getRootDimensions(), 
        
             ), 
        
           );

This test started failing:

mui-x/packages/x-data-grid-premium/src/tests/aggregation.DataGridPremium.test.tsx

Lines 145 to 150 in 1a079e1

    
           it('should correctly restore the column when changing from aggregated to non-aggregated', () => { 
        
             const { setProps } = render(<Test aggregationModel={{ id: 'max' }} />); 
        
             expect(getColumnHeaderCell(0, 0).textContent).to.equal('idmax'); 
        
             setProps({ aggregationModel: {} }); 
        
             expect(getColumnHeaderCell(0, 0).textContent).to.equal('id'); 
        
           });

The only reason I can think of is that aggregations didn't explicitly trigger column updates, but somehow were piggybacking on viewportInnerSizeChange event for taking care of it. Adding an explicit updateColumns call fixed it. Is there any other explanation to it?

If that's the case, then I suppose there was a bug whereby columns didn't clear when there was no totals row visible, as viewPortInnerSize would only change if there was a pinned row.

packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx

packages/x-data-grid-pro/src/tests/events.DataGridPro.test.tsx

packages/x-data-grid-premium/src/hooks/features/aggregation/useGridAggregation.ts

packages/x-data-grid/src/tests/layout.DataGrid.test.tsx

cherniavskii · 2025-02-03T23:58:23Z

packages/x-data-grid/src/hooks/features/filter/gridFilterState.ts

@@ -67,4 +73,4 @@ export type GridFilteringMethodValue = Omit<GridFilterState, 'filterModel'>;
 * A row is visible if it is passing the filters AND if its parents are expanded.
 * If a row is not registered in this lookup, it is visible.
 */
-export type GridVisibleRowsLookupState = Record<GridRowId, boolean>;
+export type GridVisibleRowsLookupState = Record<GridRowId, false>;


Same change for visibleRowsLookup as previously with filteredRowsLookup.

packages/x-data-grid/src/hooks/utils/useGridSelector.ts

This reverts commit 676da6b.

MBilalShafi · 2025-02-04T09:25:13Z

packages/x-data-grid/src/hooks/features/filter/gridFilterState.ts

@@ -20,7 +26,7 @@ export interface GridFilterState {
   * If a row is not registered in this lookup, it is filtered.
   * This is the equivalent of the `visibleRowsLookup` if all the groups were expanded.
   */
-  filteredRowsLookup: Record<GridRowId, boolean>;
+  filteredRowsLookup: Record<GridRowId, false>;


Since we don't store true values now, does it make sense to simplify the type further? It could be slightly better performance wise, and more aligned DX wise, as the filteredRowsLookup no longer stores the "filtered" rows, rather it stores "excluded" ones.

For consistency, a similar concept could be applied on the visible rows (invisibleRows: Set<GridRowId>), column visibility model (hiddenColumns: Set<GridColDef['field']>), etc.

Suggested change

filteredRowsLookup: Record<GridRowId, false>;

excludedRows: Set<GridRowId>;

Changing Record to Set/Map won't necessarily be faster – see #9120 (comment)

See #10068 (comment) and #10068 (comment). I think the change is good and has potential for improvement, but I didn't have the bandwidth to investigate it more.

packages/x-data-grid/src/hooks/features/filter/gridFilterState.ts

packages/x-data-grid-pro/src/components/GridPinnedRows.tsx

packages/x-data-grid/src/components/GridScrollArea.tsx

packages/x-data-grid/src/internals/selectors/dimensionSelectors.ts

romgrk · 2025-02-04T17:01:51Z

packages/x-data-grid/src/internals/selectors/dimensionSelectors.ts

+export const gridDimensionsColumnsTotalWidthSelector = (state: GridStateCommunity) =>
+  state.dimensions.columnsTotalWidth;


You said there is a name conflict with another selector called gridColumnsTotalWidthSelector. Are the two selectors returning the same value, calculated differently? If so, would it be appropriate to remove one of them?

Otherwise, could we rename the other one and keep the short name for this one?

Are the two selectors returning the same value, calculated differently? If so, would it be appropriate to remove one of them?

No, the original gridColumnsTotalWidthSelector calculates the value, and that value change triggers the dimensions state update and gets stored in that state.
AFAIU @lauri865 used gridDimensionsColumnsTotalWidthSelector instead because it guarantees that the dimensions state is up to date.

I've moved the original selector to useGridDimensions, it's only used internally now.
I've also renamed gridDimensionsColumnsTotalWidthSelector to gridColumnsTotalWidthSelector and used createSelector to keep the signature of the original selector.

@lauri865 Does this make sense to you?

Exactly. I believe your change makes sense, as nothing else probably depends on the calculated size other than useGridDimensions, it makes sense to localise it.

packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx

romgrk

LGTM beyond the small comments above.

github-actions · 2025-02-05T20:54:37Z

Cherry-pick PRs will be created targeting branches: v7.x

…de (mui#15648) Co-authored-by: Andrew Cherniavskyi <[email protected]>

arminmeh · 2025-02-06T14:06:43Z

packages/x-data-grid/src/hooks/features/dimensions/gridDimensionsSelectors.ts

+  (dimensions) => dimensions.columnsTotalWidth,
+);
+
+export const gridRowHeightSelector = (state: GridStateCommunity) => state.dimensions.rowHeight;


@lauri865
any particular reason why all these selectors are not using gridDimensionsSelector as an input?

What would it be solving to wrap them in another function call? Just went with my gut feel tbh - simple selectors, simpler to write plainly. Localized to a single file, enforced by typescript.

I am asking because in this PR I have to replace all params with apiRef, so each one of these will have apiRef.current.state.dimensions.X repeated. Having just dimensions will be much easier to read.

They should probably be transformed to something like const xxxSelector = createRootSelector(state => state.dimensions.xxx) to keep them readable.

lauri865 added 3 commits November 28, 2024 11:54

add useIsSSR hook to with useSyncExternalStore to avoid double-render…

c0944e1

… pass in SPAs

remove unused import

ad08bcd

add missing dependency

2ff2567

lauri865 added 2 commits November 28, 2024 12:32

add missing types

128234a

fix failing test

8da261b

lauri865 force-pushed the avoid-double-render-pass-in-spas branch from dd1a1e1 to 8da261b Compare November 28, 2024 13:53

Merge branch 'master' into avoid-double-render-pass-in-spas

e680805

flaviendelangle added the component: data grid This is the name of the generic UI component, not the React module! label Nov 28, 2024

flaviendelangle changed the title ~~[Data Grid] Avoid GridRoot double-render pass on mount in SPA mode~~ [Data Grid] Avoid <GridRoot /> double-render pass on mount in SPA mode Nov 28, 2024

lint

eb491c6

romgrk reviewed Dec 2, 2024

View reviewed changes

packages/x-data-grid/src/hooks/features/pagination/useGridPagination.ts Show resolved Hide resolved

romgrk approved these changes Dec 2, 2024

View reviewed changes

fix some tests

8d2a78d

fix flicker on mount

c8580d9

lauri865 force-pushed the avoid-double-render-pass-in-spas branch from 12fb3da to c8580d9 Compare December 4, 2024 12:39

lauri865 added 4 commits December 4, 2024 14:35

don't hydrate widths unless there's a flex column

c459ad6

fix tests

01cbb00

lint

d4707a1

fix: aggregation should trigger column update directly, and not lean …

5ca918f

…on resize event

romgrk reviewed Dec 4, 2024

View reviewed changes

packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx Outdated Show resolved Hide resolved

romgrk reviewed Dec 4, 2024

View reviewed changes

packages/x-data-grid-pro/src/tests/events.DataGridPro.test.tsx Outdated Show resolved Hide resolved

romgrk reviewed Dec 4, 2024

View reviewed changes

packages/x-data-grid-premium/src/hooks/features/aggregation/useGridAggregation.ts Outdated Show resolved Hide resolved

romgrk reviewed Dec 4, 2024

View reviewed changes

packages/x-data-grid/src/tests/layout.DataGrid.test.tsx Outdated Show resolved Hide resolved

lauri865 added 2 commits December 4, 2024 18:32

Merge branch 'master' into avoid-double-render-pass-in-spas

9cd2134

Fix for grid aggregation out of order column hydration

4287d0a

cherniavskii added 2 commits February 4, 2025 00:18

useEventCallback

676da6b

Merge branch 'master' into avoid-double-render-pass-in-spas

76fb889

cherniavskii approved these changes Feb 3, 2025

View reviewed changes

cherniavskii added 2 commits February 4, 2025 10:18

Revert "useEventCallback"

f8ca726

This reverts commit 676da6b.

Merge branch 'master' into avoid-double-render-pass-in-spas

e45e07a

MBilalShafi reviewed Feb 4, 2025

View reviewed changes

packages/x-data-grid/src/hooks/features/filter/gridFilterState.ts Outdated Show resolved Hide resolved

cherniavskii requested a review from romgrk February 4, 2025 15:15

romgrk reviewed Feb 4, 2025

View reviewed changes

packages/x-data-grid-pro/src/components/GridPinnedRows.tsx Outdated Show resolved Hide resolved

romgrk reviewed Feb 4, 2025

View reviewed changes

packages/x-data-grid/src/components/GridScrollArea.tsx Outdated Show resolved Hide resolved

romgrk reviewed Feb 4, 2025

View reviewed changes

packages/x-data-grid/src/internals/selectors/dimensionSelectors.ts Outdated Show resolved Hide resolved

romgrk reviewed Feb 4, 2025

View reviewed changes

packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx Outdated Show resolved Hide resolved

romgrk approved these changes Feb 4, 2025

View reviewed changes

cherniavskii added 9 commits February 5, 2025 12:32

Merge branch 'master' into avoid-double-render-pass-in-spas

bab03a8

avoid changing object shape

1c5e1d2

update jsdoc

9708bf1

remove unnecessary useEventCallback

c0f1205

move dimensions selectors to feature folder

6bbf36d

keep only one gridColumnsTotalWidthSelector

6bc48b2

Merge branch 'master' into avoid-double-render-pass-in-spas

86d2dcc

remove export

3771d7a

format markfown diff

fa21f18

cherniavskii merged commit ad55b60 into mui:master Feb 5, 2025
18 checks passed

cherniavskii changed the title ~~[Data Grid] Avoid <GridRoot /> double-render pass on mount in SPA mode~~ [DataGrid] Avoid <GridRoot /> double-render pass on mount in SPA mode Feb 5, 2025

cherniavskii added a commit to cherniavskii/mui-x that referenced this pull request Feb 5, 2025

[DataGrid] Avoid <GridRoot /> double-render pass on mount in SPA mo…

eb74477

…de (mui#15648) Co-authored-by: Andrew Cherniavskyi <[email protected]>

cherniavskii mentioned this pull request Feb 5, 2025

[DataGrid] Avoid <GridRoot /> double-render pass on mount in SPA mode (@lauri865) #16480

Merged

arminmeh reviewed Feb 6, 2025

View reviewed changes

lauri865 mentioned this pull request Feb 7, 2025

[DataGrid] Avoid undefined value for pagination rowCount #16488

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Avoid `<GridRoot />` double-render pass on mount in SPA mode #15648

[DataGrid] Avoid `<GridRoot />` double-render pass on mount in SPA mode #15648

lauri865 commented Nov 28, 2024 •

edited by cherniavskii

Loading

mui-bot commented Nov 28, 2024 •

edited

Loading

lauri865 commented Nov 28, 2024

romgrk left a comment

lauri865 commented Dec 4, 2024 •

edited

Loading

lauri865 commented Dec 4, 2024

lauri865 commented Dec 4, 2024 •

edited

Loading

cherniavskii Feb 3, 2025

MBilalShafi Feb 4, 2025 •

edited

Loading

cherniavskii Feb 4, 2025

romgrk Feb 4, 2025

romgrk Feb 4, 2025 •

edited

Loading

cherniavskii Feb 5, 2025

lauri865 Feb 5, 2025

romgrk left a comment

github-actions bot commented Feb 5, 2025

arminmeh Feb 6, 2025

lauri865 Feb 6, 2025

arminmeh Feb 6, 2025

romgrk Feb 6, 2025

	filteredRowsLookup: Record<GridRowId, false>;
	excludedRows: Set<GridRowId>;

		export const gridDimensionsColumnsTotalWidthSelector = (state: GridStateCommunity) =>
		state.dimensions.columnsTotalWidth;

[DataGrid] Avoid <GridRoot /> double-render pass on mount in SPA mode #15648

[DataGrid] Avoid <GridRoot /> double-render pass on mount in SPA mode #15648

Conversation

lauri865 commented Nov 28, 2024 • edited by cherniavskii Loading

Changelog

Breaking changes

mui-bot commented Nov 28, 2024 • edited Loading

lauri865 commented Nov 28, 2024

romgrk left a comment

Choose a reason for hiding this comment

lauri865 commented Dec 4, 2024 • edited Loading

lauri865 commented Dec 4, 2024

lauri865 commented Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

MBilalShafi Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romgrk Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romgrk left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[DataGrid] Avoid `<GridRoot />` double-render pass on mount in SPA mode #15648

[DataGrid] Avoid `<GridRoot />` double-render pass on mount in SPA mode #15648

lauri865 commented Nov 28, 2024 •

edited by cherniavskii

Loading

mui-bot commented Nov 28, 2024 •

edited

Loading

lauri865 commented Dec 4, 2024 •

edited

Loading

lauri865 commented Dec 4, 2024 •

edited

Loading

MBilalShafi Feb 4, 2025 •

edited

Loading

romgrk Feb 4, 2025 •

edited

Loading