fix(solid-query): untrack the one-shot client and options reads on mount - #11456
fix(solid-query): untrack the one-shot client and options reads on mount#11456mnkprs wants to merge 3 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNo actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughSolid Query now wraps one-shot initialization reads in ChangesSolid Query untracked initialization
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Solid Query hook initialization no longer creates unintended reactive dependencies or Solid 2 diagnostics, while normal observer-driven updates remain intact. The affected hook behaviors are covered by focused tests and no merge-blocking risk remains. 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 |
Every hook seeds an observer or a signal from `client()` and its options accessor while the hook itself is running. Later changes reach the observers through `setOptions`/`setQueries` and the surrounding computations, so those initial reads are one-shot by design — but they were still made inside whatever tracking scope called the hook. On Solid 2 that makes each hook call log a `[STRICT_READ_UNTRACKED]` diagnostic on mount, drowning out the diagnostics that are about application code. It is also observable on Solid 1: a hook called from inside a computation makes that computation depend on the client and the options, so changing either re-runs it and rebuilds the observer. Wrap the reads in `untrack`, which is exactly the case it exists for. Fixes TanStack#11358
d850a22 to
4cd21ee
Compare
1 participant
Fixes #11358.
Every solid-query hook seeds an observer or a signal from
client()and its options accessor while the hook itself is running. Later changes reach those observers throughsetOptions/setQueriesand the computations around them, so those initial reads are one-shot by design — but they were still made inside whatever tracking scope called the hook. On Solid 2 that makes each hook call log a[STRICT_READ_UNTRACKED]diagnostic on mount, so a screen with a handful of queries and mutations fills the console with library noise and drowns out the diagnostics that are about application code.This wraps those reads in
untrack, which is the case it exists for. Affected:useBaseQuery—initialOptions, theObserverconstruction, and the firstgetOptimisticResultuseMutation— theMutationObserverconstructionuseQueries— theQueriesObserver, the initial store contents, the seedingbatch, and the initialgetProxies()useIsFetching/useIsMutating/useMutationState— the signal seedsWhy this is not a behaviour change for components
Solid's
createComponentalready wraps component bodies inuntrack, so for the way these hooks are actually called the reads were untracked before this PR too. I checked this by running the same scenarios against the merge base and against this branch — client swaps and option changes behave identically in a component either way.The behaviour that does change is a hook called directly inside a
createComputed/createMemo. There, the tracked reads used to tear the hook down and rebuild it wholesale whenever the client or the options changed. That accidental rebuild is what the new tests pin down as gone.Tests
src/__tests__/untrackedReads.test.tsxcalls each hook inside a computation that reads nothing itself, then invalidates the client and the options the hook read, and asserts the computation does not re-run. This makes the leak observable on Solid 1 as well, rather than only as a Solid 2 diagnostic. All six fail onmain(3 runs instead of 1) and pass here.Note for reviewers
While verifying the above I found three pre-existing bugs that this PR deliberately does not touch, since they are out of scope for the ticket and all reproduce on
maintoday in ordinary component usage:useBaseQuery'son(client, …)handler callscreateClientSubscriber()— which reads theobserversignal — beforesetObserver(newObserver), so it resubscribes to the outgoing observer and later updates to the new client's cache never arrive.useMutationanduseQuerieshave no client-change handling at all; swapping the client leaves them on the old one.useMutationStatenever recomputes when its filters change — the solid twin of useMutationState does not update when filters change (react-query) #11272.I have fixes and regression tests ready for (1) and (3) and will send them as separate PRs if you'd like them.
✅ Checklist
pnpm run test:pr, or these tests do not apply to this pull request.🚀 Release Impact