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

React 16 : componentDidUpdate Warning: Scheduled a cascading update #834

Closed
gontard opened this issue Dec 2, 2017 · 38 comments
Closed

React 16 : componentDidUpdate Warning: Scheduled a cascading update #834

gontard opened this issue Dec 2, 2017 · 38 comments

Comments

@gontard
Copy link

gontard commented Dec 2, 2017

I profiled the performance of my application using react redux by following this article by Ben Schwarz.

In the user timing section, i get these warnings (with a no entry sign):

There is two messages:

  • (Committing Changes) Warning: Lifecycle hook scheduled a cascading update
  • Connect(MyComponent).componentDidUpdate Warning: Scheduled a cascading update

I made some search but i found nothing special. It seems related to the componentDidUpdate function of the connect HOC of react-redux.

What does these messages means ?

@timdorr
Copy link
Member

timdorr commented Dec 3, 2017

It's likely a side effect of how we do subscriptions. But it's nothing to worry about.

@timdorr timdorr closed this as completed Dec 3, 2017
@Iliyass
Copy link

Iliyass commented Dec 7, 2017

@gontard Did you get any useful info on these warnings ? I got the same

@gontard
Copy link
Author

gontard commented Dec 7, 2017

@Iliyass no. I opened this issue, asked a question on stackoverflow and even chatted about this in the #react-internals chan of reactiflux on discord. But i did not get more details.

Since @timdorr said to not worry about this so I did not look further

I think it might a new warning of React 16 and in this case a false positive.

@gaearon
Copy link
Contributor

gaearon commented Dec 7, 2017

Eh. Really unfortunate that this approach causes people to learn to ignore an otherwise valid and useful warning. Hopefully we can figure out a solution with the new context API soon. :-)

@markerikson
Copy link
Contributor

@gaearon , @acdlite : can either of you give some additional info on what the warning actually means, both in terms of the cause and resulting behavior?

@Amareis
Copy link

Amareis commented Dec 15, 2017

image
That is picture from chrome dev tools, react profiling. Seems to this issue can significantly increase render time, just look at the gap between two render cycles. Can we fix it somehow?

@Amareis
Copy link

Amareis commented Dec 15, 2017

Okay, after some researching, I fix it in fork, https://github.com/Amareis/react-redux/commit/1303cb5f18d52b5b9509af1420fefa45f6538b94 there is commit. May be i broke something, i dunno, but at least it works as expected. I can made PR if needed.

@timdorr
Copy link
Member

timdorr commented Dec 15, 2017

A PR would be awesome.

@Amareis
Copy link

Amareis commented Dec 15, 2017

On the next week I'll improve my solution and will make some PR.

@gaearon
Copy link
Contributor

gaearon commented Dec 15, 2017

can either of you give some additional info on what the warning actually means, both in terms of the cause and resulting behavior?

It means what it says: you scheduled a cascading update. "Cascading" means an update inside an update. React first reconciles the tree by calling render, then commits DOM changes, then calls lifecycle hooks. If you call setState in componentDidMount, you are causing React to repeat the cycle: reconcile the tree, commit DOM changes, and call the hooks. This is wasteful.

@markerikson
Copy link
Contributor

@Amareis : it would also be helpful if you could put together an example project that demonstrates the behavior, and some benchmarks that compare the behavior before and after, and in React 15 vs React 16.

@jimbolla
Copy link
Contributor

@Amareis Keep in mind that the cascading updates was a solution to the problem with connected child components getting newest state before they got new props from their connected parent components. I believe the test for that is should pass state consistently to mapState.

@Amareis
Copy link

Amareis commented Dec 15, 2017

Ok, I look into this.

@Amareis
Copy link

Amareis commented Dec 18, 2017

I start my investigation. Seems to this issue don't appears on simple samples, i.e. https://github.com/notrab/create-react-app-redux, need to something more complicated.

@Amareis
Copy link

Amareis commented Dec 18, 2017

Okay, in my case it behavior was caused by little quick'n'dirty hack - pseudo-selector, which returned data from local storage without any memoization, so objects weren't equals by ===. Without that hack all works as expected and no cascading updates are called.

But I have a simple question. How we can prevent this errors? I bet that @gontard have similar issue it his code, but how he can track this issue without reducing code in search of bad string? For example, why-did-you-update don't complains about this issue (because of new render scheme in react 16, may be?..)

@gontard
Copy link
Author

gontard commented Dec 18, 2017

@Amareis Thanks for your investigation. I thought that it was an issue in react-redux but if it doesn't happen in simple case it might be a problem on my side.

Can you give me more details on your "quick'n'dirty hack - pseudo-selector" ?

@gontard
Copy link
Author

gontard commented Dec 18, 2017

@gaearon when you wrote:

If you call setState in componentDidMount, you are causing React to repeat the cycle: reconcile the tree, commit DOM changes, and call the hooks. This is wasteful.

Does that means that there is an issue in the connect HOC of react-redux ? (Connect(MyComponent).componentDidUpdate Warning: Scheduled a cascading update)

@Amareis
Copy link

Amareis commented Dec 18, 2017

