Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query onCompleted callback is not called when the query data updates due to a cache update #6373

Open
dobesv opened this issue Jun 1, 2020 · 8 comments

Comments

@dobesv
Copy link
Contributor

dobesv commented Jun 1, 2020

When trying to upgrade from react-apollo 2.5.8 to 3.1.5 we found that the behavior of the onCompleted callback on the Query component had changed in a breaking manner.

Intended outcome:

A Query components onCompleted callback should be called any time the data for that Query changes, even if the change is made by another operation such as a mutation or a cache update.

Actual outcome:

The onCompleted callback might not be called if changes are made to the cache that affect the query data.

How to reproduce the issue:

I made a reproduction of this behavior in this branch here:

https://github.com/dobesv/react-apollo-error-template/tree/query-oncomplete-bug

If you open the console you will see that the onCompleted function logs only on the initial load, if you click the button it does not log that any more.

If you rollback to the older version using npm i [email protected] and restart the app, clicking the button logs onCompleted on every click.

Versions

  System:
    OS: Linux 5.0 Linux Mint 19 (Tara)
  Binaries:
    Node: 12.13.1 - ~/.nvm/versions/node/v12.13.1/bin/node
    Yarn: 1.19.0 - /usr/bin/yarn
    npm: 6.12.1 - ~/.nvm/versions/node/v12.13.1/bin/npm
  Browsers:
    Chrome: 77.0.3865.90
    Firefox: 76.0
  npmPackages:
    apollo-cache-inmemory: ^1.6.6 => 1.6.6 
    apollo-client: 2.6.10 => 2.6.10 
    react-apollo: 3.1.5 => 3.1.5 

Workaround

Instead of using onCompleted to detect changes, you could do the same code in the children render prop. This might be called more often than onCompleted would have been called, but in many cases it probably won't be a serious performance concern. Note that if you use setState in your handler you may have to add a setTimeout and make sure you do not call setState again if the value hasn't changed, to avoid infinite setState -> render loops.

@unutoiul
Copy link

unutoiul commented Jun 1, 2020

same issue heree

@dobesv
Copy link
Contributor Author

dobesv commented Jun 1, 2020

@unutoiul Can you use the "emoji" button to add a "thumbs up" instead of adding a "me too" comment? That seems to be the preferred etiquette for "me too" on github these days, and makes it easier for people to count up the responses.

@unutoiul

This comment has been minimized.

@andrewnaeve
Copy link

andrewnaeve commented Aug 14, 2020

@dobesv Can you confirm this codesandbox is a repro of the issue you're experiencing?

https://codesandbox.io/s/compassionate-golick-5042m?file=/src/App.tsx

@dobesv
Copy link
Contributor Author

dobesv commented Aug 14, 2020

@andrewnaeve It looks that way to me, yes.

@natalieytan
Copy link

natalieytan commented Nov 10, 2020

Is it possible to confirm if this is a bug or intended behaviour?

On the other hand I am investigating how to perform a side-effect on the first successful network call, and not on subsequent cache updates.

If this is the intended behaviour of onComplete, it looks like it would suit my purposes.

If it is not the intended behaviour (and the behaviour will change on version bump), I will have to find alternatives.

@salazarm
Copy link

salazarm commented Mar 1, 2024

Is this still the expected behavior?

@dobesv
Copy link
Contributor Author

dobesv commented Nov 4, 2024

Maybe related: #11526

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants