fix(regression): avoid calling useQuery onCompleted for cache writes

[alessbell]

There are a few related issues pointing to a regression that occurred between v3.4.17 and v3.5.0 and greater: writes to the cache (it doesn't matter how the cache update occurs: directly via cache.writeQuery, via a separate useMutation call, etc.) trigger the onCompleted callback passed to useQuery, whereas in v3.4.x, onCompleted was triggered when the initial network request completed, and not on subsequent cache writes.

• cache-and-network: No way to determine if the network request has completed #10059
• The onCompleted callback of useQuery executes from cache writes. #10076
• The onCompleted callback of the useQuery hook executes on data mutations #9688

My proposed solution here is to compare the previous and current result's loading states: if the loading state doesn't change, onCompleted won't re-fire. This approach has the benefit of working as expected with the notifyOnNetworkStatusChange option which will cause onCompleted to fire after subsequent network requests if notifyOnNetworkStatusChange is set to true.

NB: this change would mean that both direct cache updates to the watched selection set and updates as a result of other network requests (beyond the useQuery hook invocation in question) would never cause onCompleted to fire. This seems more intuitive to me.

refetch and pollInterval

Calls to refetch and queries with pollInterval configured work the same way: they don't update the loading state unless notifyOnNetworkStatusChange is true. If we'd like to preserve the current behavior whereby polling queries fire onChange when watched cache data changes without updating loading/networkStatus, we can keep the condition in b521d12#diff-fb7fa652de6d84c08b87344a06a4d120b664c6e8bf491d16575794c755141477R517. I'm in favor of keeping onCompleted execution consistent across the board, but would love input on this.

There also seems to be some confusion re: polling and onCompleted: looking at this issue, the callback is not repeatedly firing because the cache never changes. #5531

Checklist:

• If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests

[alessbell]

Thanks @benjamn! I've added that condition in e763b92. Curiously, while the tests are still passing locally they are now consistently failing on CI... wondering if our CI runners are under heavy load or there's some other reason tests would suddenly start failing (it's even a test added by this PR that is consistently failing now, but was consistently passing before).

[alessbell]

Welp... tests are green now. Ready for a final pass 🙇‍♀️

[benjamn]

Thanks @alessbell!

[mogelbrod]

@alessbell: Given that this PR has been merged into v3.8 - is it correct that the intended behaviour of onCompleted() is to only be called when a request for the associated query has responded with different data than what's in the cache? If that's the case, would it be possible to also expose another callback like onResponse() that gets called whenever a response for a request triggered by the hook is returned, no matter if the data has changed or not? Otherwise there doesn't seem to be any reliable way to react to refetches/polling updates without opting in to having the component also get re-rendered on network status (which can have significant negative performance impact). I'd be happy to open up an issue for this if relevant.

(following up on #5531 (comment))