Clone fix/preserve set query data type inference #14
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Summary of ChangesHello @visz11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a type inference issue within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughUpdates typings for QueryClient.setQueryData in core and Vue packages to use NoInfer in return types. Adds new type-level tests validating updater parameter inference within functions with explicit return types and different arrow bodies. No runtime behavior changes are introduced. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧬 Code graph analysis (4)packages/query-core/src/__tests__/queryClient.test-d.tsx (1)
packages/vue-query/src/__tests__/queryClient.test-d.ts (1)
packages/vue-query/src/queryClient.ts (3)
packages/query-core/src/queryClient.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
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 |
codeant-ai
bot
added
the
size:M
|
/refacto-visz |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a fix to preserve type inference for setQueryData's updater function, which is a great improvement for type safety. The changes in queryClient.ts for both query-core and vue-query correctly use NoInfer to prevent the contextual return type from incorrectly influencing type deduction.
The added tests effectively cover the scenarios this fix addresses. However, I've noticed a couple of @ts-expect-error directives in the new tests for query-core that seem to be incorrect now that the fix is in place. My review includes suggestions to remove them. Overall, this is a solid fix.
| const queryKey = ['key'] as DataTag<Array<string>, number> | ||
| const queryClient = new QueryClient() | ||
|
|
||
| // @ts-expect-error |
There was a problem hiding this comment.
With the NoInfer fix on setQueryData, this assignment should be type-correct, and the type of data within the updater is correctly inferred. This means there should be no type error on the following line.
The @ts-expect-error directive is likely now causing an "Unused @ts-expect-error" warning from the TypeScript compiler and should be removed.
| const queryKey = ['key'] as DataTag<Array<string>, number> | ||
| const queryClient = new QueryClient() | ||
|
|
||
| // @ts-expect-error |
There was a problem hiding this comment.
Similar to the test above, this @ts-expect-error directive should be removed. The fix makes the assignment valid, so a type error is not expected.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| >, | ||
| options?: MaybeRefDeep<SetDataOptions>, | ||
| ): TInferredQueryFnData | undefined | ||
| ): NoInfer<TInferredQueryFnData> | undefined |
There was a problem hiding this comment.
Suggestion: Add a trailing semicolon to the overload signature to make it a proper TypeScript overload declaration. [possible bug]
| ): NoInfer<TInferredQueryFnData> | undefined | |
| ): NoInfer<TInferredQueryFnData> | undefined; |
Why Change? ⭐
In TypeScript, overload signatures (declarations without an implementation) must be terminated with a semicolon.
The snippet being changed is an overload signature that precedes the implementation signature (which has a body).
Adding a trailing semicolon corrects the syntax without changing any runtime behavior. It does not reference any
identifiers outside the shown context and is consistent with the surrounding overload patterns in the file.
Assumptions: the next signature in the file is the implementation (has a function body), and the TypeScript
compiler expects overload terminators. This is a purely syntactic fix and will not introduce bugs.
| updater: Updater<NoInfer<TData> | undefined, NoInfer<TData> | undefined>, | ||
| options?: MaybeRefDeep<SetDataOptions>, | ||
| ): TData | undefined | ||
| ): NoInfer<TData> | undefined |
There was a problem hiding this comment.
Suggestion: Add a trailing semicolon to the second overload signature so both overloads are declared consistently and validly. [possible bug]
| ): NoInfer<TData> | undefined | |
| ): NoInfer<TData> | undefined; |
Why Change? ⭐
This is the second overload signature in the same overload group. As with the first suggestion,
TypeScript overload signatures must end with semicolons. Adding the semicolon here fixes syntax,
keeps overload declarations consistent, and does not alter runtime semantics. The change purely
affects declaration syntax and relies only on surrounding code already present in the file (the
implementation signature with a body follows). No new identifiers or behavior are introduced.
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Pull Request Feedback 🔍
|
| >, | ||
| options?: SetDataOptions, | ||
| ): TInferredQueryFnData | undefined { | ||
| ): NoInfer<TInferredQueryFnData> | undefined { |
There was a problem hiding this comment.
Suggestion: Revert the explicit NoInfer return type on setQueryData back to the original inferred return type to avoid surprising inference changes for direct-value callers; alternatively, if keeping NoInfer here, apply NoInfer consistently to the related setQueriesData return type — ensure updater inference and consumers' expectations remain stable. [possible issue]
Code Review: TypeScript Type Safety Enhancement👍 Well Done
📁 Selected files for review (4)
🎯 Custom Instructions
📝 Additional Comments
|
![refacto-visz[bot]](https://avatars.githubusercontent.com/u/196302493?s=60&v=4){kind=small}
| >, | ||
| options?: SetDataOptions, | ||
| ): TInferredQueryFnData | undefined { | ||
| ): NoInfer<TInferredQueryFnData> | undefined { |
There was a problem hiding this comment.
Type Safety Pattern Enhancement
NoInfer 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
- Clean-Code-Type-Safety
- SOLID-OCP
- Maintainability-Quality-Consistency
|
CodeAnt AI finished reviewing your PR. |
| ): NoInfer<TInferredQueryFnData> | undefined | ||
| setQueryData<TQueryFnData, TData = NoUnknown<TQueryFnData>>( | ||
| queryKey: MaybeRefDeep<QueryKey>, | ||
| updater: Updater<NoInfer<TData> | undefined, NoInfer<TData> | undefined>, | ||
| options?: MaybeRefDeep<SetDataOptions>, | ||
| ): TData | undefined | ||
| ): NoInfer<TData> | undefined | ||
| setQueryData<TData>( | ||
| queryKey: MaybeRefDeep<QueryKey>, | ||
| updater: Updater<TData | undefined, TData | undefined>, | ||
| options: MaybeRefDeep<SetDataOptions> = {}, | ||
| ): TData | undefined { | ||
| ): NoInfer<TData> | undefined { |
There was a problem hiding this comment.
Vue Consistency Pattern
Vue 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
- Clean-Code-Consistency
- SOLID-LSP
- Maintainability-Quality-Interface-Consistency
2 participants
CodeAnt-AI Description
Fix setQueryData type inference so updater callback keeps correct types
What Changed
Impact
✅ Fewer TypeScript errors when updater is used inside functions✅ Better editor completions for setQueryData updater callbacks✅ Consistent updater types in both core and Vue clients💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit