[DataGridPro] Keep incremental root fetches from aborting in-flight children requests - #23327
Draft
JCQuintas wants to merge 3 commits into
Draft
[DataGridPro] Keep incremental root fetches from aborting in-flight children requests#23327JCQuintas wants to merge 3 commits into
JCQuintas wants to merge 3 commits into
Conversation
…hildren requests Nested lazy loading routed its incremental viewport loads and revalidation polls through the public `dataSource.fetchRows()`, which clears the data source state and therefore aborted the child requests still in flight. Their responses were dropped and nothing re-requested them, leaving the groups as skeleton rows. Split the two intents instead of changing what `fetchRows()` means: - `fetchRootRowsIncremental()` fetches the root rows without invalidating, and the nested lazy loader uses it for viewport loads and revalidation. - `fetchRows()` keeps invalidating for every strategy, and the clear now marks the nested rows stale so the following root response rebuilds the tree and re-issues the child requests it aborted. Fixes mui#22715
JCQuintas
added
type: bug
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
github-actions
Bot
added
the
PR: out-of-date
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…etch Conflicts in `useGridDataSourceNestedLazyLoader.ts` against mui#23303, which added the incomplete-filter-item guard to the same handler this branch refactored: - kept both `fetchRootRowsIncremental` and `useGridDataSourceFilterModelChange` - `handleGridFilterModelChange` runs the `hasFilterModelChanged` guard first, then `markRowsStale()`, which is the helper this branch extracted from the three lines master still inlines
github-actions
Bot
removed
the
PR: out-of-date
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
1 participant
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.
Fixes #22715. Alternative approach to #23060.
The bug
With
dataSource+treeData/row grouping +lazyLoading, the nested lazy loader routes its incremental root loads (viewport scrolling,dataSourceRevalidateMspolling) through the publicdataSource.fetchRows(). That method clears the data source state, which aborts the child requests still in flight; their responses are then dropped by theRequestStatus.UNKNOWNguard infetchRowChildren. Nothing re-requests them afterwards, so the affected groups stay as skeleton rows indefinitely.Most visible with
defaultGroupingExpansionDepth={-1}and withdataSourceRevalidateMs, where every poll tick cancelled the nested loads it had just started.The fix
The collision is that one method serves two different intents: "the user asked for a refresh" and "the loader needs the next viewport chunk". They are split instead of changing what
fetchRows()means:fetchRootRowsIncremental()(private) fetches the root rows without invalidating. The nested lazy loader uses it for viewport loads and revalidation polls, the only two root fetches of that strategy that are not invalidating events.fetchRows()keeps clearing the state for every strategy, so its public semantics are unchanged.apiRef.current.dataSource.fetchRows()was not a working hard refresh for nested lazy loading.Sort and filter already marked the rows stale by hand; they now share the same
markRowsStalehelper.Notes
No public API change, so no docs or demo updates are needed.
dataSource.fetchRows()keeps meaning "refresh from scratch" for every strategy, and it now actually rebuilds the tree under nested lazy loading.fetchRows({ keepChildrenExpanded: false })is still ignored by the nested lazy loading strategy (it is only read byhandleGroupedDataUpdate). That is a pre-existing gap, left for a follow-up.Each source change is covered by a test that was verified to fail without it:
fetchRows()are re-fetched (fails if the clear no longer marks the rows stale)