diff --git a/packages/query-core/src/mutationObserver.ts b/packages/query-core/src/mutationObserver.ts index 20f7e62da2..af66852895 100644 --- a/packages/query-core/src/mutationObserver.ts +++ b/packages/query-core/src/mutationObserver.ts @@ -166,7 +166,7 @@ export class MutationObserver< private notify(options: NotifyOptions) { notifyManager.batch(() => { // First trigger the mutate callbacks - if (this.mutateOptions) { + if (this.mutateOptions && this.hasListeners()) { if (options.onSuccess) { this.mutateOptions.onSuccess?.( this.currentResult.data!, diff --git a/packages/query-core/src/tests/mutations.test.tsx b/packages/query-core/src/tests/mutations.test.tsx index 5eced09b45..ccb6ae2ad1 100644 --- a/packages/query-core/src/tests/mutations.test.tsx +++ b/packages/query-core/src/tests/mutations.test.tsx @@ -308,49 +308,24 @@ describe('mutations', () => { expect(onSettled).toHaveBeenCalled() }) - test('setState should update the mutation state', async () => { - const mutation = new MutationObserver(queryClient, { - mutationFn: async () => { - return 'update' - }, - onMutate: (text) => text, - }) - await mutation.mutate() - expect(mutation.getCurrentResult().data).toEqual('update') - - // Force setState usage - // because no use case has been found using mutation.setState - const currentMutation = mutation['currentMutation'] - currentMutation?.setState({ - context: undefined, - variables: undefined, - data: 'new', - error: undefined, - failureCount: 0, - failureReason: null, - isPaused: false, - status: 'success', - }) + test('addObserver should not add an existing observer', async () => { + const mutationCache = queryClient.getMutationCache() + const observer = new MutationObserver(queryClient, {}) + const currentMutation = mutationCache.build(queryClient, {}) - expect(mutation.getCurrentResult().data).toEqual('new') - }) + const fn = jest.fn() - test('addObserver should not add an existing observer', async () => { - const mutation = new MutationObserver(queryClient, { - mutationFn: async () => { - return 'update' - }, - onMutate: (text) => text, + const unsubscribe = mutationCache.subscribe((event) => { + fn(event.type) }) - await mutation.mutate() - // Force addObserver usage to add an existing observer - // because no use case has been found - const currentMutation = mutation['currentMutation']! - expect(currentMutation['observers'].length).toEqual(1) - currentMutation.addObserver(mutation) + currentMutation.addObserver(observer) + currentMutation.addObserver(observer) - expect(currentMutation['observers'].length).toEqual(1) + expect(fn).toHaveBeenCalledTimes(1) + expect(fn).toHaveBeenCalledWith('observerAdded') + + unsubscribe() }) test('mutate should throw an error if no mutationFn found', async () => { @@ -367,4 +342,20 @@ describe('mutations', () => { } expect(error).toEqual('No mutationFn found') }) + + test('mutate update the mutation state even without an active subscription', async () => { + const onSuccess = jest.fn() + const onSettled = jest.fn() + + const mutation = new MutationObserver(queryClient, { + mutationFn: async () => { + return 'update' + }, + }) + + await mutation.mutate(undefined, { onSuccess, onSettled }) + expect(mutation.getCurrentResult().data).toEqual('update') + expect(onSuccess).not.toHaveBeenCalled() + expect(onSettled).not.toHaveBeenCalled() + }) }) diff --git a/packages/react-query/src/__tests__/useMutation.test.tsx b/packages/react-query/src/__tests__/useMutation.test.tsx index fe626500ad..ff32046f0e 100644 --- a/packages/react-query/src/__tests__/useMutation.test.tsx +++ b/packages/react-query/src/__tests__/useMutation.test.tsx @@ -1044,7 +1044,7 @@ describe('useMutation', () => { ) }) - test('should go to error state if onSuccess callback errors', async () => { + it('should go to error state if onSuccess callback errors', async () => { const error = new Error('error from onSuccess') const onError = jest.fn() @@ -1079,7 +1079,7 @@ describe('useMutation', () => { expect(onError).toHaveBeenCalledWith(error, 'todo', undefined) }) - test('should go to error state if onError callback errors', async () => { + it('should go to error state if onError callback errors', async () => { const error = new Error('error from onError') const mutateFnError = new Error('mutateFnError') @@ -1115,7 +1115,7 @@ describe('useMutation', () => { await rendered.findByText('error: mutateFnError, status: error') }) - test('should go to error state if onSettled callback errors', async () => { + it('should go to error state if onSettled callback errors', async () => { const error = new Error('error from onSettled') const mutateFnError = new Error('mutateFnError') const onError = jest.fn() @@ -1154,4 +1154,60 @@ describe('useMutation', () => { expect(onError).toHaveBeenCalledWith(mutateFnError, 'todo', undefined) }) + + it('should not call mutate callbacks for mutations started after unmount', async () => { + const onSuccessMutate = jest.fn() + const onSuccessUseMutation = jest.fn() + const onSettledMutate = jest.fn() + const onSettledUseMutation = jest.fn() + + function Page() { + const [show, setShow] = React.useState(true) + return ( +
+ + {show && } +
+ ) + } + + function Component() { + const mutation = useMutation({ + mutationFn: async (text: string) => { + await sleep(10) + return text + }, + onSuccess: onSuccessUseMutation, + onSettled: onSettledUseMutation, + }) + + return ( +
+ +
+ ) + } + + const rendered = renderWithClient(queryClient, ) + + fireEvent.click(rendered.getByRole('button', { name: /mutate/i })) + fireEvent.click(rendered.getByRole('button', { name: /hide/i })) + + await waitFor(() => expect(onSuccessUseMutation).toHaveBeenCalledTimes(1)) + await waitFor(() => expect(onSettledUseMutation).toHaveBeenCalledTimes(1)) + + expect(onSuccessMutate).toHaveBeenCalledTimes(0) + expect(onSettledMutate).toHaveBeenCalledTimes(0) + }) })