fix(react-query): recompute useMutationState when the filters change - #11329
fix(react-query): recompute useMutationState when the filters change#11329MahathirMohammadShuvo wants to merge 1 commit into
Conversation
After the first render, `result.current` was only rewritten from inside the `mutationCache.subscribe()` callback, and `getSnapshot` returns that ref, so a render carrying new `filters` or a new `select` recomputed nothing. The hook kept serving the previous options' result until something unrelated touched the cache, and if nothing did, it stayed wrong indefinitely. `useIsMutating` is built on this hook and went stale with it. It is now recomputed on render as well. `useIsFetching` does the same work inside `getSnapshot`, which it can afford because it returns a number; this hook returns an array, and a fresh one per call never satisfies the snapshot consistency check. So the array stays in a ref, and the recompute is guarded on the options the same way `QueryObserver` guards `select`, which keeps an unrelated render from re-running it. Fixes TanStack#11272
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml 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. 📝 WalkthroughWalkthrough
ChangesMutation state updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change makes mutation state and mutation counts update when filters or selectors change, correcting stale results without changing the public API or deployment behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description provides a detailed explanation of the bug, the fix, test coverage, test results, and deliberate adapter scope. It omits the template headings and unchecked checklist, but the required technical information is present. Full details: Linked Issues checkExplanation The implementation satisfies issue Full details: Out of Scope Changes checkExplanation The changeset, production fix, and tests are directly related to issue Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 2 files. (1 skipped: 1 unsupported.)
✨ 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 |
Fixes #11272
useMutationStateignored changes to its options. After the first renderresult.currentwas only rewritten from inside themutationCache.subscribe()callback, and
getSnapshotreturns that ref, so a render carrying newfiltersora new
selectrecomputed nothing. The hook kept serving the previous options'result until something unrelated touched the cache — and if nothing did, it stayed
wrong indefinitely.
useIsMutatingis built on this hook and went stale in the same way, which is thewider half of the bug: switching its
filtersleft the count reporting the oldfilter's mutations.
The fix
The result is recomputed on render as well.
useIsFetchingdoes the same work directly insidegetSnapshot, which it canafford because it returns a
number. This hook returns an array, and a fresh arrayper call never satisfies React's snapshot consistency check — I tried it, and it
logs "The result of getSnapshot should be cached to avoid an infinite loop" and
then throws
Maximum update depth exceeded. So the array stays in a ref.Recomputing unconditionally would then re-run
selecton every unrelated render,which
QueryObserveralready avoids by memoising onoptions.select === this.#selectFn.The recompute is guarded the same way —
replaceEqualDeeponfilters, identity onselect— so an unrelated render costs nothing, and inline options behave exactly asthey do in
useQuerytoday.Tests
Six added to
useMutationState.test.tsx. Four fail onmainbecause the value neverupdates: filters change,
selectchanges, filters stop matching, anduseIsMutatingwith changing filters. The other two are regression guards, and each fails against a
specific defect rather than passing trivially — removing
replaceEqualDeepfails thereference-stability test, and removing the guard fails "should not re-run a stable
select on an unrelated render".
pnpm test:libon@tanstack/react-query: 35 files, 577 tests passing.Other adapters
preact-query'suseMutationStateis a line-for-line copy and has the identicalbug;
svelte-queryandsolid-queryshare the shape.vue-queryalready fixes it,via a
watchon the resolved options.Left out of this PR deliberately, following how the repo split the same situation
recently — #11130 fixed
useQueriesin react and #11315 did preact separately, withits own changeset — and the same for #11305. Happy to follow up on the others once
the approach here is settled.
Summary by CodeRabbit
Bug Fixes
useMutationStateso results update when filters or selection logic change.useIsMutatingvalues when its filters change.Tests