fix: Prevent infinite render loops when useSuspenseQueries has duplicate queryKeys #9886
Conversation
🦋 Changeset detectedLatest commit: a3ae6a0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughChanges shift observer tracking to arrays per queryHash, reuse observers to avoid duplicate-key infinite renders, add a regression test for duplicate query keys in useSuspenseQueries, and add a changeset bumping two TanStack packages to patch releases. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant USQ as useSuspenseQueries
participant QO as queriesObserver
participant Cache as Query Cache
rect rgb(241,250,255)
Note over QO: group prevObservers by queryHash → observer[]
end
App->>USQ: call useSuspenseQueries(queries with duplicate keys)
USQ->>QO: request observer assignments
loop per query
QO->>QO: check observer array for queryHash
alt array non-empty
QO->>QO: reuse observer (shift from array)
else array empty
QO->>QO: create new QueryObserver
end
QO->>Cache: attach/update observer for query
end
Cache-->>USQ: observers registered
USQ-->>App: render (suspended/resolved) with stable observers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-08-19T03:18:18.303ZApplied to files:
🧬 Code graph analysis (1)packages/query-core/src/queriesObserver.ts (1)
⏰ 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). (2)
🔇 Additional comments (2)
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 |
olivierbeaulieu
changed the title
![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 (1)
packages/query-core/src/queriesObserver.ts (1)
254-263: Observer consumption logic correctly prevents state thrashing.The use of
shift()at line 256 ensures each query gets its own observer instance, even when multiple queries share the same queryHash. This prevents the infinite loop that occurred when a single observer was reconfigured multiple times per render with conflicting options.The fallback to
new QueryObserverat line 257 handles cases where no previous observer exists for reuse. The logic is sound and maintains observer stability across renders.Optional: While
shift()is O(n), it's acceptable here since duplicate keys are rare and arrays are small. If performance becomes a concern in the future, consider tracking an index instead of mutating the array. However, this is a minor optimization and not necessary now.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/nine-shoes-tap.md(1 hunks)packages/query-core/src/queriesObserver.ts(1 hunks)packages/react-query/src/__tests__/useSuspenseQueries.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
.changeset/nine-shoes-tap.md
🧬 Code graph analysis (2)
packages/react-query/src/__tests__/useSuspenseQueries.test.tsx (1)
packages/react-query/src/__tests__/utils.tsx (1)
renderWithClient(9-23)
packages/query-core/src/queriesObserver.ts (1)
packages/query-core/src/queryObserver.ts (2)
QueryObserver(41-747)options(379-385)
🔇 Additional comments (3)
packages/react-query/src/__tests__/useSuspenseQueries.test.tsx (1)
186-257: Excellent test coverage for the duplicate key scenario.The test properly validates that duplicate query keys with different selectors no longer cause infinite render loops. The structure is clear:
- Two queries share the same base key but have different
selectfunctionsrenderCounttracks total renders to detect loops- Assertions verify single suspend/resolution and bounded renders
Minor suggestion: The
renderCount < 10assertion at line 256 is somewhat arbitrary. Consider adding a comment explaining the expected render count (e.g., "Expected: 1 suspend + 1 resolution ≈ 2-3 renders") to make the test's intent clearer.packages/query-core/src/queriesObserver.ts (1)
238-250: LGTM! Observer grouping by queryHash is correctly implemented.The change from
Map<string, QueryObserver>toMap<string, Array<QueryObserver>>is the key fix. Each queryHash now maps to an array of observers, enabling proper tracking when duplicate query keys exist. The grouping logic correctly handles both the case where an entry already exists (push to array) and when it doesn't (create new array)..changeset/nine-shoes-tap.md (1)
1-6: LGTM! Changeset properly documents the patch release.The changeset correctly identifies both affected packages (
@tanstack/react-queryand@tanstack/query-core) and uses patch version bumps, which is appropriate for a bug fix. The description clearly states what issue is being resolved.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/query-core/src/queriesObserver.ts (1)
253-262: Observer matching logic is correct with minor performance consideration.The use of
shift()to consume observers from the array ensures each query gets a distinct observer instance, which is exactly what's needed to prevent the state thrashing. The FIFO order provides reasonable stability when query order doesn't change.Performance note:
shift()has O(n) complexity for arrays, but given that duplicate query keys are unsupported (and a warning is logged), the arrays should be small in practice, making this acceptable.If performance becomes a concern with many duplicates in the future, consider using an index-based approach:
const prevObserversMap = new Map<string, { observers: Array<QueryObserver>; index: number }>() // During matching: const entry = prevObserversMap.get(defaultedOptions.queryHash) const match = entry && entry.index < entry.observers.length ? entry.observers[entry.index++] : undefinedHowever, this optimization is not necessary given the current use case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/query-core/src/queriesObserver.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/query-core/src/queriesObserver.ts (1)
packages/query-core/src/queryObserver.ts (2)
QueryObserver(41-747)options(379-385)
🔇 Additional comments (1)
packages/query-core/src/queriesObserver.ts (1)
238-249: Excellent fix for the duplicate key issue!The change from mapping
queryHashto a single observer to mapping it to an array of observers correctly addresses the root cause of the infinite render loop. This ensures each query with a duplicate key gets its own stable observer instance instead of sharing one.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/query-core/src/queriesObserver.ts (1)
255-264: Observer matching logic is sound.The use of
shift()(line 257) ensures each query gets a distinct observer from the pool, with new observers created only when needed. The optional chaining and nullish coalescing handle all edge cases correctly.Consider adding a brief comment explaining the shift pattern for future maintainers, as it's a key part of preventing the duplicate key issue:
queries.forEach((options) => { const defaultedOptions = this.#client.defaultQueryOptions(options) + // Shift ensures each query gets its own observer, preventing shared state conflicts const match = prevObserversMap.get(defaultedOptions.queryHash)?.shift() const observer = match ?? new QueryObserver(this.#client, defaultedOptions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/query-core/src/queriesObserver.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/query-core/src/queriesObserver.ts (1)
packages/query-core/src/queryObserver.ts (2)
QueryObserver(41-747)options(379-385)
🔇 Additional comments (1)
packages/query-core/src/queriesObserver.ts (1)
238-251: Excellent fix for the infinite render loop issue.The array-based grouping is the right approach to handle duplicate query keys. Each query will now get its own stable observer instance instead of sharing one, which prevents the state thrashing that caused infinite re-renders.
Also, I notice the past review concern about the non-null assertion on
queryHashhas been addressed. The code now readsobserver.options.queryHashwithout the!assertion (line 241) and includes a defensive checkif (!key) return(line 242) to handle undefined cases gracefully.
|
View your CI Pipeline Execution ↗ for commit a3ae6a0
☁️ Nx Cloud last updated this comment at |
|
nice work! does that mean we can actually support the same key multiple times within useQueries? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9886 +/- ##
===========================================
+ Coverage 45.79% 59.84% +14.05%
===========================================
Files 200 129 -71
Lines 8421 5640 -2781
Branches 1928 1536 -392
===========================================
- Hits 3856 3375 -481
+ Misses 4116 1959 -2157
+ Partials 449 306 -143 🚀 New features to boost your workflow:
|
2 participants
🎯 Changes
Fixes an infinite re-render loop in useSuspenseQueries (and useQueries) when duplicate query keys are provided introduced in #8771.
While duplicate query keys are technically unsupported and log a warning, the current behavior causes the application to crash due to an infinite render loop, which is a severe degradation.
The issue
QueriesObserver used a Map keyed by queryHash to track observers, causing duplicate queries to overwrite each other in the map. This forced all duplicate queries to share a single observer instance. During updates, this shared observer was repeatedly reconfigured with conflicting options (like different select functions) within the same render cycle, causing its state to thrash and triggering infinite re-renders.
The fix
Updated QueriesObserver to track a list of observers per hash (Map<string, QueryObserver[]>). This ensures each duplicate query is matched to its own distinct, stable observer instance, preventing the state conflict.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.