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 2622902.
☁️ 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.40% 84.09% +39.68%
===========================================
Files 206 26 -180
Lines 8174 371 -7803
Branches 1822 109 -1713
===========================================
- Hits 3630 312 -3318
+ Misses 4100 50 -4050
+ Partials 444 9 -435 🚀 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.
There was a problem hiding this comment.
yeah we can’t make it required; yes filters won’t work then; it’s one of the reasons why I’m against this helper in the first place 😅
|
@Ubinquitous Resolve eslint error please |
|
Sorry, I fix error that 'should not allow excess properties' test don't have assertions |
|
Really looking forward to this PR! Its really helpful to add reusable optimistic updates, rollbacks etc callbacks for mutations. |
|
Thank you, @andredewaard. I'm not sure if I understood it correctly, but it seems like this can basically be implemented using the spread operator. I’ve used a similar pattern when working with queryOptions as well. Is this what you were referring to? const options = mutationOptions({
...
})
useMutation({
...options,
onSuccess: () => {},
})If you're only changing the funnel in a specific component, I recommend wrapping it in a function for better management. I used to do it that way as well when working with Alternatively, if you're using a function, you could also add logic to branch only when a specific component name is passed in. export const postQueries = {
all: () => ['post-list'],
list: (): UseQueryOptions<Post[], Error, string> =>
queryOptions({
queryKey: [...postListQueries.all()],
queryFn: () => getPostList(),
select: (data) => getPostListString(data),
}),
detail: (type: PostType): UseSuspenseQueryOptions<PostInfo, Error> =>
queryOptions({
queryKey: [...postListQueries.all(), 'detail', type],
queryFn: () => getPostDetail({ type }),
select: (data) => data,
}),
...
}; |
|
@Ubinquitous Thanks for your reply. So more like this. useMutation({
...options,
onSuccess: () => {
options.onSuccess()
// added logic
},
}) |
|
yes, with optional chaining on the options callback: |
|
To override queryOptions without using the spread operator twice, you can use a prop getter. const compose = (...functions) => (...args) =>
functions.forEach((fn) => fn?.(...args))
const options = queryOptions({ ... })
const getOptions = ({ onSuccess }) => {
return {
onSuccess: compose(onSuccess, options.onSuccess),
...options
}
}
getOptions({
onSuccess: () => {} // behaves the same
}) |
mutationOptions helps extracting mutation options into separate functions.