fix(solid-query): don't re-create and refetch a removed query - #11360
Open
theRizwan wants to merge 1 commit into
Open
fix(solid-query): don't re-create and refetch a removed query#11360theRizwan wants to merge 1 commit into
theRizwan wants to merge 1 commit into
Conversation
The read layer bumps its per-hook version signal on every cache event for its hash, 'removed' included, and the recompute that followed called `queryCache.build()`, putting the entry the caller had just deleted straight back. The resurrection was not passive: the rebuilt entry also re-pointed the still-live observer, whose mount-fetch policy then refetched and repopulated the key, so `removeQueries()` and `clear()` could not be made to stick while any hook observed the key. `query()` now reuses the entry it last read when the cache no longer holds that hash, and builds only when the hash is genuinely new. A removal leaves the cache empty and fires no fetch, while the mounted reader holds its last value until options change or a real entry returns through `setQueryData`, a refetch or a later mount. The resulting cache event sequence, entry count, data and fetch count are identical to react-query's for the same scenario, and to solid-query's at 6.0.0-rc.0. Fixes TanStack#11350
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the 鈿欙笍 Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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
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.
Fixes #11350.
Targets
solid-query-v6-pre, since the affected read layer arrived with the 2.0-native rewrite (#11308) and the regression is between6.0.0-rc.0and6.0.0-rc.1. One housekeeping note that follows from that base: GitHub only honours a closing keyword when the PR targets the default branch, so theFixesabove registers as a plain cross-reference and #11350 will need closing by hand on merge.The bug
Calling
queryClient.removeQueries()while auseQueryobserver is mounted removes the entry and then immediately brings it back, and the recreated entry starts a fresh fetch of the key that was just removed. A caller that removes entries to make them unreadable, for example an identity boundary that must guarantee the previous user's data cannot be read, cannot make the removal last.Cause
useBaseQueryLayerhas one version signal per hook, bumped byonCacheEventfor any cache event on its hash.'removed'is such an event, and the recompute it triggers ran throughquery():queryCache.build()creates the entry when the cache does not hold it, so the hook's own removal notification made it rebuild what the caller had just deleted. The resurrection is not passive: the'added'entry also re-points the still-live observer, and its mount-fetch policy then refetches and repopulates the key. Hence the reported event sequence,removed, added, observerRemoved, observerAdded, updated.This contradicts the documented contract for the method, that
removeQueries"removes matching queries from the cache instead of refetching them".Fix
query()keeps the entry it last read and falls back to it when the cache no longer holds that hash, building only when the hash is genuinely new. A removal now leaves the cache empty and fires no fetch, and the mounted reader holds its last value until options change or a real entry returns throughsetQueryData, a refetch or a later mount.cache.get()is consulted first on every read, so a live entry is never shadowed by the held-over one. The fallback is dropped on a client swap insyncClient, as it is only meaningful for the cache it was read from.Because
useInfiniteQueryanduseQueriesboth go through this layer, the one change covers all three hooks.Verification
The reporter's scenario, run against the react adapter on this branch to establish the reference, and against solid-query before and after the change:
removedremoved, added, observerRemoved, observerAdded, ...removedgetQueryDataundefined'v2'undefinedv1v2v1The patched adapter matches react-query exactly, and matches
6.0.0-rc.0as reported.Two regression tests in
useQuery-semantics.test.tsx. Both fail on the unpatched branch, the first on the event sequence and the second on the entry count:leaves a removed query out of the cache while an observer is mounted, which asserts the cache event list is exactly['removed']with no'added'behind it, an empty cache, an unchanged fetch count, and that the reader holds its last value rather than suspending over a key that no longer exists.picks up a real entry written after a removal, which guards the fallback against becoming a stale read: asetQueryDataafter the removal rebuilds the entry, and the reader must track the new instance.Full
@tanstack/solid-querysuite passes, 26 files and 348 tests, along with@tanstack/solid-query-devtoolsand@tanstack/solid-query-persist-client.test:eslintandtsc --buildare clean.