fix: useMutation calls onError if errorPolicy is all #11103
Conversation
|
@caylahamann: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
|
| Name | Link |
|---|---|
| 🔨 Latest commit | 33d4e17 |
🦋 Changeset detectedLatest commit: b383d17 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
We've tweaked the file size checks in our release-3.8 branch, so feel free to ignore that failed check for now. We can get that updated once we've merged that branch into main 🙂. |
There was a problem hiding this comment.
Thank you sooooo much for fixing this! Funny that we called this out so explicitly in our docs, but this had been a bug for so long 😆. I had some minor suggestions for you if you'd like to take a look.
As for releasing this, I'll check to see if we want to get anything else in for our 3.8.0 release, otherwise we'll make this a fast-follow in 3.8.1. Thanks so much for the contribution!
| 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(); |
| @@ -479,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 () => { | |||
There was a problem hiding this comment.
Thanks so much for adding this test!
src/react/hooks/useMutation.ts
Outdated
|
|
||
| const onError = executeOptions.onError || ref.current.options?.onError | ||
|
|
||
| if (error && errorPolicy === 'all' && onError) { |
There was a problem hiding this comment.
| if (error && errorPolicy === 'all' && onError) { | |
| if (error && onError) { |
I believe some of the internals of Apollo Client (QueryManager in particular) will remove the error property from the result for you when errorPolicy is set to ignore, so the explicit check is not needed. Check to see if you're able to remove this check and maintain passing tests.
There was a problem hiding this comment.
Confirmed that removing this works as expected 🎉 . I'm going to go ahead and apply it :)
Co-authored-by: Jerel Miller <[email protected]>
Co-authored-by: Jerel Miller <[email protected]>
Co-authored-by: Jerel Miller <[email protected]>
|
@caylahamann we will go ahead and get this released with 3.8.0 🙂. I'm going to repoint this PR to our release branch and merge into that. We should be going live with 3.8 very soon. Thanks so much for fixing this! |
|
This has been bugging us for ages. Thank you. |
Fixes #10973
Fixes a bug in
useMutationso thatonErroris called when an error is returned from the request witherrorPolicyset to 'all'.Checklist: