Repository navigation
Bug: too hard to fix "Cannot update a component from inside the function body of a different component." #18178
Description
Note: React 16.13.1 fixed some cases where this was overfiring. If upgrading React and ReactDOM to 16.13.1 doesn't fix the warning, read this: #18178 (comment)
React version:
16.13.0
Steps To Reproduce
- Build a time machine.
- Go to the year 2017.
- Build a huge application of 10K lines of code.
- Get 80 (!) dependencies at package.json file including ones that become no longer maintained.
- Update React to the latest version at February 27, 2020.
- Get tons of errors that you don't know how to fix.
- Tell your client that fixes are going to take unknown time and it's going to cost $$$ + days or weeks of investigation or we're going to get stuck with the outdated version of React and related libraries forever which will cost more $$$ but later.
Being serious, the business I work for isn't interested on that at all. Obviously I'd never made it happen to get such warnings to appear if I'd get them earlier. Currently that's impossibly hard to make the errors to be fixed because I get them at many different cases and with a huge stack trace. I tried to fix at least one of the appearing errors and it already took a lot of time. I tried to debug some of used libraries but got no luck.
Just one example:
There we can notice the use of an outdated react-router, an outdated redux-connect (which I had to put to the project source to fix errors of outdated componentWillReceiveProps method), some HOCs created by recompose etc. It isn't just a simple virtual DOM tree where I can walk thru components developed by me and search by setState string to fix the bug, that's way more complicated than that.
Please make an "UNSAFE" option to disable this error or provide a simpler way to find where the error is thrown 🙏
Activity
sebmarkbage commentedon Feb 28, 2020
I wish we had added this warning earlier. I'm sorry we didn't. It was an oversight with the introduction of Hooks. I believe this must be caused by newer code that uses Hooks, because there was already the same warning for classes much earlier so any earlier code would've already seen this warning.
Note that this will likely start hard failing in future versions of React. Intentional or otherwise (we've had a lot of bugs with this pattern). So regardless, you might end up getting stuck on an old version. If it's not possible to fix it, I'd recommend pinning to an older version of React.
However, that said, we want to help you and make it as easy as possible to fix these, including seeing if we can get help from library authors to publish fixed versions.
If you expand the little
>arrow in the Chrome console you should also see an additional Stack trace (in addition to the Component stack in your screen shot). Can you post that too? That should show you the exact callsite that is causing a side-effect in render.RodolfoSilva commentedon Feb 29, 2020
With me this appears when i use formik
finom commentedon Mar 2, 2020
@sebmarkbage thank you for the response. The stacktrace appearing after clicking > is ridiculous. It contains 200+ items!
I was going to paste it there or give a link to pastebin but tried a different direction. I walked thru Github issues of some of used libraries and found out that one of suspects is redux-form: redux-form/redux-form#4619. I hope that's the only library which causes the errors and I'm going to wait for a fix before upgrading React.
But still, I'd ask to not close the this issue and I propose other developers to mention here libraries which also cause these errors.
finom commentedon Mar 2, 2020
@RodolfoSilva are you sure that it is caused by formik? If yes, can you please create an issue there and post a link to it here? I'm going to create a list of such issues at the beginning of my first message if the list is going to contain more than one item.
strongui commentedon Mar 2, 2020
This really needs to be addressed ASAP. It makes upgrading impossible. The error trace is pretty much impossible.
sebmarkbage commentedon Mar 2, 2020
Hm. I wonder if describing which line to look for in the error message would help.
In this case the first line to look for is the line after
dispatchAction. That should be the thing that calls into React.@RodolfoSilva can you post the source of
FormItemInput.js, if it's something you can share? That seems to be calling dispatch or setState on line 71.l0gicgate commentedon Mar 3, 2020
I think it is imperative that this error message be modified to include exactly which line of code is causing the error. It's almost impossible to pinpoint whether it is local code or library code that's causing the issue. External librairies
libraries like react-redux and react-router are most likely the culpritmay very well be the culprits yet it is impossible to easily determine that.markerikson commentedon Mar 3, 2020
I'm pretty sure that React-Redux is not at fault here, but agreed that the error message and component stack make it kind of hard to know what's actually going on.
musme commentedon Mar 4, 2020
I face the same problem with Redux-form!
JuanCamiloMendozaBalanta commentedon Mar 5, 2020
I have the same problem and I'm seeing that the warning is showing the first time that i write in my redux field or when i clear all it
martinezwilmer commentedon Mar 6, 2020
I have that problem too, this is my component:
`const [price, setPrice] = useState(0);
const updatePrice = (newPrice) => {
setPrice(newPrice)
}
< CardContainer onPriceUpdated={updatePrice} ></ CardContainer >
`
So, in this case my CardContainer component, notify to the parent component when the price is updated and the parent component renders the new prince.
So i guess React is warning me that i'm trying to update the state of a component using the function of other component.
I'm new in react, so i'm not sure if this is a React pattern or is in fact a bug.
If you guys have any suggestion to solve this warning i would appreciate it``
gaearon commentedon Mar 6, 2020
@l0gicgate
There are limits to what we can do in JavaScript. But all the information is in the stack you see in the browser. All you need to do is to skip the lines that are inside React.
To see the JavaScript stack, you need to click on a small arrow next to the error message.
For example, look at this screenshot from earlier:
I appreciate it's a bit annoying to dig through the stack, but it shouldn't be that hard to skip the first several frames. The very next frame is the source of the problem. In this case, it seems like something inside the Formik library. So you can file an issue with it.
gaearon commentedon Mar 6, 2020
@martinezwilmer This example is too small to help. Create a sandbox please.
84 remaining items
machineghost commentedon Mar 23, 2020
Right, but when you're coding your component you're not thinking of the timing of its parent. That's part of the beauty of React components: encapsulation.
So again, I'm not saying the new warning is bad at all, I'm saying before we had two rules that any good React dev could follow. Now under X conditions those rules go out the window, but only under X (where it sounds like X = "while the parent component is also updating").
I just think there needs to be more focus on explaining that, and on how to avoid the problem, instead of just being like "this is a problem now!".
markerikson commentedon Mar 23, 2020
@machineghost : you're truly not getting what I'm saying here.
Parent/child timing is not the issue.
Queueing state updates in other components while rendering a function component is the issue.
machineghost commentedon Mar 23, 2020
By definition it has to be a parent/child (or grandchild): how else could it have passed the state setter down?
I'm not saying this can't also be an issue in other component relationships, but I'm specifically talking about people following React best practices, passing state setters down, and then getting this warning.
That's all I'm talking about, and all I'm saying is, "it could be explained better, with more focus on how to code well instead of just 'here's a new error and here's what it means'".
markerikson commentedon Mar 23, 2020
Timing. Is. Not. The. Issue.
A function component is allowed to queue a state update, while rendering, for itself only. As my example showed, that acts as the equivalent of
getDerivedStateFromProps.Queueing an update for any other component from within the actual rendering body of a function component is illegal.
That's what this warning is telling you.
I don't know how I can explain that any clearer.
machineghost commentedon Mar 23, 2020
Timing is not the issue: not your's, not mine. My issue is documentation, or lack thereof.
But you've decided to start a war in an issue thread with an Internet stranger, instead of listening to what they're saying and ... I have no desire to continue to engage with you.
markerikson commentedon Mar 23, 2020
The point is that no rules have changed. This has always been a wrong pattern. It's just now being highlighted so that you're aware your code is buggy.
machineghost commentedon Mar 23, 2020
And literally nothing you just said disagrees with anything I wrote. In fact, it's almost like I've said the exact same thing from the start ... and all I ever asked for this whole time was a better explanation of those same rules, the ones you say haven't changed (and of course they haven't ... what changed and "made a new world" was the warning).
P.S. You also seem to fail to realize the irony here. If I fail to understand anything, that's making the case that the documentation could be improved. Yelling at me about how poorly I understand things only strengthens my position; it doesn't magically improve the documentation.
gaearon commentedon Mar 23, 2020
Hi folks, let’s cool down a bit. 🙂
@markerikson Appreciate you jumping in but I think this discussion is getting a bit too heated.
@machineghost Thanks for expressing your concerns. I know it’s annoying when new warnings pop up for patterns that might have seemed innocuous before.
I agree this warning requires some prior context. Essentially, you needed to know two things from the class era:
That you shouldn’t setState during render. Classes always warned about this.
That function component body is essentially the same thing as class component render method.
It is indeed our omission that setState on another component during the function component body did not warn before. You could infer it’s a bad pattern from the two points above but it’s fair to say one could not realize it. We’re sorry for the inconvenience.
If you feel there’s any particular place in the docs where this should be mentioned, please raise an issue on the docs repository. We’re planning a rewrite of the docs to be based around Hooks so that’s something we can keep in mind. Thanks!
machineghost commentedon Mar 23, 2020
I in no way mean to make anyone feel bad about anything, and I refuse to accept your apology ;) Hooks are genius, and y'all are geniuses for inventing them. And any engineer who faults another engineer for not perfectly imagining every outcome is ... a jerk.
All I've been trying to communicate is, currently when I got this warning, I did what everyone does: I googled it. I then found a page that said "we've got this new warning".
I just think it would have been better (and could still be better) if there could have been (can be) a link in that announcement, or similar places that someone might find by googling, to some "higher level discussion" of "here's why we introduced this error and here's how you can be an awesome React dev and follow some basic guidelines to never run into it yourself."
But again, hooks are awesome, the React team is awesome, and even this new warning is awesome (I'm sure it beats the hell out of discovering the error it's trying to guard against). If anyone owes anyone an aplogoy it's me for not leading with that.
gaearon commentedon Mar 23, 2020
Sure, no hard feelings. The answer to “why” isn’t any more complex than “if one component triggers updates on other components during rendering, it becomes very hard to trace the data flow of your app because it no longer goes from top down”. So if you do that you’re kind of throwing away the benefits of React.
Again, to clarify, this isn’t new per se — classes always had the same warning. We missed it with Hooks, and are rectifying the mistake. I suppose that the way you fix it depends on the case — so we can’t give you exact instructions — but happy to help if you share an example you’re struggling with. Generally saying, you’d fix it in a similar way to how you’d fix the corresponding class warning that always existed.
Hope that helps somewhat!
strongui commentedon Mar 23, 2020
I will add my two cents to this discussion and agree with @machineghost that there has been a lot of confused since the introduction of functional components and hooks. The community is looking to the react team for leadership but things that used to be simple are becoming convoluted and there seems to be a lack of communication and clear examples.
Case and point being ComponentDidMount and Unmount, first we're told use function components, then use useEffect with an empty array, then we're told that's no good, now we've got this error message mess. I don't know, I appreciate all the hard work going into react but we really need more effort put into documentation and best practices.
machineghost commentedon Mar 23, 2020
I've been on the function bandwagon for so long (trying to avoid classes with Recompose and such even before hooks) that I don't even remember those class warnings.
And while I appreciate your response, I was mainly just hoping for "rules of thumb", guidelines, best practices, etc. Stuff like "keep your state as high as it needs to be to cover all components that use it, but no higher" or "pass state setters down to child components using the inversion of control pattern".
Maybe there just aren't any here, but maybe something like "if functional component A changes state, it shouldn't render child component B that it passes a state setter to (it should return right then or something), because then if the child component renders and changes state you'll have problems".
Or maybe it's late on Sunday, I've been working all day on a personal project, and my brain is just too fried and is making something simple into something hard. Either way, I'll post back if I have any further suggestions for guidelines, but otherwise I certainly don't want anyone else spending their Sunday night on this.
Thanks again though!
gaearon commentedon Mar 23, 2020
I think we’re getting to the point where this thread has outlived its usefulness.
I’m sorry that our documentation hasn’t helped you but it’s very hard to say what specific problems you ran into. This is unfortunately very vague and is a distortion of what we’ve tried to say. Like a game of broken telephone. If you have a specific problem please file a new issue and try to describe it in more detail. We’ll try to help you if you can be more specific.
The rule of thumb has always been “don’t perform side effects while rendering”. Think of rendering as a pure computation. Side effects go into a different place (lifecycle methods in classes, or useEffect in function components). There’s not much more to it.
I think there’s still some misunderstanding here. It’s perfectly fine to pass state setters to a child component. Has always been fine, and will always be.
The problem is in calling them while rendering. That should generally be completely unnecessary. It’s hard for me to guess why you’re doing it without a concrete example. So it’s hard to help.
The general theme in both of these conversations is that we’re going in circles. The discussion has switched to be meta, and instead of talking about specific cases we’re discussing vague generalities. It is likely we misunderstand each other, but the lack of concrete examples is making this misunderstanding impossible to resolve.
For this reason I’m going to lock this thread. I very much appreciate everybody’s input here, and we’d love to hear more from you if you struggle to fix this warning. The way to get help is to file an issue with a minimal reproducing example. Then we can discuss your concrete problem and help brainstorm a solution. This will be more productive for everyone involved, and will also avoid sending emails to dozens of people who have already commented on this thread and ended up subscribing. Thank you!
gaearon commentedon Mar 30, 2020
As a small update (sorry for pinging everyone), I heard https://github.com/final-form/react-final-form/issues/751#issuecomment-606212893 resolved a bunch of these errors for people who were using that library.