-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Deduplicate the "Can only update a mounted or mounting component" warning #11140
Comments
@gaearon Sure .. Would be happy to take it up ! |
Actually @gaearon, do you want me to do a full check on all the warnings? It'll be better than raising individual issues. |
Yea that would be amazing. I don't think they necessarily always have to be deduped... but it might be easier to discusss specific cases where they shouldn’t, and convert the rest. |
So, any pointers on how I can go about identifying those specific cases that don't need to be deduped? @gaearon |
I don't have any clear guideline. I think the most useful one is: does the warning happen during render. If it does then it's likely it'll happen again and again and again, and spam the console. Then it should be deduped. Whereas if the warning itself can only happen when user calls some specific API, and that API is typically called outside of render phase, it might not make sense to dedupe. |
I found 2 versions of the "Can only update a mounted or mounting component" warning:
The second one is from ReactNoopUpdateQueue.js (which is imported by ReactBaseClasses.js)
@gaearon Just have a few doubts:
Thanks in advance :) |
The one in Let’s dedupe both I think. This means that in the very unlikely case both fire, there will be at most two warnings rather than one. But that’s okay. |
* Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext()
Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext()
There were totally ~120 warnings in the React codebase. I checked them all and came up with a list of ~20 warnings which were not deduped. I have fixed the ones which I felt will be duplicated during re-render(as per your suggestion) and have given the PR. I still have a list of other warnings which I am unsure if they need deduplication or not. I can share the list if you wish to take a look at it. Apologies for the delay in giving the PR. @gaearon |
Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext()
Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext()
* Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext() * Code review changes made for facebook#11140
* Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext() * Code review changes made for facebook#11140
* Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext() * Code review changes made for facebook#11140
* Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext() * Code review changes made for facebook#11140
* Deduplicated many warnings (#11140) * Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext() * Code review changes made for #11140 * Minor style fix * Test deduplication for noop updates in server renderer * Test deduplication for cWRP warning * Test deduplication for cWM setState warning * Test deduplication for unnmounted setState warning * Fix existing Flow typing * Test deduplication for invalid updates * Test deduplication of update-in-updater warning
This was fixed. |
* Deduplicated many warnings (facebook#11140) * Deduplicated the following warnings: 1. Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op 2. %s.componentWillReceiveProps(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.' 3. An update (setState, replaceState, or forceUpdate) was scheduled from inside an update function. Update functions should be pure, with zero side-effects. Consider using componentDidUpdate or a callback. 4. setState(...): Cannot call setState() inside getChildContext() * Code review changes made for facebook#11140 * Minor style fix * Test deduplication for noop updates in server renderer * Test deduplication for cWRP warning * Test deduplication for cWM setState warning * Test deduplication for unnmounted setState warning * Fix existing Flow typing * Test deduplication for invalid updates * Test deduplication of update-in-updater warning
I getting this Error in console, I don't know why
|
Note: @anushreesubramani is working on this, please don’t send PRs if you aren’t her :-)
Similar to #11081.
@anushreesubramani Wanna take this one as well? It would need to deduplicate based on owner/stack info, similar to how #11120 works.
The text was updated successfully, but these errors were encountered: