React Warning with v16.13: Cannot update a component while rendering a different component, bad setState() call #21049
Comments
|
@nerrad Any thoughts on this warning in |
|
Thanks for documenting this @ocean90. I followed the steps to swap in React v16.13. I also see the warning when I just select an existing image block (the first time after loading the editor...appears it doesn't happen on subsequent selects). It's clear this needs to be investigated. I'm not sure offhand yet how severe this is. |
|
I spent a bit of time trying to debug this, but I'm a bit stumped. It looks like the The one thing I was able to verify, is that if I change this code so that I also discovered that if I wait a few seconds before selecting the image on initial editor load, there is no warning when I select the image. So that gives further credence to the race condition theory here. I'm suspecting this timeline:
|
|
cc @aduth and @youknowriad might need your input on this as well. If the above timeline is correct, this could be problematic when we bump the React version to React 16.3+ |
|
Created this PR #21289 to allow us to investigate this better. |
|
Can we reproduce this issue elsewhere? I mean the image block is a big one, it would be good if we find ways to reproduce this in a more contained environment. |
|
Seems like this is a related PR on a similar library https://github.com/dai-shi/react-tracked/pull/42/files |
|
Digging a little bit, it seems the I found that wrapping I wonder where this rabbit hole leads :P |
You mean in Gutenberg or a React app which just uses the |
|
Oooh, I've been keeping an eye on this one. It's something that came up as React is getting stricter and stricter in preparation for Concurrent Mode (CM). Another thing that we will need to take care of later is tearing. This is when state changes while a render is interrupted and the continuation of the render uses different values. It's even harder to get right for Third-party library authors are going crazy in trying to get their implementations to work: The React team published an external There are still competing RFCs, though, e.g., there are talks on having Context support selectors so that it can replace all of these performance hacks. That would circumvent tearing because as long as updates are not triggered in renders, React makes sure that a paused render continues with cached context values. Jumping to any one of these solutions would be premature and send us down a costly rabbit hole. This specific issue is flaring up because of selectors synchronously triggering resolvers, which in turn synchronously trigger store subscriptions. I think that it makes a lot of sense for resolvers to trigger asynchronously. I pushed that to #21289, and the warning is gone. I still think we should hold off on upgrading React until they stabilize a proper API for this, and we can refactor |
|
I faced something similar when I tried to bind onClick on Parent component loop instead of passing method as prop to lift up state. I solved it by:``` //parent component props.markSelectedhandler(props.contact.userId)}>
```
Posting this here just in case someone else faces this issue, I get that context is different(wordpress in current thread) but error message is exactly same so if any one comes across this while googling they may find this helpful. |
|
I have similar error, different project, an workaround that I finded is call setState into a setTimeout function error: |
|
@mayanktiwaridotcom estuve probando con tu ejemplo pero lo que hace no es renderizar el componente sino hasta darle el onClick. seguire investigando sobre este error. |
|
I had also met this errors when using useReducer hook in a component, I deliver the dispatch function to the child component, then I called the dispatch to change the status of ancestor's,then the erros occured. I am confused and don't how to fix the errors. |
|
So I've been battling with this and e2e test issues and I have a better understand understanding of the problems we have: In WordPress Data, running selectors does this now (pseudo code) resolveSelector(args) {
if ( resolverHasBeenAlreadyTriggeredForArgs(args) ) {
return;
}
markSelectorAsTriggeredForArgs(args);
runResolverForArgs(args)
}
runSelector( args ) {
resolveSelector( args );
return getSelectorValue( args );
}Notice that above both A naive fix is to: resolveSelector(args) {
if ( resolverHasBeenAlreadyTriggeredForArgs(args) ) {
return;
}
delay( () => {
markSelectorAsTriggeredForArgs(args);
runResolverForArgs(args)
} );
}so now the resolution is delayed and the React warning is gone but there's another issue here. It is possible that resolvers get called multiple times unnecessarily. This happens because when So for me, the solution requires making the "resolution state" independent from the "rendering state" in order to be able to do this without triggering warnings resolveSelector(args) {
if ( resolverHasBeenAlreadyTriggeredForArgs(args) ) {
return;
}
// This is synchronous now.
markSelectorAsTriggeredForArgs(args);
delay( () => {
runResolverForArgs(args)
} );
}But there's another issue, several components rely on these resolution selectors to show spinners etc... which makes me think, maybe we need "two resolutions states", one internal to the registry and one to be used in components so we'd end up with something like: resolveSelector(args) {
if ( resolverHasBeenAlreadyTriggeredForArgs(args) ) {
return;
}
// This is synchronous now.
markSelectorAsTriggeredForArgs(args);
delay( () => {
markSelectorAsTriggeredForArgsInState(args);
runResolverForArgs(args)
} );
}Anyway, this is starting to get complex and I appreciate toughts and ideas here. cc @nerrad @epiqueras |
|
Yeah, that's basically what I said above:
|
|
But it's not as easy as making the resolvers async, the hasStarted thing need to be sync. |
|
I did that too. |
|
@epiqueras where can I see that? In the PR, it was all wrapped in setTimeout including the "startResolution" call. |
|
There's a local variable that updates synchronously to avoid duplicate calls. |
|
But that local variable doesn't depend on "args" which means it's going to create issues where potential resolvers won't run at all? |
|
Yes, I guess it needs to be a map that uses args. |
Describe the bug
React 16.13.0 introduced a warning for when a function component is updated during another component's render phase (facebook/react#17099). In version 16.13.1 the warning was adjusted to be more specific (facebook/react#18330).
The warning looks like this:
Such a warning is currently triggered when using the
useSelect()hook from the@wordpress/datapackage in a React app with v16.13.x.In my app I have three components which all retrieve the data from the store through the
useSelect()hook which now each log such a warning.The warning seems to get triggered by onStoreChange.
To reproduce
I was able to get the warning with Gutenberg as well.
gutenberg_register_vendor_scripts()and add the following lines:npm iandnpm run devError Message
Error Stack
Expected behavior
No warning should be triggered.
Desktop (please complete the following information):
Additional context
The text was updated successfully, but these errors were encountered: