There is a breaking change in v3.7.4

[ArianHamdi]

Issue Description

Prefer the onError and onCompleted callback functions passed to the execute function returned from useMutation instead of calling both callback handlers.

The change broke my whole app since I rely on both callbacks calling.

I think previous behavior was better than now, usually I like to create custom hooks to handle API calls and do something related to that API such as updating cache in the hook level and closing a modal after an API call in the mutate function. In this model my custom hook is reusable and reuse api related logic and do things in addition to this whenever necessary in mutate function.

Keeping the current behavior (which I really don't like) should be a major version update rather than a patch update.

Link to Reproduction

#10425

Reproduction Steps

If you upgrade from version 3.7.3 to version 3.7.4,
you can check that onCompleted in the hook level and onCompleted in the mutate function are not called together.

[bignimbus]

Hi @ArianHamdi 👋🏻 thanks for opening this issue! I totally understand the frustration here and I want to acknowledge that longstanding bugs like this can invite an implicit expectation of stability. And it's totally our responsibility that the bug fixed by #10425 existed in the first place 😞

Here’s how I think about the issue: our documentation specifies what the correct behavior is. The library was not conforming to that specification, so there was unambiguously a bug to be patched. That seems well within the scope of a patch release. It seems counterintuitive to ask users to wait for a major version release so we can preserve functionality that contradicts the public API documentation. That’s just my two cents though, I hope it makes sense. And I’m happy to keep this issue open in case there are additional thoughts here 🙏🏻

[henryqdineen]

our documentation specifies what the correct behavior is. The library was not conforming to that specification, so there was unambiguously a bug to be patched.

I am super grateful for everyone's work on Apollo Client but I disagree with this strategy. We have run into subtle breaking changes like this with when trying to upgrade Apollo Client in the past and it can cause protracted and complicated migrations. We might be a unique case because we have hundreds of apps using Apollo Client all on the same version. I don't know if this specific issue is affecting us but in general would prefer not to have any similar behavior changes.

[ArianHamdi]

Hi @bignimbus thanks for your response
It would be useful to have the option to call both callbacks.

The change was strange to me because I didn't want to upgrade Apollo client to a new version, I just had a problem with my workflow and tried to reinstall my packages. It automatically installed the latest patch level version since I included Apollo client in the patch level update. As a result, it broke my app (I was really having a hard time figuring out what was going on 😄).

I can't be convinced this is a patch level update and now I locked my apollo client to exact version.

[jerelmiller]

Hey @henryqdineen and @ArianHamdi 👋

I definitely hear your frustration. I'll admit, as a maintainer of such a popular library, it can be difficult at times to try and weigh all the feedback and make the right decision, especially given conflicting advice on what others believe is the "correct" behavior or not. In one case, one might view a particular behavior as buggy or incomplete, while another has come to rely on the "buggy" behavior as a feature of the library. No matter what decision we make, someone will be unhappy, and that's just the unfortunate part about writing a library that needs to cater to thousands of use cases.

In this instance, we received feedback that calling the callbacks twice were viewed as a bug and/or surprising behavior (#10287, #7167 (comment)). I tend to use our docs when trying to decide what the correct behavior should be, given this is how we are communicating our APIs and behaviors to our users. According to our docs for useMutation's mutate function:

Any option you pass here overrides any existing value for that option that you passed to useMutation.

Seeing as we communicate that mutate is meant to override options passed to the useMutation hook, I opted to view this as a bug. As such, I view this less as a breaking change and more of a bug fix, which fits the description of a patch release.

Unfortunately it seems on your end, you came to rely on this buggy behavior as a feature of the library. I do apologize this resulted in a bit of churn on your end. I had expected this change to make minimal impact on existing users.

As for your request:

It would be useful to have the option to call both callbacks.

I can't guarantee we will an option for this, though we will consider it. We already have quite a few options that can be passed to our hooks, so we are pretty deliberate when deciding when to add new options, which must be motivated by solid use cases. For now, I'd make a bet this is likely something we won't add.

---

As you've done, I'd recommend pinning your Apollo Client version to an exact version until you've had a chance to migrate away from the old behavior. I'd also highly recommend to pay close attention to our docs as much as possible as we the maintainers tend to use that as our "tie breaker" in cases where we are trying to decide the correct behavior.

If there are ever cases in our documentation that feel incomplete or ambiguous, please please please feel free to open issues for us so we can address them. We want our documentation to be as clear as possible so that we run into these types of situtations less and less.

I sincerely appreciate the feedback here!

[jerelmiller]

Hey all 👋

Seeing as there is no actionable feedback to address, I'm going to go ahead and close this issue. Thanks again for bringing this to light! Let us know if you need any more help. Thanks!