feat(vue-query): add 'mutationOptions'#10381
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a typed Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Hook as useMutation / useIsMutating
participant Client as QueryClient / MutationCache
participant MutFn as mutationFn
Dev->>Hook: provide `mutationOptions` (object or getter)
Hook->>Client: register/start mutation (include `mutationKey` if present)
Hook->>MutFn: call `mutationFn` with TVariables
MutFn-->>Client: resolve/reject (TData or error)
Client-->>Hook: emit mutation cache update
Hook-->>Dev: update reactive state, useIsMutating count, useMutationState selectors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 0761b46
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview1 package(s) bumped directly, 23 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
size-limit report 📦
|
| TVariables = void, | ||
| TOnMutateResult = unknown, | ||
| >( | ||
| options: VueMutationOptions<TData, TError, TVariables, TOnMutateResult>, |
There was a problem hiding this comment.
Should this also allow at least getter?
There was a problem hiding this comment.
@DamianOsipiuk Added getter overloads for both WithRequired and Omit variants, following the queryOptions pattern. Also added type tests and runtime tests for getter support. (ce18db8)
packages/vue-query/src/index.ts
Outdated
| UseInfiniteQueryReturnType, | ||
| } from './useInfiniteQuery' | ||
| export type { UseMutationOptions, UseMutationReturnType } from './useMutation' | ||
| export type { VueMutationOptions } from './types' |
There was a problem hiding this comment.
Would just MutationOptions work here?
There was a problem hiding this comment.
@DamianOsipiuk Renamed VueMutationOptions to MutationOptions across all files. (25ea553)
| expect(states.value).toEqual(['data1']) | ||
| }) | ||
|
|
||
| it('should work with options getter and be reactive when used with useIsMutating', async () => { |
There was a problem hiding this comment.
I do not think this test is testing what is mentioned in the title.
I would expect getter to be passed to mutationOptions which would auto update and trigger refetch when ref is updated
There was a problem hiding this comment.
@DamianOsipiuk Fixed — getter is now passed to mutationOptions(() => ({...})) instead of useIsMutating. (ce18db8)
| expect(isMutating.value).toEqual(1) | ||
| }) | ||
|
|
||
| it('should work with options getter and be reactive when used with useMutationState', async () => { |
There was a problem hiding this comment.
@DamianOsipiuk Fixed — same change applied to useMutationState test. (ce18db8)
…riptions to match actual behavior
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/vue-query/src/__tests__/mutationOptions.test.ts (1)
343-362:⚠️ Potential issue | 🟡 MinorGetter tests still don’t verify reactive key updates
Both tests create
keyRef, but never change it. This only proves the getter form is accepted, not that it re-evaluates when the ref changes (the core behavior implied by the test titles and prior feedback). Please update one/both tests to mutatekeyRefbetween mutations and assert filtering follows the new key.Suggested test direction
it('should work with getter passed to mutationOptions when used with useIsMutating', async () => { const keyRef = ref('isMutatingGetter') const mutationOpts = mutationOptions(() => ({ mutationKey: [keyRef.value], mutationFn: () => sleep(10).then(() => 'data'), })) const { mutate } = useMutation(mutationOpts) - mutate() - - const isMutating = useIsMutating({ - mutationKey: [keyRef.value], - }) + const isMutating = useIsMutating(() => ({ + mutationKey: [keyRef.value], + })) + + mutate() + await vi.advanceTimersByTimeAsync(0) + expect(isMutating.value).toEqual(1) + await vi.advanceTimersByTimeAsync(10) + expect(isMutating.value).toEqual(0) + + keyRef.value = 'isMutatingGetter-next' + mutate() await vi.advanceTimersByTimeAsync(0) expect(isMutating.value).toEqual(1) await vi.advanceTimersByTimeAsync(10) expect(isMutating.value).toEqual(0) })Also applies to: 364-381
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-query/src/__tests__/mutationOptions.test.ts` around lines 343 - 362, Update the tests that use keyRef, mutationOptions, useMutation and useIsMutating to actually mutate keyRef between mutations and assert the isMutating count follows the updated key; specifically, after the first mutate() and relevant timer advances/assertions, change keyRef.value to a new key, call mutate() again, and then advance timers and assert that useIsMutating (created with mutationKey: [keyRef.value] or a getter) reflects the new key (i.e., the previous key is no longer counted and the new key is counted while pending). Ensure the test uses the same mutationOptions/getter form and updates keyRef before the second mutation to verify reactive re-evaluation.
🧹 Nitpick comments (1)
packages/vue-query/src/__tests__/mutationOptions.test.ts (1)
21-28: Use reference-equality assertions for “without modification”
toStrictEqualvalidates structure, not identity. Since these tests claim unchanged pass-through behavior,toBeis a stronger assertion.Proposed assertion tweak
- expect(mutationOptions(object)).toStrictEqual(object) + expect(mutationOptions(object)).toBe(object)Also applies to: 30-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-query/src/__tests__/mutationOptions.test.ts` around lines 21 - 28, The test asserts that mutationOptions returns the same object instance but uses toStrictEqual (structural equality); update the two relevant tests (the one using the object with mutationKey and the similar one at lines 30-36) to assert reference identity by replacing expect(mutationOptions(object)).toStrictEqual(object) with expect(mutationOptions(object)).toBe(object), so the test verifies pass-through behavior by reference equality for the mutationOptions function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/vue-query/src/__tests__/mutationOptions.test.ts`:
- Around line 343-362: Update the tests that use keyRef, mutationOptions,
useMutation and useIsMutating to actually mutate keyRef between mutations and
assert the isMutating count follows the updated key; specifically, after the
first mutate() and relevant timer advances/assertions, change keyRef.value to a
new key, call mutate() again, and then advance timers and assert that
useIsMutating (created with mutationKey: [keyRef.value] or a getter) reflects
the new key (i.e., the previous key is no longer counted and the new key is
counted while pending). Ensure the test uses the same mutationOptions/getter
form and updates keyRef before the second mutation to verify reactive
re-evaluation.
---
Nitpick comments:
In `@packages/vue-query/src/__tests__/mutationOptions.test.ts`:
- Around line 21-28: The test asserts that mutationOptions returns the same
object instance but uses toStrictEqual (structural equality); update the two
relevant tests (the one using the object with mutationKey and the similar one at
lines 30-36) to assert reference identity by replacing
expect(mutationOptions(object)).toStrictEqual(object) with
expect(mutationOptions(object)).toBe(object), so the test verifies pass-through
behavior by reference equality for the mutationOptions function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2bd3f67e-579e-483c-b80f-6e3f3e196d56
📒 Files selected for processing (3)
packages/vue-query/src/__tests__/mutationOptions.test-d.tspackages/vue-query/src/__tests__/mutationOptions.test.tspackages/vue-query/src/mutationOptions.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/vue-query/src/tests/mutationOptions.test-d.ts
- packages/vue-query/src/mutationOptions.ts
github-actions
bot
added
the
documentation
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue-query/src/__tests__/mutationOptions.test-d.ts`:
- Around line 18-21: The `@ts-expect-error` is placed before mutationFn but the
intentional TypeScript error is on the onMutates property; move the
`@ts-expect-error` directive so it precedes the onMutates line (the line
containing onMutates: 1000) to correctly suppress the type error for onMutates
while leaving mutationFn and mutationKey unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ebc2c21-70da-4921-90f2-c7b1aa9a206e
📒 Files selected for processing (5)
packages/vue-query/src/__tests__/mutationOptions.test-d.tspackages/vue-query/src/index.tspackages/vue-query/src/mutationOptions.tspackages/vue-query/src/types.tspackages/vue-query/src/useMutation.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/vue-query/src/index.ts
- packages/vue-query/src/useMutation.ts
- packages/vue-query/src/mutationOptions.ts
- packages/vue-query/src/types.ts
| // @ts-expect-error this is a good error, because onMutates does not exist! | ||
| mutationFn: () => Promise.resolve(5), | ||
| mutationKey: ['key'], | ||
| onMutates: 1000, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification: confirm which line each `@ts-expect-error` currently guards.
file="packages/vue-query/src/__tests__/mutationOptions.test-d.ts"
awk '
/@ts-expect-error/{
dirLine=NR
while (getline next) {
if (next ~ /^[[:space:]]*$/) continue
print "Directive at line " dirLine
print "Next non-empty line: " NR ": " next
print "---"
break
}
}
' "$file"Repository: TanStack/query
Length of output: 150
🏁 Script executed:
cat -n packages/vue-query/src/__tests__/mutationOptions.test-d.ts | sed -n '16,25p'Repository: TanStack/query
Length of output: 473
🏁 Script executed:
rg "@ts-expect-error" packages/vue-query/src/__tests__/mutationOptions.test-d.ts -nRepository: TanStack/query
Length of output: 423
🏁 Script executed:
cat -n packages/vue-query/src/__tests__/mutationOptions.test-d.ts | sed -n '95,105p'Repository: TanStack/query
Length of output: 517
🏁 Script executed:
cat -n packages/vue-query/src/__tests__/mutationOptions.test-d.ts | sed -n '173,180p'Repository: TanStack/query
Length of output: 335
🏁 Script executed:
cat -n packages/vue-query/src/__tests__/mutationOptions.test-d.ts | sed -n '192,200p'Repository: TanStack/query
Length of output: 351
🏁 Script executed:
cat -n packages/vue-query/src/__tests__/mutationOptions.test-d.ts | sed -n '211,220p'Repository: TanStack/query
Length of output: 319
@ts-expect-error directive is on the wrong line.
The directive on line 18 applies to line 19 (mutationFn), but the intentional error is on line 21 (onMutates). Move the directive to line 21 to guard the correct property.
Fix
mutationOptions({
- // `@ts-expect-error` this is a good error, because onMutates does not exist!
mutationFn: () => Promise.resolve(5),
mutationKey: ['key'],
+ // `@ts-expect-error` this is a good error, because onMutates does not exist!
onMutates: 1000,
onSuccess: (data) => {
expectTypeOf(data).toEqualTypeOf<number>()
},
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // @ts-expect-error this is a good error, because onMutates does not exist! | |
| mutationFn: () => Promise.resolve(5), | |
| mutationKey: ['key'], | |
| onMutates: 1000, | |
| mutationFn: () => Promise.resolve(5), | |
| mutationKey: ['key'], | |
| // `@ts-expect-error` this is a good error, because onMutates does not exist! | |
| onMutates: 1000, | |
| onSuccess: (data) => { | |
| expectTypeOf(data).toEqualTypeOf<number>() | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-query/src/__tests__/mutationOptions.test-d.ts` around lines 18 -
21, The `@ts-expect-error` is placed before mutationFn but the intentional
TypeScript error is on the onMutates property; move the `@ts-expect-error`
directive so it precedes the onMutates line (the line containing onMutates:
1000) to correctly suppress the type error for onMutates while leaving
mutationFn and mutationKey unchanged.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| id: mutationOptions | |||
There was a problem hiding this comment.
You also need to add reference in the config
Ex.
https://github.com/TanStack/query/blob/main/docs/config.json#L1003
There was a problem hiding this comment.
@DamianOsipiuk Added mutationOptions entry to docs/config.json. (0761b46)
2 participants
🎯 Changes
mutationOptionshelper function for type-safe mutation option creation in vue-query() => options) overloads, followingqueryOptionspatternVueMutationOptionstoMutationOptionsfor consistency with other adaptersUseMutationOptionsBaseinuseMutation.tsto useMutationOptionsfromtypes.tsmutationOptions, not touseIsMutating/useMutationState)mutationOptionsreference page and config.json navigation entry✅ Checklist
pnpm run test:pr.🚀 Release Impact