fix(vue-query): fix type of queryOptions to allow plain properies or getters#10452
fix(vue-query): fix type of queryOptions to allow plain properies or getters#10452DamianOsipiuk merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds getter/thunk support across vue-query: new Changes
Sequence DiagramsequenceDiagram
participant User
participant queryOptions
participant QueryClient
participant toValueDeep
participant Core
User->>queryOptions: provide plain object or zero-arg getter
alt Getter path
queryOptions->>queryOptions: accept/return thunk overload
queryOptions-->>User: thunk that yields options
User->>QueryClient: call invalidateQueries/fetchQuery with thunk
else Plain value path
queryOptions->>queryOptions: accept direct overload
User->>QueryClient: call invalidateQueries/fetchQuery with value
end
QueryClient->>toValueDeep: normalize filters/options (invoke if function)
toValueDeep-->>QueryClient: normalized plain value
QueryClient->>Core: forward normalized filters/options to core API
Core-->>QueryClient: result/ack
QueryClient-->>User: update/refetch behavior
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 0649ccb
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview2 package(s) bumped directly, 22 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
47e4c6b to
0649ccb
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vue-query/src/queryClient.ts (1)
304-310:⚠️ Potential issue | 🔴 CriticalImplementation signature and body don't handle getter functions.
The overload (lines 286-296) accepts
(() => FetchQueryOptions<...>), but the implementation signature (lines 305-307) only declaresMaybeRefDeep<FetchQueryOptions<...>>. When a getter is passed, it will be mishandled bycloneDeepUnrefthe same way as ininvalidateQueries.🐛 Proposed fix to handle getters in fetchQuery
fetchQuery< TQueryFnData, TError = DefaultError, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey, TPageParam = never, >( - options: MaybeRefDeep< - FetchQueryOptions<TQueryFnData, TError, TData, TQueryKey, TPageParam> - >, + options: + | MaybeRefDeep< + FetchQueryOptions<TQueryFnData, TError, TData, TQueryKey, TPageParam> + > + | (() => FetchQueryOptions<TQueryFnData, TError, TData, TQueryKey, TPageParam>), ): Promise<TData> { - return super.fetchQuery(cloneDeepUnref(options)) + return super.fetchQuery( + cloneDeepUnref( + toValueDeep(options) as MaybeRefDeep< + FetchQueryOptions<TQueryFnData, TError, TData, TQueryKey, TPageParam> + >, + ), + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-query/src/queryClient.ts` around lines 304 - 310, The implementation of fetchQuery doesn't handle getter functions even though an overload accepts (() => FetchQueryOptions<...>); update the implementation signature to accept either a getter or MaybeRefDeep (e.g., options: MaybeRefDeep<FetchQueryOptions<...>> | (() => FetchQueryOptions<...>)) and inside fetchQuery resolve the getter before cloning: detect if options is a function (the getter) and call it to obtain the options, then pass the resolved value to cloneDeepUnref and forward to super.fetchQuery; reference fetchQuery and cloneDeepUnref to locate where to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/three-pillows-enter.md:
- Line 5: Typo in the changeset title string "fix(vue-query): fix type of
queryOptions to allow plain properies or getters" — change "properies" to
"properties" so the sentence reads "...allow plain properties or getters";
update that exact text in the .changeset entry to correct the spelling.
In `@packages/vue-query/src/queryClient.ts`:
- Line 3: The file imports toValueDeep but never uses it; update the call sites
where query keys or args are processed (notably in functions/methods named
invalidateQueries and fetchQuery) to resolve getter functions before
deep-unwrapping by passing inputs through toValueDeep and then into
cloneDeepUnref (e.g., replace direct cloneDeepUnref(someInput) with
cloneDeepUnref(toValueDeep(someInput))). This ensures getter-based keys/params
are evaluated prior to cloning and keeps the existing import usage.
- Around line 205-209: The public overload for invalidateQueries in
queryClient.ts is missing getter support for the options parameter; update the
overload’s options type to include a getter by changing options?:
MaybeRefDeep<InvalidateOptions> to options?: MaybeRefDeep<InvalidateOptions> |
(() => InvalidateOptions) (or (() => MaybeRefDeep<InvalidateOptions>) to match
your ref handling) so the public signature matches the implementation that
accepts (() => InvalidateOptions); adjust the overload for invalidateQueries and
ensure references to InvalidateOptions and MaybeRefDeep remain consistent.
- Around line 216-221: The code currently passes potential getter functions
directly into cloneDeepUnref (e.g., when building filtersCloned and
optionsCloned), but cloneDeepUnref(unrefGetters=false) won't invoke root-level
getters so function objects slip through; fix by first resolving any getters
with toValueDeep on the original inputs (filters and options) and then pass the
resolved values into cloneDeepUnref (i.e., replace the direct
cloneDeepUnref(filters as MaybeRefDeep<...>) and cloneDeepUnref(options as
MaybeRefDeep<...>) flows with toValueDeep(...) followed by cloneDeepUnref(...)
so root-level getter functions are invoked before cloning).
In `@packages/vue-query/src/queryOptions.ts`:
- Around line 25-27: The mapped type for QueryOptions incorrectly forces the
'enabled' property to be a getter only; change the mapping for Property extends
'enabled' in QueryOptions so it accepts the core library's Enabled type (boolean
| (query)=>boolean) rather than only a no-arg function. Locate the mapped type
in QueryOptions (the clause currently returning "() => Enabled<...>") and
replace it with the correct Enabled<TQueryFnData, TError, TQueryData,
DeepUnwrapRef<TQueryKey>> type (or a union of boolean and the predicate
signature) so that plain true/false and predicate functions are both accepted;
ensure QueryObserverOptions usage remains unchanged.
In `@packages/vue-query/src/utils.ts`:
- Around line 113-114: toValueDeep currently returns source() directly when
source is a function, skipping deep unref/clone normalization and causing
inconsistent behavior; modify toValueDeep so it always passes the resolved value
through cloneDeepUnref (e.g., compute const resolved = isFunction(source) ?
source() : source and return cloneDeepUnref(resolved)) so both function and
non-function inputs get identical deep-unref/clone treatment; keep references to
toValueDeep, isFunction, cloneDeepUnref and MaybeRefDeep in the change.
---
Outside diff comments:
In `@packages/vue-query/src/queryClient.ts`:
- Around line 304-310: The implementation of fetchQuery doesn't handle getter
functions even though an overload accepts (() => FetchQueryOptions<...>); update
the implementation signature to accept either a getter or MaybeRefDeep (e.g.,
options: MaybeRefDeep<FetchQueryOptions<...>> | (() => FetchQueryOptions<...>))
and inside fetchQuery resolve the getter before cloning: detect if options is a
function (the getter) and call it to obtain the options, then pass the resolved
value to cloneDeepUnref and forward to super.fetchQuery; reference fetchQuery
and cloneDeepUnref to locate where to change.
🪄 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: 3298b37d-8c6f-49e8-b9b3-09276d25ac36
📒 Files selected for processing (9)
.changeset/three-pillows-enter.mdpackages/vue-query/src/__tests__/queryOptions.test-d.tspackages/vue-query/src/__tests__/queryOptions.test.tspackages/vue-query/src/__tests__/useQueries.test-d.tspackages/vue-query/src/index.tspackages/vue-query/src/queryClient.tspackages/vue-query/src/queryOptions.tspackages/vue-query/src/types.tspackages/vue-query/src/utils.ts
| '@tanstack/vue-query': patch | ||
| --- | ||
|
|
||
| fix(vue-query): fix type of queryOptions to allow plain properies or getters |
There was a problem hiding this comment.
Fix typo in changeset text.
properies should be properties in the release note sentence.
📝 Proposed edit
-fix(vue-query): fix type of queryOptions to allow plain properies or getters
+fix(vue-query): fix type of queryOptions to allow plain properties or getters📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fix(vue-query): fix type of queryOptions to allow plain properies or getters | |
| fix(vue-query): fix type of queryOptions to allow plain properties or getters |
🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Ensure spelling is correct
Context: ...fix type of queryOptions to allow plain properies or getters
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/three-pillows-enter.md at line 5, Typo in the changeset title
string "fix(vue-query): fix type of queryOptions to allow plain properies or
getters" — change "properies" to "properties" so the sentence reads "...allow
plain properties or getters"; update that exact text in the .changeset entry to
correct the spelling.
| const filtersCloned = cloneDeepUnref( | ||
| filters as MaybeRefDeep<InvalidateQueryFilters<TTaggedQueryKey>>, | ||
| ) | ||
| const optionsCloned = cloneDeepUnref( | ||
| options as MaybeRefDeep<InvalidateOptions>, | ||
| ) |
There was a problem hiding this comment.
Getter functions passed as arguments will not be invoked.
When a getter function () => InvalidateQueryFilters is passed, the current implementation casts it to MaybeRefDeep<T> and passes it directly to cloneDeepUnref. However, cloneDeepUnref with default parameters (unrefGetters=false) does not invoke root-level getter functions—it only handles nested getters for queryKey at level 1.
This means the function object itself (not its return value) will be passed to query-core, causing silent runtime failures.
The getter should be resolved first using toValueDeep before cloning:
🐛 Proposed fix to resolve getters before cloning
- const filtersCloned = cloneDeepUnref(
- filters as MaybeRefDeep<InvalidateQueryFilters<TTaggedQueryKey>>,
- )
- const optionsCloned = cloneDeepUnref(
- options as MaybeRefDeep<InvalidateOptions>,
- )
+ const filtersCloned = cloneDeepUnref(
+ toValueDeep(filters) as MaybeRefDeep<InvalidateQueryFilters<TTaggedQueryKey>>,
+ )
+ const optionsCloned = cloneDeepUnref(
+ toValueDeep(options) as MaybeRefDeep<InvalidateOptions>,
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-query/src/queryClient.ts` around lines 216 - 221, The code
currently passes potential getter functions directly into cloneDeepUnref (e.g.,
when building filtersCloned and optionsCloned), but
cloneDeepUnref(unrefGetters=false) won't invoke root-level getters so function
objects slip through; fix by first resolving any getters with toValueDeep on the
original inputs (filters and options) and then pass the resolved values into
cloneDeepUnref (i.e., replace the direct cloneDeepUnref(filters as
MaybeRefDeep<...>) and cloneDeepUnref(options as MaybeRefDeep<...>) flows with
toValueDeep(...) followed by cloneDeepUnref(...) so root-level getter functions
are invoked before cloning).
| >]: Property extends 'enabled' | ||
| ? () => Enabled<TQueryFnData, TError, TQueryData, DeepUnwrapRef<TQueryKey>> | ||
| : QueryObserverOptions< |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Inspect Enabled/QueryObserverOptions definitions in query-core"
rg -n --type=ts -C3 'export type Enabled|enabled\??:' packages/query-core
echo
echo "2) Inspect vue-query QueryOptions enabled mapping"
rg -n --type=ts -C3 "Property extends 'enabled'" packages/vue-query/src/queryOptions.ts
echo
echo "3) Find boolean-enabled queryOptions call patterns in vue-query tests"
rg -n --type=ts -C2 'enabled:\s*(true|false)' packages/vue-query/src/__tests__Repository: TanStack/query
Length of output: 22769
🏁 Script executed:
# Get the full Enabled type definition from query-core
sed -n '113,125p' packages/query-core/src/types.ts
echo "---"
# Check the full QueryOptions field definition in query-core
sed -n '326,330p' packages/query-core/src/types.ts
echo "---"
# View the full QueryOptions mapped type in vue-query to understand the structure
sed -n '12,40p' packages/vue-query/src/queryOptions.tsRepository: TanStack/query
Length of output: 1291
enabled is now getter-only; plain enabled values are no longer accepted.
The QueryOptions mapped type restricts enabled to () => Enabled<...> (function-only), but the core library's Enabled type is defined as boolean | ((query) => boolean). This breaks existing code using enabled: true or enabled: false.
Proposed type fix
>]: Property extends 'enabled'
- ? () => Enabled<TQueryFnData, TError, TQueryData, DeepUnwrapRef<TQueryKey>>
+ ?
+ | Enabled<TQueryFnData, TError, TQueryData, DeepUnwrapRef<TQueryKey>>
+ | (() => Enabled<
+ TQueryFnData,
+ TError,
+ TQueryData,
+ DeepUnwrapRef<TQueryKey>
+ >)
: QueryObserverOptions<🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-query/src/queryOptions.ts` around lines 25 - 27, The mapped type
for QueryOptions incorrectly forces the 'enabled' property to be a getter only;
change the mapping for Property extends 'enabled' in QueryOptions so it accepts
the core library's Enabled type (boolean | (query)=>boolean) rather than only a
no-arg function. Locate the mapped type in QueryOptions (the clause currently
returning "() => Enabled<...>") and replace it with the correct
Enabled<TQueryFnData, TError, TQueryData, DeepUnwrapRef<TQueryKey>> type (or a
union of boolean and the predicate signature) so that plain true/false and
predicate functions are both accepted; ensure QueryObserverOptions usage remains
unchanged.
| export function toValueDeep<T>(source: (() => T) | MaybeRefDeep<T>): T { | ||
| return isFunction(source) ? source() : cloneDeepUnref(source) |
There was a problem hiding this comment.
Getter branch skips deep unref/clone normalization.
When source is a function, toValueDeep returns source() directly, so nested refs/getters are not normalized like they are for non-function input. This can produce different runtime behavior for equivalent inputs.
💡 Proposed fix
export function toValueDeep<T>(source: (() => T) | MaybeRefDeep<T>): T {
- return isFunction(source) ? source() : cloneDeepUnref(source)
+ const value = isFunction(source) ? source() : source
+ return cloneDeepUnref(value as MaybeRefDeep<T>)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function toValueDeep<T>(source: (() => T) | MaybeRefDeep<T>): T { | |
| return isFunction(source) ? source() : cloneDeepUnref(source) | |
| export function toValueDeep<T>(source: (() => T) | MaybeRefDeep<T>): T { | |
| const value = isFunction(source) ? source() : source | |
| return cloneDeepUnref(value as MaybeRefDeep<T>) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-query/src/utils.ts` around lines 113 - 114, toValueDeep
currently returns source() directly when source is a function, skipping deep
unref/clone normalization and causing inconsistent behavior; modify toValueDeep
so it always passes the resolved value through cloneDeepUnref (e.g., compute
const resolved = isFunction(source) ? source() : source and return
cloneDeepUnref(resolved)) so both function and non-function inputs get identical
deep-unref/clone treatment; keep references to toValueDeep, isFunction,
cloneDeepUnref and MaybeRefDeep in the change.
size-limit report 📦
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vue-query/src/queryClient.ts (1)
286-310:⚠️ Potential issue | 🔴 Critical
fetchQueryoverload accepts getter but implementation doesn't invoke it.The public overload accepts
(() => FetchQueryOptions<...>), but the implementation only acceptsMaybeRefDeep<FetchQueryOptions<...>>. When a getter function is passed,cloneDeepUnrefwill not invoke it—the function object itself gets passed tosuper.fetchQuery(), causing a runtime error.Replace
cloneDeepUnref(options)withtoValueDeep(options)to properly handle both getter functions and reactive references. ThetoValueDeeputility already handles this pattern correctly by checkingisFunction(source)first before falling back tocloneDeepUnref.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-query/src/queryClient.ts` around lines 286 - 310, The fetchQuery implementation currently passes cloneDeepUnref(options) to super.fetchQuery which doesn't invoke getter overloads; change the call in the fetchQuery method to use toValueDeep(options) so functions (getter overloads) and refs are resolved before forwarding to super.fetchQuery; ensure you update any imports to include toValueDeep and keep the existing overload signatures (MaybeRefDeep and FetchQueryOptions) unchanged.
🧹 Nitpick comments (1)
packages/vue-query/src/index.ts (1)
8-9: Consider combining exports from./queryOptions.Both the runtime
queryOptionsfunction and theQueryOptionstype are exported from the same module. These can be combined into a single export statement.♻️ Optional consolidation
-export { queryOptions } from './queryOptions' -export { type QueryOptions } from './queryOptions' +export { queryOptions, type QueryOptions } from './queryOptions'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-query/src/index.ts` around lines 8 - 9, The exports in index.ts export both the runtime symbol queryOptions and the type QueryOptions separately; combine them into a single consolidated export from './queryOptions' by exporting queryOptions and type QueryOptions together (reference symbols: queryOptions, QueryOptions, and module './queryOptions') so the file uses one export statement instead of two.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/vue-query/src/queryClient.ts`:
- Around line 286-310: The fetchQuery implementation currently passes
cloneDeepUnref(options) to super.fetchQuery which doesn't invoke getter
overloads; change the call in the fetchQuery method to use toValueDeep(options)
so functions (getter overloads) and refs are resolved before forwarding to
super.fetchQuery; ensure you update any imports to include toValueDeep and keep
the existing overload signatures (MaybeRefDeep and FetchQueryOptions) unchanged.
---
Nitpick comments:
In `@packages/vue-query/src/index.ts`:
- Around line 8-9: The exports in index.ts export both the runtime symbol
queryOptions and the type QueryOptions separately; combine them into a single
consolidated export from './queryOptions' by exporting queryOptions and type
QueryOptions together (reference symbols: queryOptions, QueryOptions, and module
'./queryOptions') so the file uses one export statement instead of two.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61d62eb3-86e6-47db-9747-92d7e1d567e7
📒 Files selected for processing (9)
.changeset/three-pillows-enter.mdpackages/vue-query/src/__tests__/queryOptions.test-d.tspackages/vue-query/src/__tests__/queryOptions.test.tspackages/vue-query/src/__tests__/useQueries.test-d.tspackages/vue-query/src/index.tspackages/vue-query/src/queryClient.tspackages/vue-query/src/queryOptions.tspackages/vue-query/src/types.tspackages/vue-query/src/utils.ts
✅ Files skipped from review due to trivial changes (3)
- packages/vue-query/src/tests/queryOptions.test.ts
- packages/vue-query/src/types.ts
- packages/vue-query/src/tests/useQueries.test-d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/vue-query/src/utils.ts
- packages/vue-query/src/tests/queryOptions.test-d.ts
🎯 Changes
Fixes: #7892
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
New Features