-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Description
Bug report
Description / Observed Behavior
When using useSWRMutation, the onSuccess callback is invoked after mutate, but it is not awaited.
If a developer provides an async onSuccess (for example, to revalidate data by calling mutate(key)), it runs in parallel with the return of trigger.
This leads to confusing and incorrect behavior:
UI first updates with the "raw mutation result",
then asynchronously updates again with the revalidated data,
and errors inside async onSuccess become unhandled promise rejections.
const { trigger } = useSWRMutation(
'/api/item',
updateItemFetcher,
{
onSuccess: async () => {
// Expectation: refetch data and update UI with backend state
await mutate('/api/item')
}
}
)
async function handleClick() {
const result = await trigger({ id: 1 })
console.log('trigger result', result)
}Current behavior:
trigger returns the mutation result immediately.
onSuccess runs asynchronously, but its result is ignored.
The component first renders with outdated/mutation-only data, then updates again later.
Expected Behavior
If onSuccess is async, trigger should await it before returning.
This ensures await trigger() is consistent and that UI/state updates happen in the expected order.
Repro Steps / Code Example
const trigger = useCallback(
async (arg: any, opts?: SWRMutationConfiguration<Data, Error>) => {
const [serializedKey, resolvedKey] = serialize(keyRef.current)
if (!fetcherRef.current) {
throw new Error('Can’t trigger the mutation: missing fetcher.')
}
if (!serializedKey) {
throw new Error('Can’t trigger the mutation: missing key.')
}
// Disable cache population by default.
const options = mergeObjects(
mergeObjects(
{ populateCache: false, throwOnError: true },
configRef.current
),
opts
)
// Trigger a mutation, and also track the timestamp. Any mutation that happened
// earlier this timestamp should be ignored.
const mutationStartedAt = getTimestamp()
ditchMutationsUntilRef.current = mutationStartedAt
setState({ isMutating: true })
try {
const data = await mutate<Data>(
serializedKey,
(fetcherRef.current as any)(resolvedKey, { arg }),
// We must throw the error here so we can catch and update the states.
mergeObjects(options, { throwOnError: true })
)
// If it's reset after the mutation, we don't broadcast any state change.
if (ditchMutationsUntilRef.current <= mutationStartedAt) {
startTransition(() =>
setState({ data, isMutating: false, error: undefined })
)
options.onSuccess?.(data as Data, serializedKey, options)
}
return data
} catch (error) {
// If it's reset after the mutation, we don't broadcast any state change
// or throw because it's discarded.
if (ditchMutationsUntilRef.current <= mutationStartedAt) {
startTransition(() =>
setState({ error: error as Error, isMutating: false })
)
options.onError?.(error as Error, serializedKey, options)
if (options.throwOnError) {
throw error as Error
}
}
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[]
)options.onSuccess?.(data, serializedKey, options)
return dataBecause the callback is not awaited, async functions cause race conditions and unhandled rejections.
Current documentation does not mention that onSuccess must be synchronous.
At minimum, docs should warn developers that async callbacks will not be awaited.
Ideally, the code should await async onSuccess, making trigger behavior consistent and predictable.
Additional Context
SWR version - 2.3.6