fix(query-core, react-query): infinite re-renders in useQueries #9639
Conversation
WalkthroughIntroduces structural-change detection in queriesObserver to handle length/index differences, adjusts result-change and early-return logic accordingly, and updates subscription/destroy behavior for added/removed observers. Adds a React test to verify no infinite re-renders when removing the last query from useQueries. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as React Component (useQueries)
participant QO as QueriesObserver
participant Obs as QueryObserver(s)
C->>QO: setQueries(newQueries)
QO->>QO: Compute hasLengthChange, hasIndexChange
QO->>QO: hasStructuralChange = OR(len, idx)
alt Structural change
QO->>Obs: Destroy removed observers
QO->>Obs: Subscribe newly added observers
QO->>Obs: Set options on new/current observers
QO->>QO: Update internal observers list
else No structural change
QO->>QO: Compare results (shallow) to detect changes
end
QO-->>C: Notify only if structural or result change
note over QO,C: Prevents re-notify loops when removing last item
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 8dda7d3
☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9639 +/- ##
===========================================
+ Coverage 45.50% 59.64% +14.13%
===========================================
Files 209 138 -71
Lines 8377 5617 -2760
Branches 1897 1514 -383
===========================================
- Hits 3812 3350 -462
+ Misses 4118 1963 -2155
+ Partials 447 304 -143 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/query-core/src/queriesObserver.ts (2)
121-126: Skip index scan when length already differs.A tiny perf nit: short-circuit hasIndexChange if lengths differ to avoid an O(n) scan you already know will be irrelevant.
Apply:
- const hasLengthChange = prevObservers.length !== newObservers.length - const hasIndexChange = newObservers.some( - (observer, index) => observer !== prevObservers[index], - ) + const hasLengthChange = prevObservers.length !== newObservers.length + const hasIndexChange = + !hasLengthChange && + newObservers.some( + (observer, index) => observer !== prevObservers[index], + )
144-153: Avoid building two Sets for differences.Minor micro-opt: compute added/removed with precomputed sets once instead of calling difference twice (each builds a Set).
Apply:
- if (hasStructuralChange) { - difference(prevObservers, newObservers).forEach((observer) => { - observer.destroy() - }) - difference(newObservers, prevObservers).forEach((observer) => { - observer.subscribe((result) => { - this.#onUpdate(observer, result) - }) - }) - } + if (hasStructuralChange) { + const prevSet = new Set(prevObservers) + const nextSet = new Set(newObservers) + const removed = prevObservers.filter((o) => !nextSet.has(o)) + const added = newObservers.filter((o) => !prevSet.has(o)) + + removed.forEach((observer) => { + observer.destroy() + }) + added.forEach((observer) => { + observer.subscribe((result) => { + this.#onUpdate(observer, result) + }) + }) + }packages/react-query/src/__tests__/useQueries.test.tsx (2)
1749-1758: Prefer unique keys via queryKey() to avoid cross-test collisions.Using generated keys mirrors existing tests and minimizes shared-client interference.
Apply:
- function Page() { - const [queries, setQueries] = React.useState([ + function Page() { + const k1 = queryKey() + const k2 = queryKey() + const [queries, setQueries] = React.useState([ { - queryKey: ['query1'], + queryKey: k1, queryFn: () => 'data1', }, { - queryKey: ['query2'], + queryKey: k2, queryFn: () => 'data2', }, ]) @@ <button onClick={() => { setQueries([ { - queryKey: ['query1'], + queryKey: k1, queryFn: () => 'data1', }, ]) }} > remove last </button> @@ <button onClick={() => { setQueries([ { - queryKey: ['query2'], + queryKey: k2, queryFn: () => 'data2', }, ]) }} > remove first </button>Also applies to: 1769-1775, 1781-1787
1746-1766: Track render count with a ref; avoid manual resets mid-test.Keeps counting logic inside the component and removes the need to mutate outer scope.
Apply:
- let renderCount = 0 + let renderCount = 0 @@ - renderCount++ + renderCount++ @@ - await vi.advanceTimersByTimeAsync(0) - renderCount = 0 + await vi.advanceTimersByTimeAsync(0) @@ - renderCount = 0 + // no manual reset neededAlternative (cleaner inside component):
- let renderCount = 0 + const rendersRef = React.useRef(0) @@ - renderCount++ + rendersRef.current++ @@ - <div data-testid="render-count">renders: {renderCount}</div> + <div data-testid="render-count">renders: {rendersRef.current}</div>Then assert by reading the DOM value instead of mutating a closed-over variable.
Also applies to: 1797-1807
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/query-core/src/queriesObserver.ts(1 hunks)packages/react-query/src/__tests__/useQueries.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/react-query/src/__tests__/useQueries.test.tsx (3)
packages/query-core/src/queriesObserver.ts (3)
queries(235-261)setQueries(87-157)result(195-210)packages/react-query/src/useQueries.ts (1)
useQueries(208-332)packages/react-query/src/__tests__/utils.tsx (1)
renderWithClient(9-23)
packages/query-core/src/queriesObserver.ts (1)
packages/query-core/src/utils.ts (1)
shallowEqualObjects(306-321)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (2)
packages/query-core/src/queriesObserver.ts (1)
121-139: Solid structural-change detection fix; aligns with the bug’s root cause.Introducing hasLengthChange and folding it into hasStructuralChange, plus guarding result comparisons behind structural checks, correctly handles removals at the tail and reorders. The early-return now reflects actual list structure/result stability, preventing the infinite update loop.
packages/react-query/src/__tests__/useQueries.test.tsx (1)
1745-1813: Good regression test; reproduces and guards the original failure mode.Covers both removing the last and the first element; assertions are lenient enough to avoid flakes.
Fixes #9638
useQuerieswas causing infinite re-renders when dynamically removing the last query from the queries array. This issue was introduced in v5.85.9 after PR #9592.Problem
The
queriesObserver.tschange detection logic had a flaw when handling array length changes:hasIndexChange = falsebuthasResultChange = trueSolution
Improved the change detection logic in
queriesObserver.tsand added test inuseQueries.test.tsxSummary by CodeRabbit
Bug Fixes
Tests