apollo-client icon indicating copy to clipboard operation
apollo-client copied to clipboard

The `onCompleted` callback of `useQuery` executes from cache writes.

Open lawrence-yu opened this issue 1 year ago • 3 comments

Intended outcome:

The onCompleted option provided to useQuery should not be called after data was changed from an update via cache.writeQuery (where it was just a local cache update without a network request).

Actual outcome:

When using cache.writeQuery to update local data, the onCompleted option of a useQuery for the same query gets called. This is a change from what we see on 3.4.x, where the onCompleted callback is not called after these cache updates.

How to reproduce the issue:

https://github.com/lawrence-yu/react-apollo-error-onCompleted

  1. After having it running locally, click on the button with "Update person's name via writeQuery". This does a cache write to append a 1 to a person's name, which is reflected in UI with the list of names.
  2. In 3.6.8, note that the "onCompleted count" increases with each button click / cache write.
    1. There's a 3.4.17 branch in the fork that can be used to see that, with each button click, the onCompleted count remains at 1 (the one being from after initial query completion).

Versions

 System:
    OS: Windows 10 10.0.19042
  Binaries:
    Node: 16.13.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.18 - ~\.yarn\bin\yarn.CMD
    npm: 8.1.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (105.0.1343.27)
  npmPackages:
    @apollo/client: ^3.6.8 => 3.6.8

Other notes

The limited documentation for the onCompleted option leaves me unsure about what is the "correct" behavior. Is getting called by cache updates actually correct now? Or was the old behavior correct and this is an actual bug?

Potentially related to: #9688 #10059

lawrence-yu avatar Sep 06 '22 21:09 lawrence-yu

As you suspected, this is very likely the same underlying issue as https://github.com/apollographql/apollo-client/issues/10059.

srmagura avatar Sep 08 '22 17:09 srmagura

Hi @bignimbus, I'm quite confused about your comment and how it relates to my issue. Is it possible your comment/issue-close here was meant for a different issue?

I see you had a recent docs PR here, https://github.com/apollographql/apollo-client/pull/10066/files, but with that I still don't see how it relates to my ask here.

lawrence-yu avatar Sep 15 '22 21:09 lawrence-yu

You're quite right @lawrence-yu, sorry for the confusion! I've deleted the comment and will repost it where it belongs.

bignimbus avatar Sep 15 '22 21:09 bignimbus

Seems we have the same error:

  • we call query on a button click via useLazyQuery that lazily checks GQL operation by querying operation field using network-only fetch policy.
  • if query is completed successfully and the operation is enabled, we call mutation that updates data and returns the same operation field. Operation is disabled in this case, because we updated the data.
  • after the mutation is finished and its result is written to the cache, onCompleted callback of query is triggered automatically

sanex3339 avatar Oct 07 '22 07:10 sanex3339

Same problem here, And Im using Apollo 3.6.9

eveevans avatar Oct 20 '22 22:10 eveevans

Thanks for the reproduction, @lawrence-yu!

https://github.com/apollographql/apollo-client/pull/10229 is open with the fix, I'll update this issue again when it's merged and released 🙇‍♀️

alessbell avatar Oct 26 '22 18:10 alessbell