fix(query-core): ensure query refetches on mount/retry when status is error (#9728) #9927
Conversation
🦋 Changeset detectedLatest commit: 08a6f2b The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughAdds a changeset documenting a patch for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
xiangnuans
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/open-keys-create.md (1)
1-5: Changeset scope and message look accurate; optionally mention focus/reconnect.Patch scope to
@tanstack/query-coreand the summary align with theshouldFetchOnchange. Note that the implementation also affects refetch on window focus and reconnect via the same helper, which is fine but not called out here; up to you if you want to spell that out in the note.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/open-keys-create.md(1 hunks)packages/query-core/src/queryObserver.ts(1 hunks)packages/react-query/src/__tests__/issue-9728.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/react-query/src/__tests__/issue-9728.test.tsx
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
.changeset/open-keys-create.md
🧬 Code graph analysis (2)
packages/query-core/src/queryObserver.ts (1)
packages/query-core/src/query.ts (1)
isStale(296-306)
packages/react-query/src/__tests__/issue-9728.test.tsx (1)
packages/query-core/src/queryObserver.ts (1)
refetch(298-304)
🔇 Additional comments (4)
packages/query-core/src/queryObserver.ts (1)
771-791: Error-awareshouldFetchOnlogic matches the intended fix.Adding
query.state.status === 'error'to the fetch predicate ensures that enabled queries with non‑static staleTime will refetch on mount/focus/reconnect when they are in an error state, even if their data isn’t time‑stale, which is exactly what issue 9728 describes. Guarding withvalue !== falsepreserves opt‑out viarefetchOn* = false, and the existingstaleTime === 'static'early return means static queries keep their previous behavior.packages/react-query/src/__tests__/issue-9728.test.tsx (3)
16-36: Deterministic queryFn and client config nicely model the regression.Using a
vi.fnwith acount‑based async implementation plusretry: falseandstaleTime: Infinityprovides a clear, deterministic setup for success→error→success, matching the reported bug scenario.
37-78: Error boundary + reset wiring correctly exercises “retry via remount”.
Pagethrowing viathrowOnError: trueunderErrorBoundaryandQueryErrorResetBoundary, with the Retry button callingresetErrorBoundary, ensures the query is remounted after an error in the presence of existing data—exactly the path the core fix targets.
88-107: Assertions cover initial success, error transition, and final successful refetch.The three phases—first render success (1 call), refetch leading to error UI (2 calls), and Retry leading to “Success 3” (3 calls)—give good coverage for the bug and will regress if
shouldFetchOnstops refetching on error with non‑stale data.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:sherif,test:knip,tes... |
❌ Failed | 3m 13s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 1s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-12-28 19:42:20 UTC
…king error status - Remove error status check in shouldFetchOn - Set isInvalidated: true in reducer when background error occurs (only if data exists) - Add tests for staleTime: 'static' and non-static queries with background errors This approach centralizes stale logic in isStale/isStaleByTime and prevents regression where staleTime: 'static' queries would incorrectly refetch on window focus after a background error.
…nuans/query into fix/9728-retry-after-error
Set isInvalidated: true unconditionally since queries with no data are always considered stale per isStaleByTime logic.
…nuans/query into fix/9728-retry-after-error
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-query/src/__tests__/issue-9728.test.tsx (1)
38-42: Consider removing the unused forceUpdate pattern.The
useStateanduseEffectcombination forces a re-render on mount, but this doesn't appear necessary for the test scenario. The test assertions pass without relying on this behavior, and it may confuse future readers.🔎 Suggested simplification
function Page() { - const [_, forceUpdate] = React.useState(0) - - React.useEffect(() => { - forceUpdate(1) - }, []) - const { data, refetch } = useQuery({ queryKey: key, queryFn, throwOnError: true, })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-query/src/__tests__/issue-9728.test.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/react-query/src/__tests__/issue-9728.test.tsx
🔇 Additional comments (1)
packages/react-query/src/__tests__/issue-9728.test.tsx (1)
1-108: Well-structured regression test for issue 9728.The test correctly validates that queries in error state with
staleTime: Infinitywill refetch on mount/retry. The mock setup, error boundary integration, and async assertions are implemented properly. The three-step test flow (success → error → retry success) clearly demonstrates the fix.Based on learnings, the
asyncqueryFn with side effects (incrementingcount) follows the recommended pattern.
|
your new tests don’t seem to pass |
2 participants
error (#9728)
🎯 Changes
shouldFetchOninpackages/query-core/src/queryObserver.ts:query.state.status === 'error'.refetchOnWindowFocus,refetchOnMount, or manual retry via ErrorBoundary), it will trigger a fetch if the query is in an error state, even if the data is technically considered "fresh" (not stale) due tostaleTime: Infinity.packages/react-query/src/__tests__/issue-9728.test.tsxto reproduce the issue and verify the fix.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.