diff --git a/.changeset/metal-hats-cough.md b/.changeset/metal-hats-cough.md new file mode 100644 index 00000000000..dc4e7a739b9 --- /dev/null +++ b/.changeset/metal-hats-cough.md @@ -0,0 +1,5 @@ +--- +'@apollo/client': patch +--- + +Prefer the `onError` and `onCompleted` callback functions passed to the execute function returned from `useMutation` instead of calling both callback handlers. diff --git a/docs/shared/mutation-result.mdx b/docs/shared/mutation-result.mdx index 68900ac071e..c70dbcda322 100644 --- a/docs/shared/mutation-result.mdx +++ b/docs/shared/mutation-result.mdx @@ -23,6 +23,8 @@ A function to trigger the mutation from your UI. You can optionally pass this fu * `awaitRefetchQueries` * `context` * `fetchPolicy` +* `onCompleted` +* `onError` * `optimisticResponse` * `refetchQueries` * `update` diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index 032721577b0..2f02d0c6869 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -659,6 +659,54 @@ describe('useMutation Hook', () => { expect(onError).toHaveBeenCalledTimes(0); }); + it('prefers the onCompleted handler passed to the execution function rather than the hook', async () => { + const CREATE_TODO_DATA = { + createTodo: { + id: 1, + priority: 'Low', + description: 'Get milk!', + __typename: 'Todo', + }, + }; + const variables = { + priority: 'Low', + description: 'Get milk.', + } + const mocks = [ + { + request: { + query: CREATE_TODO_MUTATION, + variables, + }, + result: { + data: CREATE_TODO_DATA + }, + } + ]; + + const hookOnCompleted = jest.fn(); + + const { result } = renderHook( + () => useMutation(CREATE_TODO_MUTATION, { onCompleted: hookOnCompleted }), + { + wrapper: ({ children }) => ( + + {children} + + ) + }, + ); + + const [createTodo] = result.current; + const onCompleted = jest.fn(); + await act(async () => { + await createTodo({ variables, onCompleted }); + }); + + expect(onCompleted).toHaveBeenCalledTimes(1); + expect(hookOnCompleted).not.toHaveBeenCalled(); + }); + it('should allow passing an onError handler to the execution function', async () => { const errors = [new GraphQLError(CREATE_TODO_ERROR)]; const variables = { @@ -712,6 +760,46 @@ describe('useMutation Hook', () => { expect(onError).toHaveBeenCalledWith(errors[0], expect.objectContaining({variables})); }); + it('prefers the onError handler passed to the execution function instead of the hook', async () => { + const variables = { + priority: 'Low', + description: 'Get milk.', + } + const mocks = [ + { + request: { + query: CREATE_TODO_MUTATION, + variables, + }, + result: { + errors: [new GraphQLError(CREATE_TODO_ERROR)], + }, + } + ]; + + const hookOnError = jest.fn(); + + const { result } = renderHook( + () => useMutation(CREATE_TODO_MUTATION, { onError: hookOnError }), + { + wrapper: ({ children }) => ( + + {children} + + ) + }, + ); + + const [createTodo] = result.current; + const onError = jest.fn(); + await act(async () => { + await createTodo({ variables, onError }); + }); + + expect(onError).toHaveBeenCalledTimes(1); + expect(hookOnError).not.toHaveBeenCalled(); + }); + it('should allow updating onError while mutation is executing', async () => { const errors = [new GraphQLError(CREATE_TODO_ERROR)]; const variables = { diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts index 525121ecfc2..4937c08ba6f 100644 --- a/src/react/hooks/useMutation.ts +++ b/src/react/hooks/useMutation.ts @@ -100,8 +100,10 @@ export function useMutation< setResult(ref.current.result = result); } } - ref.current.options?.onCompleted?.(response.data!, clientOptions); - executeOptions.onCompleted?.(response.data!, clientOptions); + + const onCompleted = executeOptions.onCompleted || ref.current.options?.onCompleted + onCompleted?.(response.data!, clientOptions); + return response; }).catch((error) => { if ( @@ -121,9 +123,11 @@ export function useMutation< } } - if (ref.current.options?.onError || clientOptions.onError) { - ref.current.options?.onError?.(error, clientOptions); - executeOptions.onError?.(error, clientOptions); + const onError = executeOptions.onError || ref.current.options?.onError + + if (onError) { + onError(error, clientOptions); + // TODO(brian): why are we returning this here??? return { data: void 0, errors: error }; }