[DataGrid] Fix row tree reset when a data source grid becomes visible - #23288
Open
JCQuintas wants to merge 2 commits into
Open
[DataGrid] Fix row tree reset when a data source grid becomes visible#23288JCQuintas wants to merge 2 commits into
JCQuintas wants to merge 2 commits into
Conversation
The props-sync effect in `useGridRows` guards against re-applying rows with an identity comparison against `caches.rows.rowsBeforePartialUpdates`. With a data source the rows live in the state rather than in props, and `gridDataRowsSelector` builds a new array on every recompute, so that comparison never holds and the effect rebuilds the whole row tree every time it re-runs. React `<Activity>` re-runs effects with unchanged dependencies when it becomes visible again. The rebuild then dropped the lazy-loaded children and the skeleton rows, and reset `childrenExpanded` on every group node. Rows never come from props under a data source, so there is never a new set to apply. Only a new `getRowId` requires a rebuild, to re-key the rows already in the state. Closes mui#23262
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
JCQuintas
marked this pull request as ready for review
August 4, 2026 12:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #23262.
A data source grid with
treeData+lazyLoadinginside a React<Activity>boundary lost its lazy-loaded children and skeleton rows, and hadchildrenExpandedreset on every group node, whenever the boundary wenthidden→visible.<Activity>re-runs effects when it becomes visible again, even though no dependency changed. The props-sync effect at the bottom ofuseGridRowswas not resilient to that, because its only guard against re-applying rows is an identity comparison:rowsBeforePartialUpdatesholds the raw array passed to the lastsetRows(), whilegridDataRowsSelectorbuilds a new array on every recompute. Under a data source those are never the same reference, soareNewRowsAlreadyInStateis effectively alwaysfalseand the effect falls through to a fullthrottledRowsChangerebuild. That normally goes unnoticed, because the effect only runs when a dependency actually changed — but<Activity>re-fires it with identical dependencies, and the rebuild drops the skeleton rows (they are not data rows) and resets the expansion state.With a data source the rows live in the state rather than in props, so there is never a new set of rows to apply here. The fix makes that explicit instead of asking a question that cannot be answered by reference identity. Only a new
getRowIdstill requires a rebuild, to re-key the rows already in the state — tracked with a ref, the same way the effect already tracksrowCounta few lines above.This is a second, distinct site from #22603. That PR stopped the
<Activity>re-show from re-fetching inuseGridDataSourceBase; with it merged, the row tree was still destroyed by this effect. Verified in both directions on top ofmasterat 77536d2:childrenchildrenExpandedTests
dataSourceTreeData.DataGridPro.test.tsx— the loaded children, the skeleton rows and the expansion state survive an<Activity>hide/show. Fails onmaster.dataSourceTreeData.DataGridPro.test.tsx— a realrowCountprop update is still applied, covering the fall-through path that the early return must not swallow.dataSource.DataGridPro.test.tsx— agetRowIdchange still re-keys the rows of a data source grid. This path had no coverage, and an earlier version of this fix silently broke it while the whole suite stayed green.Changelog
Fixed the Data Grid dropping lazy-loaded rows, skeleton rows and the group expansion state when a data source grid re-appears inside a React
<Activity>boundary.