Repository navigation
Fix useList renders all records on first render - #11441
Open
fzaninotto wants to merge 5 commits into
Open
fzaninotto wants to merge 5 commits into
fzaninotto wants to merge 5 commits into
Conversation
useList stored the paginated data in a state initialized with the raw data, and only applied filters, sort and pagination in an effect. The first render exposed the whole unfiltered array, which mounted every child (and fired getMany calls for all references) before rendering the expected page. The paginated data is now derived during render with useMemo. This also ensures the result always matches ListControllerResult: no stale total while pending, and an empty list with a total of 0 when data is missing. Sorting now works on a copy so it never mutates the data prop. Closes #11440 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fzaninotto
force-pushed
the
fix/uselist-first-render
branch
from
November 6, 2026 21:49
699a0ee to
b8df854
Compare
When the filter prop changed, useList synced filterValues in an effect, so children were rendered once with records matching the previous filter. Adjusting the state during render lets React restart the render before committing it, as recommended by the React docs for resetting state when a prop changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Setting state during render is counter-intuitive, so point to the React docs that recommend this pattern for adjusting state when a prop changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
useSortState and usePaginationState synced their initial value prop in an effect guarded by an isFirstRender ref. When the sort or perPage prop changed, children were rendered once with the previous sort or page size (e.g. 50 rows mounted with their reference getMany calls when perPage went from 50 to 5) before the effect updated the state. They now adjust the state during render, so React restarts the render before committing it. The existing semantics are preserved: changing perPage resets the page to 1, falls back to 25 when perPage is removed, and the page prop is still not synced. The specs are renamed to .tsx to render a child component in the tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er change useReferenceManyFieldController still synced its filter prop in an effect, so children rendered once and a fetch went out with the previous filter. Changing the filter prop also kept the current page, which could end up past the new total and show an empty list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ThieryMichel
requested changes
Nov 10, 2026
| // restart the render before committing it, so that children never see | ||
| // the previous perPage. | ||
| // @see https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes | ||
| if (initialPagination.perPage !== prevPerPage) { |
Contributor
There was a problem hiding this comment.
If perPage become NaN, then we will get an infinite render loop, Because NaN !== NaN is always true.
We need to guard agains it.
Suggested change
| if (initialPagination.perPage !== prevPerPage) { | |
| if (a !== b && !(Number.isNaN(a) && Number.isNaN(b))) { |
This branch has not been deployed
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.
Problem
useListstored the paginated data in a state initialized with the rawdata, and only applied filtering, sorting and pagination in auseEffect. As a consequence, the first render exposed the whole unfiltered, unsorted and unpaginated array. Every child of theListContextmounted once per record (and firedgetManycalls for the references of all records) before a second render displayed the expected page.Closes #11440
Solution
useMemoinstead of storing it in a state updated by an effect. The first render now shows the right page, and the double render is gone.ListControllerResult:dataandtotalareundefined(the previous code could keep a staletotal)datais missing outside of loading, return an empty list withtotal: 0(it used to returntotal: undefined, which made<Pagination>display a bogus "1-10 of 10" below an empty list)filterprop changes, update the filter values during render instead of in an effect, as recommended by the React docs. This applies to bothuseListanduseReferenceManyFieldController, which had the same effect: children rendered once with records matching the previous filter, and<ReferenceManyField>fetched with the previous filter before fetching with the new one.filterprop changes, go back to the first page, assetFiltersalready does. Otherwise the current page could end up past the new total and display an empty list.useSortStateandusePaginationState(used byuseListanduseReferenceManyFieldController), which synced theirsortandperPageprops in an effect guarded by anisFirstRenderref. WhenperPagewent from 50 to 5, 50 rows were mounted once, with their referencegetManycalls, before the page size was updated. The existing semantics are preserved: changingperPageresets the page to 1, falls back to 25 whenperPageis removed, and thepageprop is still not synced.useListnever mutates thedataprop (this happened whenfilterwasnull), which matters now that the computation runs during render.Note on render-phase updates
When a prop changes, React still runs the body of the component calling the hook once with the previous state, then discards that pass and re-runs it with the updated state. This discarded pass is never committed: children are not rendered and effects don't run, so descendants never see the previous filter, sort or page size. Only code in the host component that has side effects during render could observe it. Before this PR, the stale render was fully committed, with all its children and effects.
How to test
useList.spec.tsxcover the first render with filter, sort and pagination, the non-mutation ofdata, missing data, the pending state, the absence of a render with stale filters when thefilterprop changes, and the return to the first page in that case.useReferenceManyFieldController.spec.tsxchecks that afilterprop change triggers a single fetch, for the first page with the new filter.useSortState.spec.tsxandusePaginationState.spec.tsx(renamed from.tsto render a child component) check that children never render with the previous sort or page size after a prop change, and that the existing semantics are preserved.Additional Checks
masterfor a bugfix or a documentation fix, ornextfor a feature🤖 Generated with Claude Code