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

"Warning: setState" printed after upgrading #509

Closed
NeoPhi opened this Issue Mar 6, 2017 · 7 comments

Comments

Projects
None yet
3 participants
@NeoPhi
Copy link

NeoPhi commented Mar 6, 2017

We recently upgraded our project as follows:

+    "apollo-client": "^1.0.0-rc.0",
-    "apollo-client": "^0.8.1",
+    "react-apollo": "^0.13.2",
-    "react-apollo": "^0.9.0",

After which we started seeing the following warnings in the console:

Warning: setState(...): Cannot update during an existing state transition (such as within `render` or another component's constructor). Render methods should be a pure function of props and state; constructor side-effects are an anti-pattern, but can be moved to `componentWillMount`.

I was able to produce a semi reliable use case in the sample repository react-apollo-error-template-setState. Please note that this seems to be a race condition so it will require clicking the randomize button at random intervals such as after a few full loads and then when there are users but not friends loaded.

@calebmer This feels like it might be related to the issue mentioned in #506

A sample trace related to the warning is as follows:

printWarning (warning.js:36)
warning (warning.js:60)
getInternalInstanceReadyForUpdate (ReactUpdateQueue.js:54)
enqueueSetState (ReactUpdateQueue.js:200)
ReactComponent.setState (ReactComponent.js:63)
GraphQL.forceRenderChildren (graphql.js:293)
next (graphql.js:270)
(anonymous) (apollo.umd.js:2095)
next (apollo.umd.js:2093)
(anonymous) (apollo.umd.js:2551)
(anonymous) (apollo.umd.js:2982)
(anonymous) (apollo.umd.js:2979)
QueryManager.broadcastQueries (apollo.umd.js:2976)
QueryManager.broadcastNewStore (apollo.umd.js:2391)
(anonymous) (apollo.umd.js:3022)
dispatch (applyMiddleware.js:45)
QueryManager.stopQueryInStore (apollo.umd.js:2669)
QueryManager.stopQuery (apollo.umd.js:2799)
ObservableQuery.tearDownQuery (apollo.umd.js:2120)
unsubscribe (apollo.umd.js:2072)
ObservableQueryRecycler.reuse (graphql.js:394)
GraphQL.createQuery (graphql.js:206)
GraphQL.setInitialProps (graphql.js:197)
Apollo(Person) (graphql.js:99)
(anonymous) (ReactCompositeComponent.js:295)
measureLifeCyclePerf (ReactCompositeComponent.js:75)
_constructComponentWithoutOwner (ReactCompositeComponent.js:294)
_constructComponent (ReactCompositeComponent.js:280)
mountComponent (ReactCompositeComponent.js:188)
mountComponent (ReactReconciler.js:46)
updateChildren (ReactChildReconciler.js:121)
_reconcilerUpdateChildren (ReactMultiChild.js:208)
_updateChildren (ReactMultiChild.js:312)
updateChildren (ReactMultiChild.js:299)
_updateDOMChildren (ReactDOMComponent.js:936)
updateComponent (ReactDOMComponent.js:754)
receiveComponent (ReactDOMComponent.js:716)
receiveComponent (ReactReconciler.js:125)
_updateRenderedComponent (ReactCompositeComponent.js:754)
_performComponentUpdate (ReactCompositeComponent.js:724)
updateComponent (ReactCompositeComponent.js:645)
performUpdateIfNecessary (ReactCompositeComponent.js:561)
performUpdateIfNecessary (ReactReconciler.js:157)
runBatchedUpdates (ReactUpdates.js:150)
perform (Transaction.js:140)
perform (Transaction.js:140)
perform (ReactUpdates.js:89)
flushBatchedUpdates (ReactUpdates.js:172)
closeAll (Transaction.js:206)
perform (Transaction.js:153)
batchedUpdates (ReactDefaultBat…Strategy.js:62)
batchedUpdates (ReactUpdates.js:97)
dispatchEvent (ReactEventListener.js:147)

@NeoPhi NeoPhi changed the title "Warning: setState" warning printed after upgrading "Warning: setState" printed after upgrading Mar 6, 2017

@calebmer

This comment has been minimized.

Copy link
Contributor

calebmer commented Mar 6, 2017

I think this is the same bug as #506! Good to have a confirmation and test case. Can you confirm that #506 fixes the problem?

@jonhester

This comment has been minimized.

Copy link

jonhester commented Mar 6, 2017

I have the same issue and #506 did indeed fix it.

@NeoPhi

This comment has been minimized.

Copy link
Author

NeoPhi commented Mar 6, 2017

It looks like #506 fixes the issue for our application as well as the sample application linked to above.

@NeoPhi

This comment has been minimized.

Copy link
Author

NeoPhi commented Mar 6, 2017

I haven't traced through the code completely but it looks like the new reuse option creates a synchronous path through the code such that when in a render of an Apollo based component if a child component is also Apollo based and the system is able to reuse an existing observable, the subscribeToQuery's next handler gets called which triggers forceRenderChildren's setState. By moving setInitialProps outside of the constructor (which #506 does), this synchronous code path is called when a setState call can be legally made.

@calebmer

This comment has been minimized.

Copy link
Contributor

calebmer commented Mar 6, 2017

Yep. This makes sense @NeoPhi. Thanks so much for investigating 😊

I think we will merge #506

@calebmer

This comment has been minimized.

Copy link
Contributor

calebmer commented Mar 7, 2017

The fix will be included in the next release 👍

@calebmer calebmer closed this Mar 7, 2017

@calebmer

This comment has been minimized.

Copy link
Contributor

calebmer commented Mar 7, 2017

Released in 0.13.3 🎉

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