-
Notifications
You must be signed in to change notification settings - Fork 0
feat(react-query): add mutationOptions #4
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
View your CI Pipeline Execution ↗ for commit ff15e5d.
☁️ Nx Cloud last updated this comment at |
@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 (
|
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: 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-error
as 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
, thedataTagSymbol
from the core package, and themutationOptions
function being tested.
15-23
: LGTM! Well-implemented type inference testThis test successfully verifies that the TypeScript compiler correctly infers the type of the
data
parameter in theonSuccess
callback 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
mutationKey
is 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
mutationFn
is provided - themutationKey
should be tagged withunknown
since there's no way to infer the data type.
42-51
: LGTM! Properly testing type preservation with callbacksThis test ensures that adding an
onSuccess
callback doesn't affect the type tagging of themutationKey
. This is important for consistent type inference throughout the library.
mutationOptions helps extracting mutation options into separate functions.
Summary by CodeRabbit
New Features
Documentation
Tests