-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feat(query-core): implement client-informed server prefetching #10023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 49be237 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 |
📝 WalkthroughWalkthroughQueryCache can now export client cache timestamps; QueryClient accepts an optional clientCacheState and uses it in prefetchQuery/prefetchInfiniteQuery to skip network prefetches when client-held data is still fresh. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant QC as QueryClient
participant QCache as QueryCache / ClientCacheState
participant Server as Server
App->>QC: prefetchQuery(queryKey)
activate QC
QC->>QCache: lookup queryHash timestamp (clientCacheState)
alt timestamp exists and staleTime is static
QCache-->>QC: return timestamp
QC->>QC: compute timeUntilStale(staleTime, timestamp)
alt still fresh
QC-->>App: resolve without network fetch (skip)
else
QC->>Server: fetch query data
Server-->>QC: return data
QC-->>App: resolve with data
end
else no timestamp or staleTime dynamic
QC->>Server: fetch query data
Server-->>QC: return data
QC-->>App: resolve with data
end
deactivate QC
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/query-core/src/__tests__/clientCacheState.test.tsx:
- Around line 36-37: Remove the outdated "@ts-ignore - API not implemented yet"
comment above the QueryClient instantiation and any accompanying remark; the
clientCacheState property is already defined on QueryClientConfig (see
types.ts), so simply delete the ts-ignore line and the obsolete comment and
leave the line constructing new QueryClient({ clientCacheState }) as-is so
TypeScript can use the implemented types.
- Around line 27-34: The test comments in the it('should SKIP prefetch when
client has fresh data (Simulated)') block are outdated and claim the feature is
"NOT implemented" and types aren't implemented; update those comments to reflect
that clientCacheState and related types/behavior are now implemented in
queryClient.ts and types.ts (remove the "NOT implemented" note and the need to
cast to any), and clarify that the test simulates a QueryClient with
clientCacheState using the clientCacheState object defined in the test.
In @packages/query-core/src/queryClient.ts:
- Around line 385-406: prefetchInfiniteQuery currently skips the
clientCacheState/staleTime optimization that prefetchQuery uses; update
prefetchInfiniteQuery to call this.defaultQueryOptions(options), compute
queryHash and clientUpdatedAt from this.#clientCacheState, and if
clientUpdatedAt exists and staleTime is not a function then: return
Promise.resolve() when staleTime === 'static', and return Promise.resolve() when
timeUntilStale(clientUpdatedAt, staleTime) > 0; otherwise proceed to call
this.fetchInfiniteQuery(defaultedOptions).then(noop).catch(noop) so behavior
matches prefetchQuery's optimization.
🧹 Nitpick comments (2)
packages/query-core/src/__tests__/clientCacheState.test.tsx (2)
6-25: Add cleanup after each test to prevent test pollution.The
QueryClientinstance should be cleaned up after tests to avoid potential memory leaks or test interference in larger test suites.♻️ Suggested improvement
describe('Client Cache State', () => { + let queryClient: QueryClient + + afterEach(() => { + queryClient?.clear() + }) + it('should prefetch query normally when no client state is provided', async () => { - const queryClient = new QueryClient() + queryClient = new QueryClient() const key = ['test']
55-60: Clean up outdated inline comments.These comments describe expected behavior vs current behavior, but since the feature is implemented, the test should simply assert the expected outcome without the explanatory scaffold.
♻️ Simplified assertion
- // CURRENT BEHAVIOR: fetchCount is 1 (Server fetches anyway) - // DESIRED BEHAVIOR: fetchCount should be 0 (Server skips because client has it) - - // We expect this to equal 0 if our feature is working. - // For now, let's assert 0 and see it fail, proving the need for the feature. + // Server should skip prefetch because client has fresh data expect(fetchCount).toBe(0)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/query-core/src/__tests__/clientCacheState.test.tsxpackages/query-core/src/queryClient.tspackages/query-core/src/types.ts
🧰 Additional context used
🧠 Learnings (4)
📓 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/query-core/src/__tests__/clientCacheState.test.tsxpackages/query-core/src/types.tspackages/query-core/src/queryClient.ts
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Applied to files:
packages/query-core/src/types.ts
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
packages/query-core/src/types.tspackages/query-core/src/queryClient.ts
🔇 Additional comments (4)
packages/query-core/src/types.ts (2)
1355-1356: LGTM!The
ClientCacheStatetype is well-defined as a simple mapping from query hash strings to timestamps. This aligns with the prefetch skip logic inqueryClient.tsthat usestimeUntilStale(clientUpdatedAt, staleTime).
1357-1362: LGTM!The optional
clientCacheStateproperty is correctly added toQueryClientConfig, maintaining backward compatibility.packages/query-core/src/queryClient.ts (2)
71-71: LGTM!Private field initialization follows the existing pattern in the class. The optional assignment correctly handles undefined config values.
Also applies to: 82-82
387-392: Consider edge case:staleTimeof0with existing client cache entry.When
staleTimeis0,timeUntilStale(clientUpdatedAt, 0)will return0(sinceMath.max(updatedAt + 0 - Date.now(), 0)is 0 whenupdatedAt <= Date.now()), which means the condition> 0is false and prefetch proceeds. This is correct behavior -staleTime: 0means data is immediately stale.The logic is sound.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/query-core/src/queryClient.ts (1)
676-679: Consider a more specific type for thestaleTimeparameter.The
staleTimeparameter is currently typed asany. While this works, it could be more precisely typed to match the actual possible values (e.g.,number | 'static' | ((query: Query) => number | 'static') | undefinedor the appropriate type from the types module).♻️ Potential type improvement
Consider importing and using the precise staleTime type if one exists in the types module, or defining it explicitly to improve type safety:
#shouldSkipPrefetch( queryHash: string, - staleTime: any, + staleTime: number | 'static' | ((query: any) => number | 'static') | undefined, ): boolean {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/query-core/src/__tests__/clientCacheState.test.tsxpackages/query-core/src/queryClient.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/query-core/src/tests/clientCacheState.test.tsx
🧰 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/query-core/src/queryClient.ts
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
packages/query-core/src/queryClient.ts
🔇 Additional comments (6)
packages/query-core/src/queryClient.ts (6)
3-47: LGTM: Import additions and reorganization are correct.The new imports for
ClientCacheStateandtimeUntilStaleare properly added to support the client-informed prefetching feature.
71-71: LGTM: Private field addition follows class conventions.The optional
#clientCacheStatefield is correctly typed and follows the existing private field pattern.
82-82: LGTM: Constructor initialization is correct.The field is properly initialized from the config parameter.
385-396: LGTM: Prefetch skip logic is correctly integrated.The method now efficiently checks if prefetching can be skipped based on client cache state before performing any expensive operations. The use of
defaultedOptionsthroughout is correct.
438-449: LGTM: Infinite query prefetch logic mirrors standard prefetch.The implementation correctly applies the same skip optimization to infinite queries. The
as anycast on line 438 is consistent with the existing pattern used infetchInfiniteQuery.
680-704: LGTM: Skip-prefetch logic is well-structured and correctly handles all edge cases.The implementation correctly:
- Returns early when optimization cannot be applied (no client cache state, undefined staleTime, function staleTime)
- Skips prefetch when client data is stale or applies early returns for always-fresh scenarios ('static' staleTime)
- Uses proper guard clauses with efficient ordering
The
timeUntilStalehelper correctly handles edge cases:Math.max(updatedAt + (staleTime || 0) - Date.now(), 0)ensures no negative values are returned, and the> 0comparison accurately identifies fresh vs stale data. All potential edge cases (zero staleTime, undefined staleTime, string 'static', and function staleTime) are properly handled before or during the calculation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/query-core/src/queryCache.ts (1)
225-239: Consider adding JSDoc documentation for this public API method.The implementation looks correct and follows the established patterns in the codebase. It properly filters queries, checks for success status, and extracts the
dataUpdatedAttimestamps. The logic is thread-safe since it operates on a snapshot fromfindAll.Since this is a new public API method that's part of the client-informed prefetching feature, adding JSDoc would improve developer experience by documenting the purpose, parameters, and return value.
📚 Suggested JSDoc documentation
+ /** + * Extracts client cache state for server-side prefetch optimization. + * Returns a mapping of query hashes to their dataUpdatedAt timestamps + * for all queries with 'success' status. + * + * @param filter - Optional QueryFilters to limit which queries are included + * @returns Record of queryHash to dataUpdatedAt timestamp (in milliseconds) + */ extractClientCacheState( filter?: QueryFilters, ): ClientCacheState {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/client-informed-prefetch.mdpackages/query-core/src/queryCache.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
.changeset/client-informed-prefetch.md
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Applied to files:
packages/query-core/src/queryCache.ts
🧬 Code graph analysis (1)
packages/query-core/src/queryCache.ts (2)
packages/query-core/src/utils.ts (1)
QueryFilters(30-55)packages/query-core/src/types.ts (1)
ClientCacheState(1355-1355)
🔇 Additional comments (2)
.changeset/client-informed-prefetch.md (1)
1-5: Changeset format looks correct.The YAML frontmatter, version bump, and feature description are properly formatted and aligned with the PR objectives. The "minor" bump is appropriate for a new feature.
packages/query-core/src/queryCache.ts (1)
1-16: LGTM! Import organization is clean.The imports are properly organized with the necessary types and utilities for the new
extractClientCacheStatemethod.
🎯 Changes
QueryClientto respect the client's existing cache state.ClientCacheStateType: AddedRecord<string, number>type for transmitting query freshness info.QueryClientUpdates:clientCacheStatetoQueryClientConfig.prefetchQueryandprefetchInfiniteQueryto skip server-side fetching if the client has fresh data.#shouldSkipPrefetchhelper.QueryCacheUpdates: AddedextractClientCacheState()helper to easily generate state for the server.Currently, SSR prefetching is redundant if the client already has fresh data. This change reduces server load and HTML payload size.
✅ Checklist
pnpm run test:lib.🚀 Release Impact
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.