Skip to content

Commit 3591b24

Browse files
committed
test: add callback return type tests for mutation callbacks
Adds test coverage for mutation callback return types, inspired by the discussions and [TkDodo's comment](TanStack#9245 (comment) ) in TanStack#9245 and the original Promise.all() edge case identified in TanStack#9202. The original PR TanStack#9202 narrowed callback return types from `Promise<unknown> | unknown` to `Promise<void> | void`, but this broke common patterns like: ```ts onSuccess: (data) => Promise.all([ invalidateQueries(), trackAnalytics(), ]) ``` As noted in [the original discussion](TanStack#9202 (comment)), this `Promise.all()` pattern was a legitimate use case that many users relied on. These tests ensure we support all the callback patterns that users expect: ✅ **Sync patterns**: Implicit void, explicit void, non-void returns ✅ **Async patterns**: Async functions, Promise.resolve(), Promise<T> returns ✅ **Promise.all() patterns**: The original breaking case from TanStack#9202 ✅ **Promise.allSettled() patterns**: Additional parallel operation support ✅ **Mixed patterns**: Different callback types in same mutation ✅ **Error handling**: All patterns work in error scenarios ✅ **Return value isolation**: Callback returns don't affect mutation result
1 parent fa828a0 commit 3591b24

File tree

1 file changed

+212
-0
lines changed

1 file changed

+212
-0
lines changed

packages/query-core/src/__tests__/mutations.test.tsx

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,4 +613,216 @@ describe('mutations', () => {
613613
'finish-B2',
614614
])
615615
})
616+
617+
describe('callback return types', () => {
618+
test('should handle all sync callback patterns', async () => {
619+
const key = queryKey()
620+
const results: Array<string> = []
621+
622+
await executeMutation(
623+
queryClient,
624+
{
625+
mutationKey: key,
626+
mutationFn: () => Promise.resolve('success'),
627+
onMutate: (variables) => {
628+
results.push('onMutate-sync')
629+
return { backup: 'data' } // onMutate can return context
630+
},
631+
onSuccess: (data, variables, context) => {
632+
results.push('onSuccess-implicit-void')
633+
// Implicit void return
634+
},
635+
onError: (error, variables, context) => {
636+
results.push('onError-explicit-void')
637+
return // Explicit void return
638+
},
639+
onSettled: (data, error, variables, context) => {
640+
results.push('onSettled-return-value')
641+
return 'ignored-value' // Non-void return (should be ignored)
642+
},
643+
},
644+
'vars',
645+
)
646+
647+
expect(results).toEqual([
648+
'onMutate-sync',
649+
'onSuccess-implicit-void',
650+
'onSettled-return-value',
651+
])
652+
})
653+
654+
test('should handle all async callback patterns', async () => {
655+
const key = queryKey()
656+
const results: Array<string> = []
657+
658+
executeMutation(
659+
queryClient,
660+
{
661+
mutationKey: key,
662+
mutationFn: () => Promise.resolve('success'),
663+
onMutate: async (variables) => {
664+
results.push('onMutate-async')
665+
await sleep(1)
666+
return { backup: 'async-data' }
667+
},
668+
onSuccess: async (data, variables, context) => {
669+
results.push('onSuccess-async-start')
670+
await sleep(2)
671+
results.push('onSuccess-async-end')
672+
// Implicit void return from async
673+
},
674+
onSettled: (data, error, variables, context) => {
675+
results.push('onSettled-promise')
676+
return Promise.resolve('also-ignored') // Promise<string> (should be ignored)
677+
},
678+
},
679+
'vars',
680+
)
681+
682+
await vi.runAllTimersAsync()
683+
684+
expect(results).toEqual([
685+
'onMutate-async',
686+
'onSuccess-async-start',
687+
'onSuccess-async-end',
688+
'onSettled-promise',
689+
])
690+
})
691+
692+
test('should handle Promise.all() and Promise.allSettled() patterns', async () => {
693+
const key = queryKey()
694+
const results: Array<string> = []
695+
696+
executeMutation(
697+
queryClient,
698+
{
699+
mutationKey: key,
700+
mutationFn: () => Promise.resolve('success'),
701+
onSuccess: (data, variables, context) => {
702+
results.push('onSuccess-start')
703+
return Promise.all([
704+
sleep(2).then(() => results.push('invalidate-queries')),
705+
sleep(1).then(() => results.push('track-analytics')),
706+
])
707+
},
708+
onSettled: (data, error, variables, context) => {
709+
results.push('onSettled-start')
710+
return Promise.allSettled([
711+
sleep(1).then(() => results.push('cleanup-1')),
712+
Promise.reject('error').catch(() =>
713+
results.push('cleanup-2-failed'),
714+
),
715+
])
716+
},
717+
},
718+
'vars',
719+
)
720+
721+
await vi.runAllTimersAsync()
722+
723+
expect(results).toEqual([
724+
'onSuccess-start',
725+
'track-analytics',
726+
'invalidate-queries',
727+
'onSettled-start',
728+
'cleanup-1',
729+
'cleanup-2-failed',
730+
])
731+
})
732+
733+
test('should handle mixed sync/async patterns and return value isolation', async () => {
734+
const key = queryKey()
735+
const results: Array<string> = []
736+
737+
const mutationResult = await executeMutation(
738+
queryClient,
739+
{
740+
mutationKey: key,
741+
mutationFn: () => Promise.resolve('actual-result'),
742+
onMutate: (variables) => {
743+
results.push('sync-onMutate')
744+
return { rollback: 'data' }
745+
},
746+
onSuccess: async (data, variables, context) => {
747+
results.push('async-onSuccess')
748+
await sleep(1)
749+
return 'success-return-ignored'
750+
},
751+
onError: (error, variables, context) => {
752+
results.push('sync-onError')
753+
return Promise.resolve('error-return-ignored')
754+
},
755+
onSettled: (data, error, variables, context) => {
756+
results.push(`settled-context-${context?.rollback}`)
757+
return Promise.all([
758+
Promise.resolve('cleanup-1'),
759+
Promise.resolve('cleanup-2'),
760+
])
761+
},
762+
},
763+
'vars',
764+
)
765+
766+
await vi.runAllTimersAsync()
767+
768+
// Verify mutation returns its own result, not callback returns
769+
expect(mutationResult).toBe('actual-result')
770+
expect(results).toEqual([
771+
'sync-onMutate',
772+
'async-onSuccess',
773+
'settled-context-data',
774+
])
775+
})
776+
777+
test('should handle error cases with all callback patterns', async () => {
778+
const key = queryKey()
779+
const results: Array<string> = []
780+
781+
try {
782+
await executeMutation(
783+
queryClient,
784+
{
785+
mutationKey: key,
786+
mutationFn: () => Promise.reject(new Error('mutation-error')),
787+
onMutate: (variables) => {
788+
results.push('onMutate')
789+
return { backup: 'error-data' }
790+
},
791+
onSuccess: (data, variables, context) => {
792+
results.push('onSuccess-should-not-run')
793+
},
794+
onError: async (error, variables, context) => {
795+
results.push('onError-async')
796+
await sleep(1)
797+
// Test Promise.all() in error callback
798+
return Promise.all([
799+
sleep(1).then(() => results.push('error-cleanup-1')),
800+
sleep(2).then(() => results.push('error-cleanup-2')),
801+
])
802+
},
803+
onSettled: (data, error, variables, context) => {
804+
results.push(`settled-error-${context?.backup}`)
805+
return Promise.allSettled([
806+
Promise.resolve('settled-cleanup'),
807+
Promise.reject('settled-error'),
808+
])
809+
},
810+
},
811+
'vars',
812+
)
813+
} catch {
814+
// Expected error
815+
}
816+
817+
await vi.runAllTimersAsync()
818+
819+
expect(results).toEqual([
820+
'onMutate',
821+
'onError-async',
822+
'error-cleanup-1',
823+
'error-cleanup-2',
824+
'settled-error-error-data',
825+
])
826+
})
827+
})
616828
})

0 commit comments

Comments
 (0)