@gontard it's pretty easy. Something in your connect(SelectionRectangleContext) props is refreshed too often. Check mapStateToProps of this component, some selector must be memoized to avoid this warning. I don't show my code because it's don't very relate to issue.

@gontard
Copy link
Author

gontard commented Dec 18, 2017

@Amareis I did not found a similar problem with my selectors.

But it seems to be an issue with nested connected component. When i have only a parent connected component, i did not get the warning. But when i add a child connected component, the warning appears.

It confirms what @jimbolla said:

Keep in mind that the cascading updates was a solution to the problem with connected child components getting newest state before they got new props from their connected parent components

@liorbrauer
Copy link

@jimbolla @gaearon So if I understand this thread correctly, and this warnings pops up when you have nested connected components (that's the case in my code) - is there any way to "fix" these warnings and the possible performance hit of the cascading udpates?

@gontard
Copy link
Author

gontard commented Jan 5, 2018

@liorbrauer do you have the same problem ?

I am trying to create a minimal application to reproduce the issue. But i did not succeeded yet.

@Amareis
Copy link

Amareis commented Jan 5, 2018

I think, this issue is appearing only in really deep nested connected components, and it's related to setState method. May be, if setState can be computed before next animation frame, there is no cascading update, and if we have some fat application, which cannot recompute new state before next frame, it schedules it later?

@liorbrauer
Copy link

liorbrauer commented Jan 7, 2018

@gontard yes, we do. As @Amareis has mentioned, in our app we have deep-nested connected components at 3 different levels. However, we do not use any local state (no setState calls).

I wonder if this is a case that might benefit from React 16's upcoming async rendering?

@Amareis
Copy link

Amareis commented Jan 7, 2018

@liorbrauer connect HOC is using his own setState method.

@Nokecy
Copy link

Nokecy commented Jan 8, 2018

@liorbrauer Do you have any idea?

@gaearon
Copy link
Contributor

gaearon commented Jan 8, 2018

To clarify again, the warning is somewhat expected if you use react-redux because of how it's currently written. The plan is to change how it works in the future to avoid this.

@kelset
Copy link

kelset commented Jan 15, 2018

the warning is somewhat expected if you use react-redux because of how it's currently written.

So is this the most likely cause of the warning? Or could it be that my code is poorly optimised too?

If the second answer is a yes, what would be a good "rule of thumb" to reduce/minimize this warning showing up? To not have connected nested components, but only connect the root one and always pass down params?

@uudens
Copy link

uudens commented Feb 28, 2018

I had the same issue and I discovered that one of my selectors was always returning a new instance. When I memoized my selector this warning disappeared from the profiler.

@lhrinaldi
Copy link

I'm receiving a similar warning but with componentDidMount instead of componentDidUpdate. Could this be related? Also I'm using redux-form.

@mreishus
Copy link

mreishus commented Aug 9, 2018

If I get a lifecycle hook scheduled a cascading update in the performance timeline in chrome:

1.) Is there any way to find out more details about which lifecycle hook in which component actually scheduled this update?
2.) Is there an easy way to find out if I did something wrong or if this is an expected case from react-redux?

@markerikson
Copy link
Contributor

@mreishus : per the rest of the discussion in this thread, the warning appears to be due to how React-Redux v5 uses componentDidUpdate to trigger nested subscriptions.

That behavior will be changed in v6.

@mreishus
Copy link

mreishus commented Aug 9, 2018

I found a case in my application where this error was my fault, and not react-redux's. I had a componentDidUpdate that dispatched actions sometimes. I assume that would still trigger this warning even after v6 of react-redux. But it's difficult for me to look in the timeline and determine exactly what is happening when I see the cascading update warning message. I suppose this is outside of the scope of react-redux and I'll ask for help in react.

@eko24ive
Copy link

eko24ive commented Dec 5, 2018

Hi guys, sorry for bringing this issue up.

I'd like to ask @gontard if you were able to come up with a minimal app to reproduce this issue because I'm pretty much trying to achieve the same.

Also, I'd like to ask @gaearon if it was fixed on react-redux side?
Because as I understood from this comment of yours it could be related to react-redux, and I want to clarify if this is still "could" be a react-redux issue or this is my fault (which I think actually is)

@markerikson
Copy link
Contributor

@eko24ive : Dan doesn't actively maintain Redux or React-Redux any more.

We just released React-Redux 6.0, which has a different internal implementation. Please upgrade to that, as I would expect that it resolves this issue.

@eko24ive
Copy link

eko24ive commented Dec 6, 2018

Hi @markerikson and thank you for the quick update.
I'll try to upgrade to 6.0 and provide an update of my results to this issue.

@rauveridgithub
Copy link

Hello @markerikson we already updated to React-Redux 6.0 but we still encounter some similar issues. Although we have observed that some warnings are gone. Do you know any possible cause? Or any suggestions on what we can do to know the reason?

@markerikson
Copy link
Contributor

@rauveridgithub : please provide a project that reproduces the issue, preferably as a CodeSandbox.

@eko24ive
Copy link

Sorry for late update guys. I've upgraded to 6.0 and issue still remains, which confirms my suspicion that react-redux has nothing to do with it.

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

No branches or pull requests