Prefer the callbacks passed to the execute function returned from useMutation (apollographql#10425)

Co-authored-by: Alessia Bellisario <alessia@apollographql.com>

Author: jerelmiller

.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.

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`

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 }) => (
+            <MockedProvider mocks={mocks}>
+              {children}
+            </MockedProvider>
+          )
+        },
+      );
+
+      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 }) => (
+            <MockedProvider mocks={mocks}>
+              {children}
+            </MockedProvider>
+          )
+        },
+      );
+
+      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 = {

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 };
       }