-
Notifications
You must be signed in to change notification settings - Fork 0
Clone fix/preserve set query data type inference #14
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?
Changes from all commits
775d072
53a2d79
b5c6dde
7c34847
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,6 +113,19 @@ describe('setQueryData', () => { | |
|
|
||
| expectTypeOf(data).toEqualTypeOf<string | undefined>() | ||
| }) | ||
|
|
||
| it('should preserve updater parameter type inference when used in functions with explicit return types', () => { | ||
| const queryKey = ['key'] as DataTag<Array<string>, number> | ||
| const queryClient = new QueryClient() | ||
|
|
||
| // Simulate usage inside a function with explicit return type | ||
| // The outer function returns 'unknown' but this shouldn't affect the updater's type inference | ||
| ;(() => | ||
| queryClient.setQueryData(queryKey, (data) => { | ||
| expectTypeOf(data).toEqualTypeOf<number | undefined>() | ||
| return data | ||
| })) satisfies () => unknown | ||
| }) | ||
| }) | ||
|
|
||
| describe('getQueryState', () => { | ||
|
|
@@ -590,3 +603,28 @@ describe('resetQueries', () => { | |
| }) | ||
| }) | ||
| }) | ||
| type SuccessCallback = () => unknown | ||
| it('should infer types correctly with expression body arrow functions', () => { | ||
| const queryKey = ['key'] as DataTag<Array<string>, number> | ||
| const queryClient = new QueryClient() | ||
|
|
||
| // @ts-expect-error | ||
| const callbackTest: SuccessCallback = () => | ||
| queryClient.setQueryData(queryKey, (data) => { | ||
| expectTypeOf(data).toEqualTypeOf<number | undefined>() | ||
| return data | ||
| }) | ||
| }) | ||
|
|
||
| it('should infer types correctly with block body arrow functions', () => { | ||
| const queryKey = ['key'] as DataTag<Array<string>, number> | ||
| const queryClient = new QueryClient() | ||
|
|
||
| // @ts-expect-error | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| const callbackTest2: SuccessCallback = () => { | ||
| queryClient.setQueryData(queryKey, (data) => { | ||
| expectTypeOf(data).toEqualTypeOf<number | undefined>() | ||
| return data | ||
| }) | ||
| } | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -185,7 +185,7 @@ export class QueryClient { | |
| NoInfer<TInferredQueryFnData> | undefined | ||
| >, | ||
| options?: SetDataOptions, | ||
| ): TInferredQueryFnData | undefined { | ||
| ): NoInfer<TInferredQueryFnData> | undefined { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Revert the explicit NoInfer return type on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type Safety Pattern EnhancementNoInfer wrapper prevents TypeScript from widening generic types in complex inference scenarios, maintaining type safety when setQueryData is used in functions with explicit return types. This ensures consistent type behavior across different usage patterns and prevents runtime type mismatches. Standards
|
||
| const defaultedOptions = this.defaultQueryOptions< | ||
| any, | ||
| any, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -114,17 +114,17 @@ export class QueryClient extends QC { | |||||
| NoInfer<TInferredQueryFnData> | undefined | ||||||
| >, | ||||||
| options?: MaybeRefDeep<SetDataOptions>, | ||||||
| ): TInferredQueryFnData | undefined | ||||||
| ): NoInfer<TInferredQueryFnData> | undefined | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Add a trailing semicolon to the overload signature to make it a proper TypeScript overload declaration. [possible bug]
Suggested change
Why Change? ⭐In TypeScript, overload signatures (declarations without an implementation) must be terminated with a semicolon. |
||||||
| setQueryData<TQueryFnData, TData = NoUnknown<TQueryFnData>>( | ||||||
| queryKey: MaybeRefDeep<QueryKey>, | ||||||
| updater: Updater<NoInfer<TData> | undefined, NoInfer<TData> | undefined>, | ||||||
| options?: MaybeRefDeep<SetDataOptions>, | ||||||
| ): TData | undefined | ||||||
| ): NoInfer<TData> | undefined | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Add a trailing semicolon to the second overload signature so both overloads are declared consistently and validly. [possible bug]
Suggested change
Why Change? ⭐This is the second overload signature in the same overload group. As with the first suggestion, |
||||||
| setQueryData<TData>( | ||||||
| queryKey: MaybeRefDeep<QueryKey>, | ||||||
| updater: Updater<TData | undefined, TData | undefined>, | ||||||
| options: MaybeRefDeep<SetDataOptions> = {}, | ||||||
| ): TData | undefined { | ||||||
| ): NoInfer<TData> | undefined { | ||||||
|
Comment on lines
+117
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Vue Consistency PatternVue client maintains type inference consistency with core client through NoInfer wrapper application. This prevents type widening in Vue-specific contexts while preserving generic type relationships, ensuring consistent developer experience across both implementations. Standards
|
||||||
| return super.setQueryData( | ||||||
| cloneDeepUnref(queryKey), | ||||||
| updater, | ||||||
|
|
||||||
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.
With the
NoInferfix onsetQueryData, this assignment should be type-correct, and the type ofdatawithin the updater is correctly inferred. This means there should be no type error on the following line.The
@ts-expect-errordirective is likely now causing an "Unused @ts-expect-error" warning from the TypeScript compiler and should be removed.