accept onError and onCompleted in the useMutation execution function (#…

…9076) * accept onError and onCompleted in the useMutation * Update CHANGELOG.md
apollographql · Nov 18, 2021 · d6b8278 · d6b8278
1 parent 30f0b67
commit d6b8278
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## Apollo Client 3.5.4 (unreleased)
+
+- Restore the ability to pass `onError()` and `onCompleted()` to the mutation execution function. <br/> [@brainkim](https://github.com/brainkim) in [#9076](https://github.com/apollographql/apollo-client/pull/9076)
+
 ## Apollo Client 3.5.3 (2021-11-17)
 
 - Avoid rewriting non-relative imported module specifiers in `config/rewriteModuleIds.ts` script, thereby allowing bundlers to resolve those imports as they see fit. <br/>

diff --git a/package.json b/package.json
@@ -56,7 +56,7 @@
     {
       "name": "apollo-client",
       "path": "./dist/apollo-client.min.cjs",
-      "maxSize": "28.5kB"
+      "maxSize": "28.6kB"
     }
   ],
   "engines": {

diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx
@@ -198,7 +198,7 @@ describe('useMutation Hook', () => {
           {
             request: {
               query: CREATE_TODO_MUTATION,
-              variables
+              variables,
             },
             result: {
               data: CREATE_TODO_RESULT,
@@ -416,6 +416,116 @@ describe('useMutation Hook', () => {
     });
   });
 
+  describe('Callbacks', () => {
+    it('should allow passing an onCompleted handler to the execution function', async () => {
+      const CREATE_TODO_DATA = {
+        createTodo: {
+          id: 1,
+          priority: 'Low',
+          description: 'Get milk!',
+          __typename: 'Todo',
+        },
+      };
+
+      const mocks = [
+        {
+          request: {
+            query: CREATE_TODO_MUTATION,
+            variables: {
+              priority: 'Low',
+              description: 'Get milk.',
+            }
+          },
+          result: {
+            data: CREATE_TODO_DATA,
+          },
+        }
+      ];
+
+      const { result } = renderHook(
+        () => useMutation<
+          { createTodo: Todo },
+          { priority: string, description: string }
+        >(CREATE_TODO_MUTATION),
+        { wrapper: ({ children }) => (
+          <MockedProvider mocks={mocks}>
+            {children}
+          </MockedProvider>
+        )},
+      );
+
+      const createTodo = result.current[0];
+      let fetchResult: any;
+      const onCompleted = jest.fn();
+      const onError = jest.fn();
+      await act(async () => {
+        fetchResult = await createTodo({
+          variables: { priority: 'Low', description: 'Get milk.' },
+          onCompleted,
+          onError,
+        });
+      });
+
+      expect(fetchResult).toEqual({ data: CREATE_TODO_DATA });
+      expect(result.current[1].data).toEqual(CREATE_TODO_DATA);
+      expect(onCompleted).toHaveBeenCalledTimes(1);
+      expect(onCompleted).toHaveBeenCalledWith(CREATE_TODO_DATA);
+      expect(onError).toHaveBeenCalledTimes(0);
+    });
+
+    it('should allow passing an onError handler to the execution function', async () => {
+      const errors = [new GraphQLError(CREATE_TODO_ERROR)];
+      const mocks = [
+        {
+          request: {
+            query: CREATE_TODO_MUTATION,
+            variables: {
+              priority: 'Low',
+              description: 'Get milk.',
+            },
+          },
+          result: {
+            errors,
+          },
+        }
+      ];
+
+      const { result } = renderHook(
+        () => useMutation<
+          { createTodo: Todo },
+          { priority: string, description: string }
+        >(CREATE_TODO_MUTATION),
+        { wrapper: ({ children }) => (
+          <MockedProvider mocks={mocks}>
+            {children}
+          </MockedProvider>
+        )},
+      );
+
+      const createTodo = result.current[0];
+      let fetchResult: any;
+      const onCompleted = jest.fn();
+      const onError = jest.fn();
+      await act(async () => {
+        fetchResult = await createTodo({
+          variables: { priority: 'Low', description: 'Get milk.' },
+          onCompleted,
+          onError,
+        });
+      });
+
+      expect(fetchResult).toEqual({
+        data: undefined,
+        // Not sure why we unwrap errors here.
+        errors: errors[0],
+      });
+
+      expect(onCompleted).toHaveBeenCalledTimes(0);
+      expect(onError).toHaveBeenCalledTimes(1);
+      expect(onError).toHaveBeenCalledWith(errors[0]);
+    });
+  });
+
   describe('ROOT_MUTATION cache data', () => {
     const startTime = Date.now();
     const link = new ApolloLink(operation => new Observable(observer => {

diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts
@@ -92,7 +92,7 @@ export function useMutation<
 
         if (
           mutationId === ref.current.mutationId &&
-          !baseOptions.ignoreResults
+          !clientOptions.ignoreResults
         ) {
           const result = {
             called: true,
@@ -108,6 +108,7 @@ export function useMutation<
         }
 
         baseOptions.onCompleted?.(response.data!);
+        executeOptions.onCompleted?.(response.data!);
         return response;
       }).catch((error) => {
         if (
@@ -127,8 +128,9 @@ export function useMutation<
           }
         }
 
-        if (baseOptions.onError) {
-          baseOptions.onError(error);
+        if (baseOptions.onError || clientOptions.onError) {
+          baseOptions.onError?.(error);
+          executeOptions.onError?.(error);
           // TODO(brian): why are we returning this here???
           return { data: void 0, errors: error };
         }
@@ -148,6 +150,5 @@ export function useMutation<
     ref.current.isMounted = false;
   }, []);
 
-
   return [execute, { reset, ...result }];
 }