feat(query-core): add simplified query methods#10658
feat(query-core): add simplified query methods#10658DogPawHat wants to merge 1 commit intoTanStack:mainfrom
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 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 3f889bb
☁️ 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
e2fa928 to
3f889bb
Compare
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?
🎯 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 will be added in follow on PR's
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