fix(solid-query): don't re-trigger Suspense on background refetches - #11230
Open
brenelz wants to merge 1 commit into
Open
fix(solid-query): don't re-trigger Suspense on background refetches#11230brenelz wants to merge 1 commit into
brenelz wants to merge 1 commit into
Conversation
Every query observer update was funneled through the underlying resource's refetch(), whose fetcher always returned a Promise. Even a Promise that resolves within the same tick leaves the resource in a pending/refreshing state for at least a microtask, so every enclosing <Suspense> boundary flipped to its fallback and back. The flip is never painted, but it detaches and re-inserts the boundary's DOM, restarting CSS animations and resetting focus/scroll/iframe state on every background refetch and on every mount with cached data. The fetcher now returns the observer result synchronously (a non-thenable) when data is available and the query is not in an initial loading state — Solid's resource completes such loads without ever entering a pending state, so Suspense is only triggered by genuine initial loads. Two exceptions preserve existing semantics: an in-flight initial load (pending resolver) still completes through the Promise path so Suspense/Transition bookkeeping resolves correctly, and no-data results (e.g. disabled queries) keep their previous behavior. Also updates the re-mount test, which codified the old suspend-on-remount-with-cached-data behavior; cached data now renders immediately while the mount refetch runs in the background, matching React's useSuspenseQuery semantics. Fixes TanStack#9955 Related: TanStack#9883, TanStack/router#8000 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe Solid Query resource fetcher now resolves available client data synchronously when no fetch is pending. Server hydration and error handling remain promise-based. Suspense tests cover cached mounts, remounts, and invalidation refetches. ChangesSolid Suspense resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change keeps cached data rendered during background refetches so Suspense boundaries no longer reset UI state; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
View your CI Pipeline Execution ↗ for commit cb08a8f
☁️ Nx Cloud last updated this comment at |
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.
Summary
Background refetches (and mounts with cached data) no longer flip enclosing
<Suspense>boundaries. This fixes CSS animations restarting, and focus/scroll/iframe state resetting, on every query update.Fixes #9955
Related: #9883, and the downstream report TanStack/router#8000 (solid-router wraps every route match in
<Suspense>, so route components can't escape a boundary — which made this present as a router bug).Root cause
useBaseQuery's client subscriber calls the resource'srefetch()on every observer notification, and the fetcher always returned a Promise — even when it resolved within the same tick (!isLoading→resolve()called synchronously in the executor). A native Promise's.thenfires a microtask later, so the resource spent at least one microtask in a pending/refreshing state withprset.Any
query.dataread that ever registered with aSuspenseContextwhileprwas set leaves behind a tracking computed that re-runs on everytrigger()and increments the boundary again (the.latestfast path can't prevent this: on mount with cached data,resolvedis still false during that first microtask, so the initial read registers anyway — and structural sharing keepsdatareferentially stable, so the reading memo never re-runs and never disposes the computed).The boundary flips to fallback and back within one microtask. That's never painted, but Suspense detaches and re-inserts the children's DOM — and reconnecting an element restarts its CSS animations. Verified in the router#8000 repro with a MutationObserver: the exact same DOM node is removed and re-inserted 1 ms apart precisely when a background refetch settles.
The existing mitigations in #10053 / #10592 gate on
isFetching === false, so the stale-mount case (refetch already in flight at mount) still flips.Fix
When the observer result has data and is not in an initial loading state, the fetcher returns the result synchronously (a non-thenable). Solid's
load()completes non-Promise results immediately without ever setting a pending promise, so Suspense is only triggered by genuine initial loads. Two exceptions preserve existing semantics:resolverset — an in-flight initial load, where the boundary is already suspended and possibly inside a transition), completion goes through the Promise path so Suspense/Transition bookkeeping resolves correctly.Behavior change
should refetch when re-mountingpreviously asserted that the Suspense fallback shows when re-mounting a component whose query has cached data. That expectation codified the reported bug; the test now expects cached data to render immediately while the mount refetch runs in the background (matching ReactuseSuspenseQuerysemantics). The refetch itself still happens and is still asserted.Validation
🤖 Generated with Claude Code
Summary by CodeRabbit