feat(react-query): add mutationOptions #8960
Conversation
github-actions
bot
added
documentation
Ubinquitous
force-pushed
the
feature/react-query-mutation-options
branch
from
5e14207 to
596896d
Compare
|
View your CI Pipeline Execution ↗ for commit 299a19f.
☁️ Nx Cloud last updated this comment at |
| TData, | ||
| TMutationKey | ||
| >['mutationFn'], | ||
| SkipToken | undefined |
There was a problem hiding this comment.
skipToken is specific to queries. This isn’t needed.
| UseMutationOptions<TMutationFnData, TError, TData, TMutationKey>, | ||
| 'mutationFn' | ||
| > & { | ||
| initialData: |
There was a problem hiding this comment.
there is no initialData for mutations
| TMutationKey | ||
| >, | ||
| ): DefinedInitialDataOptions<TMutationFnData, TError, TData, TMutationKey> & { | ||
| mutationKey: DataTag<TMutationKey, TMutationFnData, TError> |
There was a problem hiding this comment.
we also don’t need a DataTag, this is query specific.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8960 +/- ##
===========================================
+ Coverage 44.60% 84.30% +39.70%
===========================================
Files 207 27 -180
Lines 8167 376 -7791
Branches 1828 110 -1718
===========================================
- Hits 3643 317 -3326
+ Misses 4081 50 -4031
+ Partials 443 9 -434 🚀 New features to boost your workflow:
|
|
Thank you for reviewing my PR. I thought queryOptions and mutationOptions could be structured similarly since it was an options-related function. I re-created useMutation as start. I changed it to only have UseMutationOptions, excluding unnecessary data tags and initialData. |
| options: UseMutationOptions<TData, TError, TVariables, TContext>, | ||
| ): UseMutationOptions<TData, TError, TVariables, TContext> | ||
|
|
||
| export function mutationOptions(options: unknown) { |
There was a problem hiding this comment.
if there’s only declaration, we don’t need overloads
There was a problem hiding this comment.
Thank you, I changed it to a non-overloaded function.
|
I merged #9094 to resolve below ci failure because of flaky timer tests 
|
docs/framework/react/typescript.md
Outdated
| function useGroupPostMutation() { | ||
| const queryClient = useQueryClient() | ||
|
|
||
| return mutationOptions({ | ||
| mutationKey: ['groups'], | ||
| mutationFn: executeGroups, | ||
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ queryKey: ['posts'] }) | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
It might be worth highlighting in the documentation that mutationOptions can be reused across different interfaces—such as useMutation, useIsMutating, and queryClient.isMutating.
| function useGroupPostMutation() { | |
| const queryClient = useQueryClient() | |
| return mutationOptions({ | |
| mutationKey: ['groups'], | |
| mutationFn: executeGroups, | |
| onSuccess: () => { | |
| queryClient.invalidateQueries({ queryKey: ['posts'] }) | |
| }, | |
| }) | |
| } | |
| function groupMutationOptions() { | |
| return mutationOptions({ | |
| mutationKey: ['groups'], | |
| mutationFn: addGroup, | |
| }) | |
| } | |
| useMutation({ | |
| ...groupMutationOptions() | |
| onSuccess: () => queryClient.invalidateQueries({ queryKey: ['groups'] }) | |
| }) | |
| useIsMutating(groupMutationOptions()) | |
| queryClient.isMutating(groupMutationOptions()) |
There was a problem hiding this comment.
@TkDodo, @Ubinquitous I'd like to hear your thoughts on my review
| import type { DefaultError } from '@tanstack/query-core' | ||
| import type { UseMutationOptions } from './types' | ||
|
|
||
| export function mutationOptions< | ||
| TData = unknown, | ||
| TError = DefaultError, | ||
| TVariables = void, | ||
| TContext = unknown, | ||
| >( | ||
| options: UseMutationOptions<TData, TError, TVariables, TContext>, | ||
| ): UseMutationOptions<TData, TError, TVariables, TContext> { | ||
| return options | ||
| } |
There was a problem hiding this comment.
If mutationOptions is intended to be reused across various TanStack React Query interfaces—such as useMutation, useIsMutating, and queryClient.isMutating—then it might make sense to make mutationKey a required field, similar to how queryOptions.queryKey is typed.
| import type { DefaultError } from '@tanstack/query-core' | |
| import type { UseMutationOptions } from './types' | |
| export function mutationOptions< | |
| TData = unknown, | |
| TError = DefaultError, | |
| TVariables = void, | |
| TContext = unknown, | |
| >( | |
| options: UseMutationOptions<TData, TError, TVariables, TContext>, | |
| ): UseMutationOptions<TData, TError, TVariables, TContext> { | |
| return options | |
| } | |
| import type { WithRequired } from 'node_modules/@tanstack/query-core/build/legacy' | |
| import type { DefaultError } from '@tanstack/query-core' | |
| import type { UseMutationOptions } from './types' | |
| export function mutationOptions< | |
| TData = unknown, | |
| TError = DefaultError, | |
| TVariables = void, | |
| TContext = unknown, | |
| >( | |
| options: WithRequired< | |
| UseMutationOptions<TData, TError, TVariables, TContext>, | |
| 'mutationKey' | |
| >, | |
| ): WithRequired< | |
| UseMutationOptions<TData, TError, TVariables, TContext>, | |
| 'mutationKey' | |
| > { | |
| return options | |
| } | |
Making mutationKey required could help avoid situations like the following:
// Without mutationKey, it’s unavailable for useIsMutating or queryClient.isMutating
// cannot reliably identify the mutation, which may lead to unintended behavior.
function groupMutationOptions() {
return mutationOptions({
mutationFn: addGroup,
});
}
useMutation({
...groupMutationOptions(),
onSuccess: () => queryClient.invalidateQueries({ queryKey: ['groups'] }),
});
useIsMutating(groupMutationOptions())
// This cannot detect the isMutating state from the above hook
// because groupMutationOptions doesn't include a mutationKey.
// but TypeScript compiler doesn't detect this as errorSo in my opinion, mutationOptions's mutationKey should be required field.
Additionally, we can make it as optional field later if we need without BREAKING CHANGE.
So when we first add mutationOptions, it might be beneficial to make mutationKey a required field at first
There was a problem hiding this comment.
If mutationOptions is used not only in useMutation but also in other interfaces such as useIsMutating, I think it would be better to make it a required value.
One thing I'm concerned about is that developers who only use mutationOptions for useMutation's options might end up writing unnecessary code.
|
@Ubinquitous Resolve eslint error please |
|
Sorry, I fix error that 'should not allow excess properties' test don't have assertions |
mutationOptions helps extracting mutation options into separate functions.