fix(query-core): resolve suspense when query data is set programmatically#11036
fix(query-core): resolve suspense when query data is set programmatically#11036AmariahAK wants to merge 4 commits into
Conversation
…ally When useSuspenseQuery is used and setQueryData populates the cache while a fetch is in-flight, the suspense boundary stays stuck on the original fetch promise. This prevents streamedQuery and manual cache updates from releasing suspense. Modify fetchOptimistic in queryObserver to use Promise.race between the fetch promise and a cache subscriber that listens for 'updated' events. When data appears in the cache via setQueryData or a streamed chunk, the race resolves immediately without aborting the fetch. This allows the suspense boundary to release and show the available data while the fetch continues in the background. Closes TanStack#10924 Co-authored-by: atlarix-agent <agent@atlarix.dev>
Co-authored-by: atlarix-agent <agent@atlarix.dev>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesSuspense Resolution Fix
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant QueryObserver
participant QueryFn
participant QueryCache
QueryObserver->>QueryFn: query.fetch()
QueryObserver->>QueryCache: subscribe(updated)
alt cache data arrives first
QueryCache-->>QueryObserver: updated event with data
QueryObserver->>QueryCache: unsubscribe()
QueryObserver-->>QueryObserver: createResult()
else fetch completes first
QueryFn-->>QueryObserver: fetch resolved
QueryObserver->>QueryCache: unsubscribe()
QueryObserver-->>QueryObserver: createResult()
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f9c9f6e-0fdd-44e8-b976-3179c1f4098f
📒 Files selected for processing (3)
.changeset/resolve-suspense-setquerydata.mdpackages/query-core/src/queryObserver.tspackages/react-query/src/__tests__/useSuspenseQuery.test.tsx
…h rejection unsubscribe() was only called in the .then() success path. If query.fetch() rejected before the cache subscriber resolved the race, the subscription leaked — staying registered on QueryCache indefinitely and checking a stale queryHash on every 'updated' event. Use .finally() to ensure unsubscribe() runs on both success and rejection, preserving the original promise type through the chain. Co-authored-by: atlarix-agent <agent@atlarix.dev>
|
Thanks for this PR — the 1. Floating promise when fetch wins the race When the fetch promise resolves first, A cleaner approach would be to make the subscriber promise self-resolving on cleanup: let resolveEarly: ((result: QueryObserverResult<TData, TError>) => void) | undefined
const cachePromise = new Promise<QueryObserverResult<TData, TError>>((resolve) => {
resolveEarly = resolve
unsubscribe = this.#client.getQueryCache().subscribe((event) => {
if (
event.type === 'updated' &&
event.query.queryHash === query.queryHash &&
query.state.data !== undefined
) {
unsubscribe()
resolve(this.createResult(query, defaultedOptions))
}
})
})
return Promise.race([
query.fetch().then(() => {
return this.createResult(query, defaultedOptions)
}).finally(() => {
unsubscribe()
// Resolve the floating promise to avoid it pending forever
if (resolveEarly) {
// Subscriber never fired — resolve with current result so the promise settles
// This value is ignored by Promise.race since the fetch branch already won
resolveEarly(this.createResult(query, defaultedOptions))
}
}),
cachePromise,
])This ensures both promises always settle, regardless of which wins. 2. No cleanup if the component unmounts mid-fetch If the consuming component unmounts while Consider adding an AbortSignal-aware cleanup, or at minimum documenting that 3. Multiple observers on the same query hash The cache subscriber listens to 4. Test coverage consideration The three tests are solid. One edge case that might be worth adding: what happens when Overall, this is the right approach — much less invasive than #10994 and uses the existing notification system well. The main actionable item is settling the floating promise in point 1. |
3 participants
PR Description
@tanstack/query-core:test:lib → 546 passed, 0 failures
@tanstack/react-query:test:lib → 567 passed, 0 failures