feat(query-core): add simplified query methods#10658
feat(query-core): add simplified query methods#10658DogPawHat wants to merge 1 commit intoTanStack:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds imperative ChangesQuery Execute Methods and Type Refactor
Sequence Diagram(s)sequenceDiagram
participant Client as QueryClient
participant Cache as QueryCache
participant Fn as queryFn
participant Test as TestRunner
Test->>Client: call query()/infiniteQuery()
Client->>Cache: read cached entry
alt cache present & not stale
Cache-->>Client: return cached data
else cache missing or stale
Client->>Fn: invoke queryFn
Fn-->>Client: return fetched data
Client->>Cache: store fetched data
end
Client-->>Test: return Promise with (selected) data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
View your CI Pipeline Execution ↗ for commit 5d51700
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/true-cameras-wash.md:
- Line 5: Fix the spelling mistake in the changeset text by replacing the
misspelled token "impertive" with the correct word "imperative" in the changeset
entry that reads "add query and infiniteQuery methods, deprecate old impertive
methods" so release notes and docs show the correct wording.
In `@packages/query-core/src/queryClient.ts`:
- Around line 370-380: The code currently calls this.#queryCache.build(...)
before checking enabled, which creates an empty query entry on "skip" requests;
to fix, evaluate isEnabled by calling
resolveQueryBoolean(defaultedOptions.enabled, /* without building */) first and
if it's false check the cache lookup (e.g.
this.#queryCache.get(defaultedOptions.queryHash)?.state.data) for existing data
— only call this.#queryCache.build(this, defaultedOptions) when enabled or when
cached data exists; if not enabled and no cached data, reject with the existing
error message. Use the symbols resolveQueryBoolean, defaultedOptions.enabled,
this.#queryCache.get / this.#queryCache.build, query.state.data, and
defaultedOptions.queryHash to locate and implement the change.
- Around line 346-362: The generic parameter order for the query() method was
changed so that TQueryData is inserted before TQueryKey, which shifts TQueryKey
from the 4th position and breaks callers that supply explicit generics; restore
backwards compatibility by reordering the type parameters on query<TQueryFnData,
TError = DefaultError, TData = TQueryFnData, TQueryKey extends QueryKey =
QueryKey, TQueryData = TQueryFnData, TPageParam = never> (matching how
fetchQuery previously exposed TQueryKey as the 4th type param) and ensure the
QueryExecuteOptions type usage aligns with that order so existing
explicit-generic call sites do not need an extra type argument.
- Around line 448-469: The third generic TData in infiniteQuery currently
conflicts with fetchInfiniteQuery because TData is used for different meanings
(page data vs selected result), causing unsafe migrations; update
infiniteQuery's signature to introduce a distinct generic for the
select/returned result (e.g. add TSelect = TData or rename the existing generics
so that TData remains the page data type/TQueryFnData output) and keep the
return type built from InfiniteData<TQueryFnData, TPageParam> (or conditional
based on the new TSelect) so callers like infiniteQuery<TQueryFnData, TError,
TData, TQueryKey, TPageParam> still resolve to InfiniteData<TQueryFnData,
TPageParam>; change the type parameter list and the conditional return type in
the infiniteQuery method and update any references to TData inside
InfiniteQueryExecuteOptions to the appropriate new generic (reference symbols:
infiniteQuery, fetchInfiniteQuery, TData, TQueryFnData, InfiniteData,
InfiniteQueryExecuteOptions, TPageParam).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47b6c09c-48bc-4907-a213-997c33f872bf
📒 Files selected for processing (5)
.changeset/true-cameras-wash.mdpackages/query-core/src/__tests__/queryClient.test-d.tsxpackages/query-core/src/__tests__/queryClient.test.tsxpackages/query-core/src/queryClient.tspackages/query-core/src/types.ts
3fd9db4 to
e2fa928
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/query-core/src/queryClient.ts`:
- Around line 435-436: Update the deprecation JSDoc for the deprecated method to
give a copy-pasteable no-op catch example: replace `.catch(noop)` with a literal
no-op callback like `.catch(() => {})` in the deprecation text that references
using `queryClient.query(options)` (update both occurrences around
`queryClient.query` at the commented locations).
- Around line 142-144: The deprecation comment for
ensureQueryData/ensureInfiniteQueryData must note that queryClient.query({ ...,
staleTime: 'static' }) only implements the "return cached data or fetch"
behavior and does NOT preserve the background refresh that
ensureQueryData/ensureInfiniteQueryData perform when revalidateIfStale is true;
update the deprecation wording to: recommend migrating to queryClient.query({
..., staleTime: 'static' }) for simple cached-or-fetch semantics, and if callers
rely on the background revalidation, instruct them to follow the query call with
an explicit background revalidation (e.g. queryClient.fetchQuery or
queryClient.fetchInfiniteQuery) or an equivalent explicit revalidate step
instead of assuming revalidateIfStale is preserved. Include references to
ensureQueryData, ensureInfiniteQueryData, queryClient.query, revalidateIfStale,
fetchQuery and fetchInfiniteQuery in the note.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0af20707-a09c-4d31-a931-fba6a2b59452
📒 Files selected for processing (5)
.changeset/true-cameras-wash.mdpackages/query-core/src/__tests__/queryClient.test-d.tsxpackages/query-core/src/__tests__/queryClient.test.tsxpackages/query-core/src/queryClient.tspackages/query-core/src/types.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/true-cameras-wash.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/query-core/src/types.ts
- packages/query-core/src/tests/queryClient.test-d.tsx
- packages/query-core/src/tests/queryClient.test.tsx
3f889bb to
59cacda
Compare
| >, | ||
| ): Promise<TData> { | ||
| const defaultedOptions = this.defaultQueryOptions(options) | ||
| const disabledErrorMessage = `Query is disabled and no cached data is available for key: '${defaultedOptions.queryHash}'` |
There was a problem hiding this comment.
@TkDodo Per your comment here #9835 (comment)
Would the working of this error work?
59cacda to
0a9f91e
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/query-core/src/queryClient.ts (2)
346-361:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore
TQueryKeyas the 4th generic onquery().Line 350 still inserts
TQueryDataahead ofTQueryKey, so a straight rename fromfetchQuery<A, B, C, D>toquery<A, B, C, D>bindsDto query data instead of the key type. For a replacement API, that’s a breaking migration for explicit-generic call sites. A small dts regression aroundquery<..., StrictQueryKey>would be useful here too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-core/src/queryClient.ts` around lines 346 - 361, The generic parameter order on query(...) is incorrect: restore TQueryKey as the 4th generic parameter so explicit-generic call sites keep the same bindings (i.e. change the generic list on function query<TQueryFnData, TError = DefaultError, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey, TQueryData = TQueryFnData, TPageParam = never>), adjust the default for TQueryData accordingly, and update any usages/types that reference QueryExecuteOptions<TQueryFnData, TError, TData, TQueryData, TQueryKey, TPageParam> (or vice versa) to match the corrected order; also add/adjust a small .d.ts regression test demonstrating query<..., StrictQueryKey> still binds the 4th generic to the key type.
457-475:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftKeep the 3rd generic’s meaning aligned with
fetchInfiniteQuery().
fetchInfiniteQuery<Foo, Error, Bar, Key, number>returnsPromise<InfiniteData<Bar, number>>, but the currentinfiniteQuery<Foo, Error, Bar, Key, number>signature resolves toPromise<Bar>. That makes the advertised rename unsafe for explicit-generic callers. Please split “page data” from “selected result” with a separate generic instead of reusingTDatafor both roles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-core/src/queryClient.ts` around lines 457 - 475, The infiniteQuery signature reuses TData for both "page data" and "selected result", causing Promise<Bar> instead of Promise<InfiniteData<Bar, number>); change infiniteQuery's generics to mirror fetchInfiniteQuery by introducing a new generic (e.g. TSelected or TOutput) distinct from the page-data generic (keep TQueryFnData and TPageParam) and use that new generic for the method's resolved Promise type while keeping the page-data generic for InfiniteData<TQueryFnData, TPageParam>; update the function return type and any related type mappings in infiniteQuery accordingly so explicit-generic callers get Promise<InfiniteData<Bar, number>> like fetchInfiniteQuery.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/query-core/src/queryClient.ts`:
- Around line 346-361: The generic parameter order on query(...) is incorrect:
restore TQueryKey as the 4th generic parameter so explicit-generic call sites
keep the same bindings (i.e. change the generic list on function
query<TQueryFnData, TError = DefaultError, TData = TQueryFnData, TQueryKey
extends QueryKey = QueryKey, TQueryData = TQueryFnData, TPageParam = never>),
adjust the default for TQueryData accordingly, and update any usages/types that
reference QueryExecuteOptions<TQueryFnData, TError, TData, TQueryData,
TQueryKey, TPageParam> (or vice versa) to match the corrected order; also
add/adjust a small .d.ts regression test demonstrating query<...,
StrictQueryKey> still binds the 4th generic to the key type.
- Around line 457-475: The infiniteQuery signature reuses TData for both "page
data" and "selected result", causing Promise<Bar> instead of
Promise<InfiniteData<Bar, number>); change infiniteQuery's generics to mirror
fetchInfiniteQuery by introducing a new generic (e.g. TSelected or TOutput)
distinct from the page-data generic (keep TQueryFnData and TPageParam) and use
that new generic for the method's resolved Promise type while keeping the
page-data generic for InfiniteData<TQueryFnData, TPageParam>; update the
function return type and any related type mappings in infiniteQuery accordingly
so explicit-generic callers get Promise<InfiniteData<Bar, number>> like
fetchInfiniteQuery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 517f0021-cf70-4ffc-b4fa-40fc1c877f32
📒 Files selected for processing (5)
.changeset/true-cameras-wash.mdpackages/query-core/src/__tests__/queryClient.test-d.tsxpackages/query-core/src/__tests__/queryClient.test.tsxpackages/query-core/src/queryClient.tspackages/query-core/src/types.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/true-cameras-wash.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/query-core/src/types.ts
- packages/query-core/src/tests/queryClient.test.tsx
- packages/query-core/src/tests/queryClient.test-d.tsx
4006613 to
07a10f3
Compare
07a10f3 to
5d51700
Compare
🎯 Changes
implements #9135
queryandinfiniteQuerymethods as replacements forfetchQuery/fetchInfiniteQueryselect,enabledandqueryFn === skipTokenenabled === falseorqueryFn === skipTokenand no cached data is able to be returned.fetchQueryfetch tests plus the new scenarios introduced (enabled, select, skipToken, static staleTime)fetchQuery,prefetchQuery,ensureQueryData)Docs and Adaptor packages releases are added in follow up PR's stacked on top of this one:
Framework adaptors:
Documentation:
Old PR and history here #9835
Reopening the new PR to clean up commit history
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Deprecations
Tests