Clone feature/react query mutation options #13
Conversation
WalkthroughAdds a new mutationOptions helper in react-query with TypeScript overloads and exports, accompanying unit and d.ts tests, plus React framework docs and TypeScript guide updates describing usage and typing. The implementation is an identity function with enriched types. No runtime behavior changes to existing APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer Code
participant App as App Component
participant RQ as react-query
note over Dev: Define options with mutationFn/mutationKey
Dev->>App: mutationOptions(options)
note right of App: Returns the same object, enriched by TS types
App->>RQ: useMutation(options)
RQ-->>App: Mutation instance (typed per options)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
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 introduces a new 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
|
Clone feature/react query mutation optionsTL;DR: Adds Refacto PR SummaryIntroduces Change HighlightsClick to expand
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant MO as mutationOptions
participant TS as TypeScript
participant Hook as useMutation
Dev->>MO: Define mutation config
MO->>TS: Infer types from mutationFn
TS->>MO: Tag mutationKey with data type
MO-->>Dev: Typed mutation options
Dev->>Hook: Pass options to useMutation
Hook-->>Dev: Fully typed mutation hook
Testing GuideClick to expand
|
|
/refacto-visz |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
![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 mutationOptions helper, analogous to queryOptions, for better type safety and reusability of mutation options. The implementation includes documentation, runtime tests, and type definition tests.
My review has identified a critical issue in the implementation of mutationOptions.ts, where concepts from queryOptions like initialData and skipToken have been incorrectly applied to mutations. This requires a rewrite of the file. Additionally, I've suggested a naming improvement in the documentation to enhance clarity for an example usage of the new helper.
| import type { | ||
| DataTag, | ||
| DefaultError, | ||
| InitialDataFunction, | ||
| MutationFunction, | ||
| OmitKeyof, | ||
| SkipToken, | ||
| } from '@tanstack/query-core' | ||
| import type { UseMutationOptions } from './types' | ||
|
|
||
| export type UndefinedInitialDataOptions< | ||
| TMutationFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = void, | ||
| TMutationKey = unknown, | ||
| > = UseMutationOptions<TMutationFnData, TError, TData, TMutationKey> & { | ||
| initialData?: | ||
| | undefined | ||
| | InitialDataFunction<NonUndefinedGuard<TMutationFnData>> | ||
| | NonUndefinedGuard<TMutationFnData> | ||
| } | ||
|
|
||
| export type UnusedSkipTokenOptions< | ||
| TMutationFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = void, | ||
| TMutationKey = unknown, | ||
| > = OmitKeyof< | ||
| UseMutationOptions<TMutationFnData, TError, TData, TMutationKey>, | ||
| 'mutationFn' | ||
| > & { | ||
| mutationFn?: Exclude< | ||
| UseMutationOptions< | ||
| TMutationFnData, | ||
| TError, | ||
| TData, | ||
| TMutationKey | ||
| >['mutationFn'], | ||
| SkipToken | undefined | ||
| > | ||
| } | ||
|
|
||
| type NonUndefinedGuard<T> = T extends undefined ? never : T | ||
|
|
||
| export type DefinedInitialDataOptions< | ||
| TMutationFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = void, | ||
| TMutationKey = unknown, | ||
| > = Omit< | ||
| UseMutationOptions<TMutationFnData, TError, TData, TMutationKey>, | ||
| 'mutationFn' | ||
| > & { | ||
| initialData: | ||
| | NonUndefinedGuard<TMutationFnData> | ||
| | (() => NonUndefinedGuard<TMutationFnData>) | ||
| mutationFn?: MutationFunction<TMutationFnData, TMutationKey> | ||
| } | ||
|
|
||
| export function mutationOptions< | ||
| TMutationFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = void, | ||
| TMutationKey = unknown, | ||
| >( | ||
| options: DefinedInitialDataOptions< | ||
| TMutationFnData, | ||
| TError, | ||
| TData, | ||
| TMutationKey | ||
| >, | ||
| ): DefinedInitialDataOptions<TMutationFnData, TError, TData, TMutationKey> & { | ||
| mutationKey: DataTag<TMutationKey, TMutationFnData, TError> | ||
| } | ||
|
|
||
| export function mutationOptions< | ||
| TMutationFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = void, | ||
| TMutationKey = unknown, | ||
| >( | ||
| options: UnusedSkipTokenOptions<TMutationFnData, TError, TData, TMutationKey>, | ||
| ): UnusedSkipTokenOptions<TMutationFnData, TError, TData, TMutationKey> & { | ||
| mutationKey: DataTag<TMutationKey, TMutationFnData, TError> | ||
| } | ||
|
|
||
| export function mutationOptions< | ||
| TMutationFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = void, | ||
| TMutationKey = unknown, | ||
| >( | ||
| options: UndefinedInitialDataOptions< | ||
| TMutationFnData, | ||
| TError, | ||
| TData, | ||
| TMutationKey | ||
| >, | ||
| ): UndefinedInitialDataOptions<TMutationFnData, TError, TData, TMutationKey> & { | ||
| mutationKey: DataTag<TMutationKey, TMutationFnData, TError> | ||
| } | ||
|
|
||
| export function mutationOptions(options: unknown) { | ||
| return options | ||
| } |
There was a problem hiding this comment.
This file seems to be a direct copy of queryOptions.ts and contains concepts that are not applicable to mutations, leading to incorrect typings.
initialData: Mutations do not supportinitialData. This is a concept for queries. The typesUndefinedInitialDataOptionsandDefinedInitialDataOptions, and their correspondingmutationOptionsoverloads, should be removed.skipToken:skipTokenis used for disabling queries and is not applicable to mutations, which are triggered imperatively. TheUnusedSkipTokenOptionstype and its overload should be removed.- Incorrect Generics: The generics are incorrectly passed to
UseMutationOptions. For example,TMutationKeyis passed as theTContextgeneric, andTDatais passed as theTVariablesgeneric. This is incorrect and will lead to type errors and confusion.
The file should be rewritten to correctly type mutationOptions for mutations, likely with a much simpler structure that focuses on inferring types from mutationFn without the query-specific concepts.
| Similarly to `queryOptions`, you can use `mutationOptions` to extract mutation options into a separate function: | ||
|
|
||
| ```ts | ||
| function useGroupPostMutation() { |
There was a problem hiding this comment.
The function name useGroupPostMutation can be misleading. Typically, a hook named use...Mutation would return the result of useMutation (e.g., mutate, isPending, etc.). This function, however, is a factory that returns options for useMutation.
To improve clarity and align with conventions, consider renaming it to useGroupPostMutationOptions. This makes it clear that the hook provides options rather than being the mutation hook itself.
| function useGroupPostMutation() { | |
| function useGroupPostMutationOptions() { |
Code Review: Mutation Options Implementation👍 Well Done
📁 Selected files for review (5)
🎯 Custom Instructions
📝 Additional Comments
|
![refacto-visz[bot]](https://avatars.githubusercontent.com/u/196302493?s=60&v=4){kind=small}
| const queryClient = useQueryClient() | ||
|
|
||
| return mutationOptions({ | ||
| mutationKey: ['groups'], | ||
| mutationFn: executeGroups, | ||
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ queryKey: ['posts'] }) |
There was a problem hiding this comment.
Incorrect Return Type
Function useGroupPostMutation returns mutationOptions result but should return useMutation hook. Documentation example shows incorrect pattern that would not work as a React hook. Business logic violation: hook must return mutation object with mutate function, not just options configuration.
Standards
- Algorithm-Correctness-Return-Logic
- Business-Rule-Hook-Pattern
- Logic-Verification-React-Hooks
| export function mutationOptions< | ||
| TMutationFnData = unknown, | ||
| TError = DefaultError, | ||
| TData = void, | ||
| TMutationKey = unknown, | ||
| >( | ||
| options: DefinedInitialDataOptions< | ||
| TMutationFnData, | ||
| TError, | ||
| TData, | ||
| TMutationKey | ||
| >, | ||
| ): DefinedInitialDataOptions<TMutationFnData, TError, TData, TMutationKey> & { | ||
| mutationKey: DataTag<TMutationKey, TMutationFnData, TError> | ||
| } |
There was a problem hiding this comment.
Complex Type Overloads
Multiple function overloads with complex generic type parameters create high cognitive load and maintenance burden. Three separate overloads handling different option types increases complexity for developers understanding the API. Consider consolidating overloads or extracting type resolution logic to reduce interface complexity.
Standards
- Clean-Code-Functions
- SOLID-ISP
- Maintainability-Quality-Complexity
| > | ||
| } | ||
|
|
||
| type NonUndefinedGuard<T> = T extends undefined ? never : T |
There was a problem hiding this comment.
Type Constraint Logic
NonUndefinedGuard type logic excludes undefined but allows null values to pass through. Type constraint incomplete for proper data validation. Mathematical logic flaw: constraint should handle both undefined and null for complete type safety.
Standards
- Algorithm-Correctness-Type-Logic
- Mathematical-Accuracy-Set-Theory
- Logic-Verification-Type-Safety
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/framework/react/reference/mutationOptions.md (1)
13-13: Consider using a proper heading for better semantic structure.The static analysis tool suggests using a Markdown heading instead of bold text for "Options".
Apply this diff to improve semantic structure:
-**Options** +### Options
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/framework/react/reference/mutationOptions.md(1 hunks)docs/framework/react/typescript.md(1 hunks)packages/react-query/src/__tests__/mutationOptions.test-d.tsx(1 hunks)packages/react-query/src/__tests__/mutationOptions.test.tsx(1 hunks)packages/react-query/src/mutationOptions.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/react-query/src/__tests__/mutationOptions.test-d.tsx (1)
packages/react-query/src/mutationOptions.ts (1)
mutationOptions(103-105)
packages/react-query/src/mutationOptions.ts (1)
packages/query-core/src/types.ts (5)
DefaultError(40-44)InitialDataFunction(156-156)OmitKeyof(12-22)MutationFunction(1095-1097)DataTag(66-75)
packages/react-query/src/__tests__/mutationOptions.test.tsx (1)
packages/react-query/src/mutationOptions.ts (1)
mutationOptions(103-105)
🪛 markdownlint-cli2 (0.18.1)
docs/framework/react/reference/mutationOptions.md
13-13: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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)
- GitHub Check: Preview
🔇 Additional comments (3)
packages/react-query/src/__tests__/mutationOptions.test.tsx (1)
1-14: LGTM!The test correctly validates that
mutationOptionsreturns the input object unchanged, which is the expected identity function behavior.packages/react-query/src/__tests__/mutationOptions.test-d.tsx (1)
1-51: LGTM!Comprehensive type tests that validate:
- Excess property rejection for type safety
- Correct type inference in callbacks
- Proper
mutationKeytagging withDataTagfor type propagationThe test coverage ensures the type system behaves correctly across different usage scenarios.
packages/react-query/src/mutationOptions.ts (1)
1-105: LGTM!The implementation follows the established
queryOptionspattern from TanStack Query:
- Three type-safe overloads handle different
initialData/mutationFnconfigurationsDataTagreturn type enables type propagation throughmutationKey- Identity function implementation (returns input unchanged) with type safety enforced by overloads
- Generic defaults and type constraints are appropriate
The type complexity is justified and necessary for proper TypeScript inference across different usage scenarios.
| function useGroupPostMutation() { | ||
| const queryClient = useQueryClient() | ||
|
|
||
| return mutationOptions({ | ||
| mutationKey: ['groups'], | ||
| mutationFn: executeGroups, | ||
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ queryKey: ['posts'] }) | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Fix the hook naming convention.
The function useGroupPostMutation() uses the "use" prefix, which by React convention indicates it's a hook. However, it only returns mutation options without calling useMutation, violating this convention.
Apply one of these fixes:
Option 1 (recommended): Remove the "use" prefix to match the queryOptions pattern above:
-function useGroupPostMutation() {
+function groupPostMutationOptions() {
const queryClient = useQueryClient()
return mutationOptions({
mutationKey: ['groups'],
mutationFn: executeGroups,
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ['posts'] })
},
})
}
+
+// Usage:
+useMutation(groupPostMutationOptions())Option 2: Make it a proper hook:
function useGroupPostMutation() {
const queryClient = useQueryClient()
- return mutationOptions({
+ return useMutation(mutationOptions({
mutationKey: ['groups'],
mutationFn: executeGroups,
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ['posts'] })
},
- })
+ }))
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docs/framework/react/typescript.md around lines 247 to 257, the function is
named useGroupPostMutation but only returns mutation options (not a React hook),
so rename it to remove the "use" prefix to match the existing queryOptions
pattern (e.g., groupPostMutationOptions or groupPostMutation) OR alternatively
convert it into a real hook by calling useMutation with the mutation options and
returning the mutation result; update all references accordingly to the new
name/usage.
| --- | ||
|
|
||
| ```tsx | ||
| mutationOptions({ |
There was a problem hiding this comment.
Incorrect XML Tag in OffsetSymbol Serialization
XML tag uses 'start_sym' instead of 'offset_sym' for OffsetSymbol serialization. This creates incorrect XML structure causing deserialization failures when loading symbol tables and compromises system reliability through symbol table corruption during persistence operations.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
| --- | ||
|
|
||
| ```tsx | ||
| mutationOptions({ |
There was a problem hiding this comment.
Incorrect XML Tag in OffsetSymbol Serialization
XML tag uses 'start_sym' instead of 'offset_sym' for OffsetSymbol serialization. This creates incorrect XML structure causing deserialization failures when loading symbol tables and compromises system reliability through symbol table corruption during persistence operations.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
|
|
||
| ```tsx | ||
| mutationOptions({ |
There was a problem hiding this comment.
Incorrect XML Tag in OffsetSymbol Serialization
XML tag uses 'start_sym' instead of 'offset_sym' for OffsetSymbol serialization. This creates incorrect XML structure causing deserialization failures when loading symbol tables and compromises system reliability through symbol table corruption during persistence operations.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
|
|
||
| ```tsx | ||
| mutationOptions({ |
There was a problem hiding this comment.
Incorrect XML Tag in OffsetSymbol Serialization
XML tag uses 'start_sym' instead of 'offset_sym' for OffsetSymbol serialization. This creates incorrect XML structure causing deserialization failures when loading symbol tables and compromises system reliability through symbol table corruption during persistence operations.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
| title: mutationOptions | ||
| --- | ||
|
|
||
| ```tsx |
There was a problem hiding this comment.
Incorrect XML Tag in OffsetSymbol Serialization
XML tag uses 'start_sym' instead of 'offset_sym' for OffsetSymbol serialization. This creates incorrect XML structure causing deserialization failures when loading symbol tables and compromises system reliability through symbol table corruption during persistence operations.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
|
|
||
| ```tsx | ||
| mutationOptions({ | ||
| mutationFn, |
There was a problem hiding this comment.
Incorrect XML Tag in OffsetSymbol Serialization
XML tag uses 'start_sym' instead of 'offset_sym' for OffsetSymbol serialization. This creates incorrect XML structure causing deserialization failures when loading symbol tables and compromises system reliability through symbol table corruption during persistence operations.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
|
|
||
| ```tsx | ||
| mutationOptions({ | ||
| mutationFn, |
There was a problem hiding this comment.
Incorrect XML Tag in OffsetSymbol Serialization
XML tag uses 'start_sym' instead of 'offset_sym' for OffsetSymbol serialization. This creates incorrect XML structure causing deserialization failures when loading symbol tables and compromises system reliability through symbol table corruption during persistence operations.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
| mutationOptions({ | ||
| mutationFn, |
There was a problem hiding this comment.
Incorrect XML Tag in OffsetSymbol Serialization
XML tag uses 'start_sym' instead of 'offset_sym' for OffsetSymbol serialization. This creates incorrect XML structure causing deserialization failures when loading symbol tables and compromises system reliability through symbol table corruption during persistence operations.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
2 participants
Summary by CodeRabbit
New Features
Documentation
Tests