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

onCompleted is called during render phase, causing React errors (reproduction provided) #9794

Closed
dylanwulf opened this issue Jun 7, 2022 · 5 comments · Fixed by #9801
Closed
Assignees
Labels
🧞 fixed-by-contributor issues where the contributor went the extra mile and fixed the problem with a PR 🛬 fixed-in-prerelease ⚛️ React-related 🌹 has-reproduction
Milestone

Comments

@dylanwulf
Copy link
Contributor

dylanwulf commented Jun 7, 2022

Intended outcome:
There should be no React errors when using apollo client

Actual outcome:
React error is printed to the console:

Warning: Cannot update a component (`App`) while rendering a different component (`PeopleList`). To locate the bad setState() call inside `PeopleList`, follow the stack trace as described in https://reactjs.org/link/setstate-in-render

The cause of this seems to be that onCompleted gets called during the render phase if the query results are available in the cache. This can cause problems when doing something like dispatching a redux action from inside the onCompleted function.

How to reproduce the issue:
Clone and run my reproduction, using branch react-console-errors: https://github.com/dylanwulf/react-apollo-error-template/tree/react-console-errors. Install packages with npm install and run the app with npm start. When running the app, wait until "parent query completed" is true, then click the button and watch the console log.

Versions

  System:
    OS: Windows 10 10.0.22000 (envinfo doesn't seem to detect this properly, I'm actually running Windows 11)
  Binaries:
    Node: 16.15.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.5.5 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (102.0.1245.33)
  npmPackages:
    @apollo/client: ^3.6.6 => 3.6.6
dylanwulf added a commit to dylanwulf/apollo-client that referenced this issue Jun 8, 2022
…f the callback functions.

Fix broken tests and add new test for the issue.
@benjamn benjamn self-assigned this Jun 8, 2022
@benjamn benjamn added this to the v3.6.x patch releases milestone Jun 8, 2022
@dylanwulf
Copy link
Contributor Author

Submitted a PR which should fix this: #9801

@simplecommerce
Copy link

@dylanwulf thanks for this, I believe I am having the same issue with onCompleted but with useMutation.

I took your repo and did some modifications to test it with useMutation

https://github.com/simplecommerce/react-apollo-error-template/tree/mutation-onCompleted-error

Could you please confirm if this is the same problem?

In my actual project in order to bypass this, I had to use mutation().then(() => instead of onCompleted to not have the error.

@dylanwulf
Copy link
Contributor Author

@simplecommerce I can confirm that the reproduction you provided does get fixed with the PR I submitted

dylanwulf added a commit to dylanwulf/apollo-client that referenced this issue Jun 9, 2022
…f the callback functions.

Fix broken tests and add new test for the issue.
dylanwulf added a commit to dylanwulf/apollo-client that referenced this issue Jun 10, 2022
…f the callback functions.

Fix broken tests and add new test for the issue.
dylanwulf added a commit to dylanwulf/apollo-client that referenced this issue Jun 10, 2022
…f the callback functions.

Fix broken tests and add new test for the issue.
dylanwulf added a commit to dylanwulf/apollo-client that referenced this issue Jun 14, 2022
…f the callback functions.

Fix broken tests and add new test for the issue.
benjamn pushed a commit to dylanwulf/apollo-client that referenced this issue Jun 27, 2022
…f the callback functions.

Fix broken tests and add new test for the issue.
@benjamn benjamn added 🛬 fixed-in-prerelease 🧞 fixed-by-contributor issues where the contributor went the extra mile and fixed the problem with a PR ⚛️ React-related labels Jun 27, 2022
@kaufmann42
Copy link

What is the order of which onCompleted is called? Is it called sync with updating data? Or is it waited to be completed before any state updates are finished?

@dylanwulf
Copy link
Contributor Author

Fixed by #9801!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧞 fixed-by-contributor issues where the contributor went the extra mile and fixed the problem with a PR 🛬 fixed-in-prerelease ⚛️ React-related 🌹 has-reproduction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants