From 5252a6da223d5628811f7cc2a0cf1765855e09df Mon Sep 17 00:00:00 2001 From: Cayla Hamann Date: Wed, 26 Jul 2023 15:29:47 -0400 Subject: [PATCH 1/8] fix: useMutation calls onError if errorPolicy is all --- .../hooks/__tests__/useMutation.test.tsx | 42 +++++++++++++++++++ src/react/hooks/useMutation.ts | 8 ++++ 2 files changed, 50 insertions(+) diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index fd55e5bd919..889d10d2adb 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -440,6 +440,48 @@ describe('useMutation Hook', () => { expect(fetchResult.errors[0].message).toEqual(CREATE_TODO_ERROR); }); + it(`should call onError when errorPolicy is 'all'`, async () => { + const variables = { + description: 'Get milk!' + }; + + const mocks = [ + { + request: { + query: CREATE_TODO_MUTATION, + variables + }, + result: { + data: CREATE_TODO_RESULT, + errors: [new GraphQLError(CREATE_TODO_ERROR)], + }, + } + ]; + + const onError = jest.fn(); + + const { result } = renderHook( + () => useMutation(CREATE_TODO_MUTATION, { errorPolicy: 'all', onError }), + { wrapper: ({ children }) => ( + + {children} + + )}, + ); + + const createTodo = result.current[0]; + + let fetchResult: any; + await act(async () => { + fetchResult = await createTodo({ variables }); + }); + + expect(fetchResult.data).toEqual(CREATE_TODO_RESULT); + expect(fetchResult.errors[0].message).toEqual(CREATE_TODO_ERROR); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError.mock.calls[0][0].message).toBe(CREATE_TODO_ERROR); + }) + it(`should ignore errors when errorPolicy is 'ignore'`, async () => { const errorMock = jest.spyOn(console, "error") .mockImplementation(() => {}); diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts index 38b45c1e715..8aaf22aa5a0 100644 --- a/src/react/hooks/useMutation.ts +++ b/src/react/hooks/useMutation.ts @@ -86,6 +86,14 @@ export function useMutation< ? new ApolloError({ graphQLErrors: errors }) : void 0; + const errorPolicy = clientOptions.errorPolicy; + + const onError = executeOptions.onError || ref.current.options?.onError + + if (error && errorPolicy === 'all' && onError) { + onError(error, clientOptions); + } + if ( mutationId === ref.current.mutationId && !clientOptions.ignoreResults From 33d4e17540875d216fcd8915cb9b3809397eba66 Mon Sep 17 00:00:00 2001 From: Cayla Hamann Date: Wed, 26 Jul 2023 15:59:34 -0400 Subject: [PATCH 2/8] chore: check that on complete isnt called for errorpolicy all --- .../hooks/__tests__/useMutation.test.tsx | 43 ++++++++++++++++++- src/react/hooks/useMutation.ts | 5 ++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index 889d10d2adb..db2e453b0b8 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -459,9 +459,10 @@ describe('useMutation Hook', () => { ]; const onError = jest.fn(); + const onCompleted = jest.fn(); const { result } = renderHook( - () => useMutation(CREATE_TODO_MUTATION, { errorPolicy: 'all', onError }), + () => useMutation(CREATE_TODO_MUTATION, { errorPolicy: 'all', onError, onCompleted }), { wrapper: ({ children }) => ( {children} @@ -480,6 +481,7 @@ describe('useMutation Hook', () => { expect(fetchResult.errors[0].message).toEqual(CREATE_TODO_ERROR); expect(onError).toHaveBeenCalledTimes(1); expect(onError.mock.calls[0][0].message).toBe(CREATE_TODO_ERROR); + expect(onCompleted).not.toHaveBeenCalled(); }) it(`should ignore errors when errorPolicy is 'ignore'`, async () => { @@ -521,6 +523,45 @@ describe('useMutation Hook', () => { expect(errorMock.mock.calls[0][0]).toMatch("Missing field"); errorMock.mockRestore(); }); + + it(`should not call onError when errorPolicy is 'ignore'`, async () => { + const variables = { + description: 'Get milk!' + }; + + const mocks = [ + { + request: { + query: CREATE_TODO_MUTATION, + variables, + }, + result: { + errors: [new GraphQLError(CREATE_TODO_ERROR)], + }, + } + ]; + + const onError = jest.fn(); + + const { result } = renderHook( + () => useMutation(CREATE_TODO_MUTATION, { errorPolicy: "ignore", onError }), + { wrapper: ({ children }) => ( + + {children} + + )}, + ); + + const createTodo = result.current[0]; + let fetchResult: any; + await act(async () => { + fetchResult = await createTodo({ variables }); + }); + + expect(fetchResult).toEqual({}); + expect(onError).not.toHaveBeenCalled(); + + }) }); it('should return the current client instance in the result object', async () => { diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts index 8aaf22aa5a0..00d8fa3ed18 100644 --- a/src/react/hooks/useMutation.ts +++ b/src/react/hooks/useMutation.ts @@ -112,7 +112,10 @@ export function useMutation< } const onCompleted = executeOptions.onCompleted || ref.current.options?.onCompleted - onCompleted?.(response.data!, clientOptions); + + if (!error) { + onCompleted?.(response.data!, clientOptions); + } return response; }).catch((error) => { From f4c146dfc06c5b1f66de517c214d235d84067378 Mon Sep 17 00:00:00 2001 From: Cayla Hamann Date: Wed, 26 Jul 2023 16:05:15 -0400 Subject: [PATCH 3/8] chore: add changeset for bug fix --- .changeset/early-pumpkins-type.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/early-pumpkins-type.md diff --git a/.changeset/early-pumpkins-type.md b/.changeset/early-pumpkins-type.md new file mode 100644 index 00000000000..aa2cfd106f6 --- /dev/null +++ b/.changeset/early-pumpkins-type.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fixes a bug in `useMutation` so that `onError` is called when `errorPolicy` is set to 'all' From 3e2b3c5e320881725b0abe0efa40b366bd57e76a Mon Sep 17 00:00:00 2001 From: Cayla Hamann <38332422+caylahamann@users.noreply.github.com> Date: Thu, 27 Jul 2023 08:49:05 -0400 Subject: [PATCH 4/8] Update .changeset/early-pumpkins-type.md Co-authored-by: Jerel Miller --- .changeset/early-pumpkins-type.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/early-pumpkins-type.md b/.changeset/early-pumpkins-type.md index aa2cfd106f6..d706a6d1f68 100644 --- a/.changeset/early-pumpkins-type.md +++ b/.changeset/early-pumpkins-type.md @@ -2,4 +2,4 @@ "@apollo/client": patch --- -Fixes a bug in `useMutation` so that `onError` is called when `errorPolicy` is set to 'all' +Fixes a bug in `useMutation` so that `onError` is called when an error is returned from the request with `errorPolicy` set to 'all' . From f8ce693ba795f6975d1d57e684a93f7b4abd817c Mon Sep 17 00:00:00 2001 From: Cayla Hamann <38332422+caylahamann@users.noreply.github.com> Date: Thu, 27 Jul 2023 08:49:21 -0400 Subject: [PATCH 5/8] Update src/react/hooks/__tests__/useMutation.test.tsx Co-authored-by: Jerel Miller --- src/react/hooks/__tests__/useMutation.test.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index db2e453b0b8..e2710684c2e 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -560,8 +560,7 @@ describe('useMutation Hook', () => { expect(fetchResult).toEqual({}); expect(onError).not.toHaveBeenCalled(); - - }) + }); }); it('should return the current client instance in the result object', async () => { From 7d1c9779a80bee79df128b4124595415c06c9b49 Mon Sep 17 00:00:00 2001 From: Cayla Hamann <38332422+caylahamann@users.noreply.github.com> Date: Thu, 27 Jul 2023 08:49:28 -0400 Subject: [PATCH 6/8] Update src/react/hooks/__tests__/useMutation.test.tsx Co-authored-by: Jerel Miller --- src/react/hooks/__tests__/useMutation.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index e2710684c2e..27beabbd985 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -482,7 +482,7 @@ describe('useMutation Hook', () => { expect(onError).toHaveBeenCalledTimes(1); expect(onError.mock.calls[0][0].message).toBe(CREATE_TODO_ERROR); expect(onCompleted).not.toHaveBeenCalled(); - }) + }); it(`should ignore errors when errorPolicy is 'ignore'`, async () => { const errorMock = jest.spyOn(console, "error") From 222165169a29e79d0ed472270a78b641d02c3b97 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Thu, 27 Jul 2023 17:23:04 -0600 Subject: [PATCH 7/8] Update size-limit --- .size-limit.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.cjs b/.size-limit.cjs index 9de3cde6686..54cacceb903 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -1,7 +1,7 @@ const checks = [ { path: "dist/apollo-client.min.cjs", - limit: "38000" + limit: "38042" }, { path: "dist/main.cjs", From b383d170fc5b8c7e7de8525cd77216dd81497dcf Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Thu, 27 Jul 2023 17:25:10 -0600 Subject: [PATCH 8/8] Remove unneeded check for errorPolicy when calling onError --- src/react/hooks/useMutation.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts index 1f148deedff..f2ecd587b7e 100644 --- a/src/react/hooks/useMutation.ts +++ b/src/react/hooks/useMutation.ts @@ -88,11 +88,9 @@ export function useMutation< ? new ApolloError({ graphQLErrors: errors }) : void 0; - const errorPolicy = clientOptions.errorPolicy; - const onError = executeOptions.onError || ref.current.options?.onError - if (error && errorPolicy === 'all' && onError) { + if (error && onError) { onError(error, clientOptions); }