feat(react-query): add mutationOptions #4
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis update introduces comprehensive support for a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CustomHook
participant mutationOptions
participant useMutation
User->>CustomHook: Call custom hook (e.g., useGroupPostMutation)
CustomHook->>mutationOptions: Prepare mutation options object
mutationOptions-->>CustomHook: Return options (typed and tagged)
CustomHook->>useMutation: Pass options to useMutation
useMutation-->>CustomHook: Return mutation result
CustomHook-->>User: Provide mutation utilities
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/framework/react/reference/mutationOptions.md (1)
13-15: Consider using a proper markdown heading instead of emphasis.Use a markdown heading (e.g.,
## Options) instead of bold text (**Options**) for better document structure and consistency with other documentation files.-**Options** +## Options🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
packages/react-query/src/__tests__/mutationOptions.test-d.tsx (2)
6-13: Suppress ESLint warning about missing assertionsThis test case correctly validates that the TypeScript compiler rejects excess properties, but there's an ESLint warning about the test having no assertions (vitest/expect-expect). While the test is valid (using
@ts-expect-erroras a compile-time assertion), you should suppress the warning.it('should not allow excess properties', () => { + // eslint-disable-next-line vitest/expect-expect return mutationOptions({ mutationFn: () => Promise.resolve(5), mutationKey: ['key'], // @ts-expect-error this is a good error, because onMutates does not exist! onMutates: 1000, }) })🧰 Tools
🪛 GitHub Actions: pr
[warning] 6-6: ESLint warning: Test has no assertions (vitest/expect-expect).
1-51: Consider adding more comprehensive type testsWhile the current tests cover basic type inference scenarios, consider adding tests for:
- Variable argument types in
mutationFn(e.g., testing that a mutation function's parameter types are correctly inferred)- Complex nested types in the mutation result
- Type behavior with generics
- Error type inference (if applicable)
These would strengthen the type-checking coverage and ensure robust type safety for users.
🧰 Tools
🪛 GitHub Actions: pr
[warning] 6-6: ESLint warning: Test has no assertions (vitest/expect-expect).
📜 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/__tests__/mutationOptions.test.tsx (1)
packages/react-query/src/mutationOptions.ts (1)
mutationOptions(103-105)
packages/react-query/src/mutationOptions.ts (2)
packages/query-core/src/types.ts (5)
DefaultError(40-44)InitialDataFunction(156-156)OmitKeyof(12-22)MutationFunction(1095-1097)DataTag(66-75)packages/angular-query-experimental/src/types.ts (1)
NonUndefinedGuard(315-315)
🪛 markdownlint-cli2 (0.17.2)
docs/framework/react/reference/mutationOptions.md
13-13: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
🪛 GitHub Actions: pr
packages/react-query/src/__tests__/mutationOptions.test-d.tsx
[warning] 6-6: ESLint warning: Test has no assertions (vitest/expect-expect).
🔇 Additional comments (16)
packages/react-query/src/mutationOptions.ts (7)
1-10: Imports look good and match the functionality.The imports include all necessary types from the query-core package and internal types needed for the implementation.
11-21: Well-structured type definition for UndefinedInitialDataOptions.The type correctly extends UseMutationOptions and properly defines initialData as optional, allowing either undefined values or a function returning non-undefined data.
23-41: Properly defined UnusedSkipTokenOptions type.This type correctly omits the mutationFn from UseMutationOptions and redefines it as optional, excluding SkipToken and undefined. The use of OmitKeyof is appropriate here for precise type control.
43-43: NonUndefinedGuard utility type is properly implemented.This utility type correctly excludes undefined from a generic type T, which is essential for enforcing the presence of values in the typed options.
45-58: DefinedInitialDataOptions type ensures initialData is defined.The type properly ensures that initialData is always defined (non-undefined) either directly or through a function, while making mutationFn optional.
60-74: Function overloads provide proper type inference.The three overloads for mutationOptions correctly handle different option types and augment them with a typed mutationKey. This enables TypeScript to correctly infer types based on the options provided.
Also applies to: 76-85, 87-101
103-105: Implementation is minimal and correct.The function simply returns the input options without modification, serving as a type utility rather than a runtime transformer. This matches the pattern used in queryOptions and is sufficient for providing type inference benefits.
packages/react-query/src/__tests__/mutationOptions.test.tsx (2)
1-4: Test imports are appropriate.The necessary testing utilities and the function to be tested are properly imported.
5-14: Test correctly verifies the function's runtime behavior.The test effectively confirms that mutationOptions returns the exact same object that was passed to it without any modifications, which is the expected behavior for this type utility function.
docs/framework/react/reference/mutationOptions.md (1)
1-12: Documentation frontmatter and code example look good.The documentation properly defines the ID and title, and shows a clear code example of the mutationOptions function signature.
docs/framework/react/typescript.md (1)
242-259: Great addition of Typing Mutation Options section.This new section effectively explains how to use mutationOptions to extract mutation options into a separate function, similar to the existing queryOptions. The example is clear and demonstrates a practical use case including handling the mutationKey, mutationFn, and callbacks.
packages/react-query/src/__tests__/mutationOptions.test-d.tsx (5)
1-4: LGTM!The imports look good. The file correctly imports testing utilities from
vitest, thedataTagSymbolfrom the core package, and themutationOptionsfunction being tested.
15-23: LGTM! Well-implemented type inference testThis test successfully verifies that the TypeScript compiler correctly infers the type of the
dataparameter in theonSuccesscallback based on the return type ofmutationFn. This ensures type safety when working with mutation results.
25-32: LGTM! Excellent type tagging testThe test correctly verifies that the
mutationKeyis tagged with the result type of the mutation function. This is crucial for maintaining type safety throughout the query system.
34-40: LGTM! Good fallback type testThis test correctly verifies the fallback behavior when no
mutationFnis provided - themutationKeyshould be tagged withunknownsince there's no way to infer the data type.
42-51: LGTM! Properly testing type preservation with callbacksThis test ensures that adding an
onSuccesscallback doesn't affect the type tagging of themutationKey. This is important for consistent type inference throughout the library.
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
![refacto-test[bot]](https://avatars.githubusercontent.com/u/196302493?s=60&v=4){kind=small}
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
|
/refacto-test |
2 similar comments
|
/refacto-test |
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: mutationOptions Implementation👍 Well Done
📌 Files Processed
📝 Additional Comments
|
| export function mutationOptions(options: unknown) { | ||
| return options |
There was a problem hiding this comment.
Inconsistent Type Handling
Implementation returns options unmodified but type signatures promise enhanced return with DataTag. Runtime behavior doesn't match type contract, causing potential type inconsistencies.
| export function mutationOptions(options: unknown) { | |
| return options | |
| export function mutationOptions(options: unknown) { | |
| return { ...options, mutationKey: options.mutationKey } |
Standards
- ISO-IEC-25010-Functional-Correctness-Appropriateness
- DbC-Type-Contract
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: MutationOptions Implementation👍 Well Done
📌 Files Processed
📝 Additional Comments
|
| export function mutationOptions(options: unknown) { | ||
| return options | ||
| } |
There was a problem hiding this comment.
Implementation Duplication Risk
The implementation simply returns the input without modification, which could lead to confusion about its purpose. This function appears to be primarily for type inference rather than runtime behavior, which should be clarified.
Standards
- Clean-Code-Function-Purpose
- Clean-Code-Clarity
|
/refacto-test-arvi |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: mutationOptions Implementation👍 Well Done
📌 Files Processed
📝 Additional Comments
|
|
No review comments were generated during this review iteration of the PR. |
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: mutationOptions Implementation👍 Well Done
📌 Files Processed
📝 Additional Comments
|
| 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> | ||
| } |
There was a problem hiding this comment.
Redundant Function Overloads
The implementation has three function overloads but the actual implementation at line 103 simply returns the options object unchanged. Since all overloads return the same type structure with just different input types, this creates unnecessary complexity without functional differences in behavior.
Standards
- Algorithm-Correctness-Implementation
- Logic-Verification-Simplicity
| export function mutationOptions(options: unknown) { | ||
| return options | ||
| } |
There was a problem hiding this comment.
Duplicate Implementation Logic
The implementation simply returns the input parameter without modification. This pattern duplicates the implementation style of queryOptions but misses an opportunity for shared abstraction between similar functions, potentially leading to divergent implementations in future maintenance.
Standards
- Clean-Code-DRY
- Maintainability-Quality-Abstraction
- Design-Pattern-Template
| export function mutationOptions(options: unknown) { | ||
| return options | ||
| } |
There was a problem hiding this comment.
Simplify Implementation Logic
The implementation simply returns the input parameter unchanged, which means the function serves only as a type annotation helper. This pattern could be simplified with a generic identity function or type assertion to reduce code complexity while maintaining the same type safety.
Standards
- Algorithm-Correctness-Implementation
- Logic-Verification-Simplicity
mutationOptions helps extracting mutation options into separate functions.
Summary by CodeRabbit
New Features
Documentation
Tests