-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
upd(vue-query): Improve queryOptions() type #9774
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: 8d3f77a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
WalkthroughRefactors vue-query TypeScript types to support Vue reactive wrappers (Ref/ComputedRef) for query options, splitting UseQueryOptions into base/ref variants and updating Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
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: 1
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/useQuery.ts (1)
1-24: Fix lint errors for Vue type imports
ComputedRef/Refare type-only and the linter also expects external modules before relative imports. Switching to a type import and placing it with the other externals resolves both errors.-import { QueryObserver } from '@tanstack/query-core' -import { useBaseQuery } from './useBaseQuery' +import { QueryObserver } from '@tanstack/query-core' +import type { ComputedRef, Ref } from 'vue' +import { useBaseQuery } from './useBaseQuery' … -import type { QueryClient } from './queryClient' -import { ComputedRef, Ref } from 'vue' +import type { QueryClient } from './queryClient'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/seven-apes-boil.md(1 hunks)packages/vue-query/src/queryOptions.ts(1 hunks)packages/vue-query/src/useQuery.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/vue-query/src/queryOptions.ts (2)
packages/query-core/src/types.ts (3)
DefaultError(47-51)QueryKey(53-61)DataTag(73-82)packages/vue-query/src/useQuery.ts (6)
DefinedInitialQueryOptions(132-137)DefinedInitialQueryOptionsRef(120-130)DefinedInitialQueryOptionsBase(109-118)UndefinedInitialQueryOptions(102-107)UndefinedInitialQueryOptionsRef(90-100)UndefinedInitialQueryOptionsBase(78-88)
packages/vue-query/src/useQuery.ts (2)
packages/query-core/src/types.ts (6)
DefaultError(47-51)QueryKey(53-61)QueryObserverOptions(312-441)Enabled(113-120)InitialDataFunction(167-167)NonUndefinedGuard(12-12)packages/vue-query/src/types.ts (4)
MaybeRefOrGetter(27-27)DeepUnwrapRef(48-56)MaybeRefDeep(29-37)ShallowOption(58-63)
🪛 ESLint
packages/vue-query/src/useQuery.ts
[error] 21-21: All imports in the declaration are only used as types. Use import type.
(@typescript-eslint/consistent-type-imports)
[error] 21-21: vue import should occur before import of ./useBaseQuery
(import/order)
| export function queryOptions< | ||
| TQueryFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = TQueryFnData, | ||
| TQueryKey extends QueryKey = QueryKey, | ||
| TOptions = DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, | ||
| TReturnQueryKey = { | ||
| queryKey: DataTag<TQueryKey, TQueryFnData, TError> | ||
| }, | ||
| >( | ||
| options: DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, | ||
| ): DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey> & { | ||
| queryKey: DataTag<TQueryKey, TQueryFnData, TError> | ||
| } | ||
| options: TOptions, | ||
| ): TOptions extends { value: unknown } | ||
| ? DefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & | ||
| TOptions | ||
| : DefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & | ||
| TOptions | ||
|
|
||
| export function queryOptions< | ||
| TQueryFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = TQueryFnData, | ||
| TQueryKey extends QueryKey = QueryKey, | ||
| TOptions = UndefinedInitialQueryOptions< | ||
| TQueryFnData, | ||
| TError, | ||
| TData, | ||
| TQueryKey | ||
| > | ||
| >( | ||
| options: UndefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, | ||
| ): UndefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey> & { | ||
| queryKey: DataTag<TQueryKey, TQueryFnData, TError> | ||
| } | ||
| options: TOptions, | ||
| ): TOptions extends { value: unknown } | ||
| ? UndefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & | ||
| TOptions | ||
| : UndefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & | ||
| TOptions |
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.
Overload selection is now broken—second signature is unreachable
Both overloads accept a free TOptions parameter with no constraint, so the first signature always matches. Calls such as queryOptions({ queryKey: ['key'] }) now resolve to the defined branch even though no initialData is provided, regressing the typing guarantees you’re trying to preserve. Constrain each overload’s TOptions to its intended option family (Defined vs. Undefined) so TypeScript can discriminate correctly.
export function queryOptions<
TQueryFnData = unknown,
TError = DefaultError,
TData = TQueryFnData,
TQueryKey extends QueryKey = QueryKey,
- TOptions = DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>,
+ TOptions extends DefinedInitialQueryOptions<
+ TQueryFnData,
+ TError,
+ TData,
+ TQueryKey
+ > = DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>,
>(options: TOptions): TOptions extends { value: unknown }
? DefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> &
TOptions
: DefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> &
TOptions
export function queryOptions<
TQueryFnData = unknown,
TError = DefaultError,
TData = TQueryFnData,
TQueryKey extends QueryKey = QueryKey,
- TOptions = UndefinedInitialQueryOptions<
+ TOptions extends UndefinedInitialQueryOptions<
TQueryFnData,
TError,
TData,
TQueryKey
- >
+ > = UndefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>,
>(options: TOptions): TOptions extends { value: unknown }
? UndefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> &
TOptions
: UndefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> &
TOptions📝 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 queryOptions< | |
| TQueryFnData = unknown, | |
| TError = DefaultError, | |
| TData = TQueryFnData, | |
| TQueryKey extends QueryKey = QueryKey, | |
| TOptions = DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, | |
| TReturnQueryKey = { | |
| queryKey: DataTag<TQueryKey, TQueryFnData, TError> | |
| }, | |
| >( | |
| options: DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, | |
| ): DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey> & { | |
| queryKey: DataTag<TQueryKey, TQueryFnData, TError> | |
| } | |
| options: TOptions, | |
| ): TOptions extends { value: unknown } | |
| ? DefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & | |
| TOptions | |
| : DefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & | |
| TOptions | |
| export function queryOptions< | |
| TQueryFnData = unknown, | |
| TError = DefaultError, | |
| TData = TQueryFnData, | |
| TQueryKey extends QueryKey = QueryKey, | |
| TOptions = UndefinedInitialQueryOptions< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryKey | |
| > | |
| >( | |
| options: UndefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, | |
| ): UndefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey> & { | |
| queryKey: DataTag<TQueryKey, TQueryFnData, TError> | |
| } | |
| options: TOptions, | |
| ): TOptions extends { value: unknown } | |
| ? UndefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & | |
| TOptions | |
| : UndefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & | |
| TOptions | |
| export function queryOptions< | |
| TQueryFnData = unknown, | |
| TError = DefaultError, | |
| TData = TQueryFnData, | |
| TQueryKey extends QueryKey = QueryKey, | |
| TOptions extends DefinedInitialQueryOptions< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryKey | |
| > = DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, | |
| TReturnQueryKey = { | |
| queryKey: DataTag<TQueryKey, TQueryFnData, TError> | |
| }, | |
| >( | |
| options: TOptions, | |
| ): TOptions extends { value: unknown } | |
| ? DefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & | |
| TOptions | |
| : DefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & | |
| TOptions | |
| export function queryOptions< | |
| TQueryFnData = unknown, | |
| TError = DefaultError, | |
| TData = TQueryFnData, | |
| TQueryKey extends QueryKey = QueryKey, | |
| TOptions extends UndefinedInitialQueryOptions< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryKey | |
| > = UndefinedInitialQueryOptions< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryKey | |
| > | |
| >( | |
| options: TOptions, | |
| ): TOptions extends { value: unknown } | |
| ? UndefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & | |
| TOptions | |
| : UndefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & | |
| TOptions |
🤖 Prompt for AI Agents
In packages/vue-query/src/queryOptions.ts around lines 11 to 45, the overloads
both declare a free generic TOptions so the first overload always matches and
the undefined-case becomes unreachable; constrain each overload’s TOptions
generic to the specific option family it represents (i.e. use TOptions extends
DefinedInitialQueryOptions<...> for the defined overload and TOptions extends
UndefinedInitialQueryOptions<...> for the undefined overload) so TypeScript can
discriminate between the two and restore correct return typing.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:sherif,test:knip,tes... |
❌ Failed | 1m 5s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
❌ Failed | 6s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-17 09:53:13 UTC
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: 1
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/useQuery.ts (1)
1-2: Fix ESLint errors: use import type for Vue symbols and order external before internalConvert Vue imports to type-only and move them above internal imports to satisfy consistent-type-imports and import/order.
As per coding guidelines
-import { QueryObserver } from '@tanstack/query-core' -import { useBaseQuery } from './useBaseQuery' +import { QueryObserver } from '@tanstack/query-core' +import type { ComputedRef, Ref } from 'vue' +import { useBaseQuery } from './useBaseQuery' @@ -import { ComputedRef, Ref } from 'vue' +// moved above and marked as type-onlyAlso applies to: 21-21
♻️ Duplicate comments (1)
packages/vue-query/src/queryOptions.ts (1)
16-26: Overload selection is broken; constrain TOptions and use the queryKey DataTag in the returnFirst overload always matches because TOptions is unconstrained; second overload becomes unreachable. Also, TReturnQueryKey is declared but unused. Constrain TOptions per family and intersect TReturnQueryKey into the return to preserve queryKey tagging.
export function queryOptions< TQueryFnData = unknown, TError = DefaultError, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey, - TOptions = DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, - TReturnQueryKey = { + TOptions extends DefinedInitialQueryOptions< + TQueryFnData, + TError, + TData, + TQueryKey + > = DefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, + TReturnQueryKey = { queryKey: DataTag<TQueryKey, TQueryFnData, TError> }, >( - options: TOptions, -): TOptions extends { value: unknown } - ? DefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & - TOptions - : DefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & - TOptions + options: TOptions, +): TOptions extends { value: unknown } + ? DefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & + TOptions & + TReturnQueryKey + : DefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & + TOptions & + TReturnQueryKey export function queryOptions< TQueryFnData = unknown, TError = DefaultError, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey, - TOptions = UndefinedInitialQueryOptions< + TOptions extends UndefinedInitialQueryOptions< TQueryFnData, TError, TData, TQueryKey - >, + > = UndefinedInitialQueryOptions<TQueryFnData, TError, TData, TQueryKey>, + TReturnQueryKey = { + queryKey: DataTag<TQueryKey, TQueryFnData, TError> + }, >( - options: TOptions, -): TOptions extends { value: unknown } - ? UndefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & - TOptions - : UndefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & - TOptions + options: TOptions, +): TOptions extends { value: unknown } + ? UndefinedInitialQueryOptionsRef<TQueryFnData, TError, TData, TQueryKey> & + TOptions & + TReturnQueryKey + : UndefinedInitialQueryOptionsBase<TQueryFnData, TError, TData, TQueryKey> & + TOptions & + TReturnQueryKeyAlso applies to: 33-45
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/vue-query/src/queryOptions.ts(1 hunks)packages/vue-query/src/useQuery.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/vue-query/src/useQuery.ts (2)
packages/query-core/src/types.ts (6)
DefaultError(47-51)QueryKey(53-61)QueryObserverOptions(312-441)Enabled(113-120)InitialDataFunction(167-167)NonUndefinedGuard(12-12)packages/vue-query/src/types.ts (4)
MaybeRefOrGetter(27-27)DeepUnwrapRef(48-56)MaybeRefDeep(29-37)ShallowOption(58-63)
packages/vue-query/src/queryOptions.ts (2)
packages/query-core/src/types.ts (3)
DefaultError(47-51)QueryKey(53-61)DataTag(73-82)packages/vue-query/src/useQuery.ts (6)
DefinedInitialQueryOptions(153-160)DefinedInitialQueryOptionsRef(134-151)DefinedInitialQueryOptionsBase(117-132)UndefinedInitialQueryOptions(108-115)UndefinedInitialQueryOptionsRef(96-106)UndefinedInitialQueryOptionsBase(78-94)
🪛 ESLint
packages/vue-query/src/useQuery.ts
[error] 21-21: All imports in the declaration are only used as types. Use import type.
(@typescript-eslint/consistent-type-imports)
[error] 21-21: vue import should occur before import of ./useBaseQuery
(import/order)
| export type UndefinedInitialQueryOptionsRef< | ||
| TQueryFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = TQueryFnData, | ||
| TQueryKey extends QueryKey = QueryKey, | ||
| > = UseQueryOptionsRef<TQueryFnData, TError, TData, TQueryFnData, TQueryKey> & { | ||
| initialData?: | ||
| | undefined | ||
| | InitialDataFunction<NonUndefinedGuard<TQueryFnData>> | ||
| | NonUndefinedGuard<TQueryFnData> | ||
| } |
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.
Reactive undefined-initial options: initialData should be nested under value (not top-level)
For Ref/ComputedRef cases, initialData belongs inside value, mirroring DefinedInitialQueryOptionsRef. Current shape incorrectly places it at the top level of the ref.
-export type UndefinedInitialQueryOptionsRef<
- TQueryFnData = unknown,
- TError = DefaultError,
- TData = TQueryFnData,
- TQueryKey extends QueryKey = QueryKey,
-> = UseQueryOptionsRef<TQueryFnData, TError, TData, TQueryFnData, TQueryKey> & {
- initialData?:
- | undefined
- | InitialDataFunction<NonUndefinedGuard<TQueryFnData>>
- | NonUndefinedGuard<TQueryFnData>
-}
+export type UndefinedInitialQueryOptionsRef<
+ TQueryFnData = unknown,
+ TError = DefaultError,
+ TData = TQueryFnData,
+ TQueryKey extends QueryKey = QueryKey,
+> = UseQueryOptionsRef<TQueryFnData, TError, TData, TQueryFnData, TQueryKey> & {
+ value: UseQueryOptionsRef<
+ TQueryFnData,
+ TError,
+ TData,
+ TQueryFnData,
+ TQueryKey
+ >['value'] & {
+ initialData?:
+ | undefined
+ | InitialDataFunction<NonUndefinedGuard<TQueryFnData>>
+ | NonUndefinedGuard<TQueryFnData>
+ }
+}📝 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 type UndefinedInitialQueryOptionsRef< | |
| TQueryFnData = unknown, | |
| TError = DefaultError, | |
| TData = TQueryFnData, | |
| TQueryKey extends QueryKey = QueryKey, | |
| > = UseQueryOptionsRef<TQueryFnData, TError, TData, TQueryFnData, TQueryKey> & { | |
| initialData?: | |
| | undefined | |
| | InitialDataFunction<NonUndefinedGuard<TQueryFnData>> | |
| | NonUndefinedGuard<TQueryFnData> | |
| } | |
| export type UndefinedInitialQueryOptionsRef< | |
| TQueryFnData = unknown, | |
| TError = DefaultError, | |
| TData = TQueryFnData, | |
| TQueryKey extends QueryKey = QueryKey, | |
| > = UseQueryOptionsRef<TQueryFnData, TError, TData, TQueryFnData, TQueryKey> & { | |
| value: UseQueryOptionsRef< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryFnData, | |
| TQueryKey | |
| >['value'] & { | |
| initialData?: | |
| | undefined | |
| | InitialDataFunction<NonUndefinedGuard<TQueryFnData>> | |
| | NonUndefinedGuard<TQueryFnData> | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/vue-query/src/useQuery.ts around lines 96–106, the reactive
Ref/ComputedRef variant places initialData at the top-level of the ref type;
move initialData inside the ref's value property to match
DefinedInitialQueryOptionsRef. Update the type so instead of adding
initialData?: ... at the top level, add value?: { initialData?: undefined |
InitialDataFunction<NonUndefinedGuard<TQueryFnData>> |
NonUndefinedGuard<TQueryFnData> } (or equivalent nested shape used by
DefinedInitialQueryOptionsRef) so reactive refs carry initialData under value.
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.
Yeah I guess this is needed. Forgot about it
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.
@Serpentarius13, sounds good! Once you've updated the type to nest initialData under value, the reactive ref structure will be consistent with DefinedInitialQueryOptionsRef. This ensures that when users pass a ref or computed ref with optional initial data, TypeScript will correctly expect the shape to be { value: { initialData?: ... } }.
🎯 Changes
I changed UseQueryOptions and its derivatives UndefinedInitialQueryOptions / DefinedInitialQueryOptions to have base non-ref cases to later differentiate them in queryOptions function. I don't think I can foresee possible implications of this as I am a novice, but I'm thinking nothing bad will happen from splitting the types like that
✅ Checklist
pnpm run test:pr.🚀 Release Impact
I dont really know how this works, but I've done it I guess
Summary by CodeRabbit