Bug: too hard to fix "Cannot update a component from inside the function body of a different component."

[finom]

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

1. Build a time machine.
2. Go to the year 2017.
3. Build a huge application of 10K lines of code.
4. Get 80 (!) dependencies at package.json file including ones that become no longer maintained.
5. Update React to the latest version at February 27, 2020.
6. Get tons of errors that you don't know how to fix.
7. 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 🙏

[sebmarkbage]

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]

With me this appears when i use formik

[finom]

@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]

@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]

This really needs to be addressed ASAP. It makes upgrading impossible. The error trace is pretty much impossible.

[sebmarkbage]

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]

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 culprit may very well be the culprits yet it is impossible to easily determine that.

[markerikson]

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]

I face the same problem with Redux-form!

[JuanCamiloMendozaBalanta]

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]

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]

@l0gicgate

I think it is imperative that this error message be modified to include exactly which line of code is causing the error.

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]

@martinezwilmer This example is too small to help. Create a sandbox please.

[gaearon]

To the future commenters. I understand seeing a new warning is frustrating. But it points out legitimate issues that are likely causing bugs in your code. We would very much appreciate if you could refrain from expressing sarcasm and annoyance.

If you can't understand where it's coming from in the stack traces, please post screenshots or create reproducing sandboxes and we'll try to help. Most of these are probably coming from a few libraries, so the most productive thing to do is to reduce these cases and then file issues with those libraries.

Thank you all.

[Aarbel]

Hard to find the precise line concerned by the warning here:

@gaearon do you again have one tip about it ?

[gaearon]

Hard to find the precise line concerned by the warning here:

What makes it hard? As I noted above it’s the first line that doesn’t say “react” on the right side. In this case, it’s connectAdvanced from Redux. Please file an issue in React Redux so that the maintainers have a chance to look at this.

[markerikson]

As I said upthread, I would be very surprised if whatever's happening here is an issue with React-Redux.

That said, I'm also not even sure exactly what triggers this message in the first place. I half-get what the error message is saying, but what kind of app code pattern would actually be an example of that behavior?

[milesj]

I ran into this recently and the fix was wrapping setState handler call sites in useEffect, like so: airbnb/lunar@db08613#diff-1c3bdd397b1ce5064142488877045306R56 (onChange and onSubmit use setState higher up the chain).

@martinezwilmer Where is onPriceUpdated being called? Maybe try wrapping it in useEffect?

[JoviDeCroock]

The same issue seems to be happening for urql

We are using use-subscription + wonka (for streams) to orchestrate our updates, however an update can come in synchronously. Here we have already fetched the todos so if we click the Open button the result should just instantly pop up, however this seems to trigger the following error.

In our case this is intended, we can't just show fetching: true for a sync result, that would result in jumpy interfaces.

[hugo]

This is starting to crop up more and more in third party libraries now: urql, Apollo.

I ran into this and for several hours I assumed the problem was in my code. The condensed stacktrace points at my components, and it's not unusual for me to see third party libraries in the expanded stacktrace when I did explicitly trigger an error. From my (albeit limited) research into this particular warning, it seems that most developers are not causing this issue themselves, but rather depending on code that does. Usually it's good practice to assume it isn't a bug upstream but when it is an upstream bug, wasting time tracking down a problem in your code that doesn't exist is rather frustrating. Is there anything React can do to help an average user determine if it was code they wrote, or code they depend upon that caused the issue?

One thing I note from the Apollo issue is:

The warning's stacktrace is showing the component that initialed the changes, not the one that gets re-renderd [sic] by those changes

If this is correct, can React give more information here? Perhaps telling us both the initiating component and the components that it caused to be updated?

[ericis]

Like @hugo , I encountered this when testing a new Ionic application:

1. npx ionic start demo sidemenu --type=react
2. react-scripts test

Indeed, the cause is buried in the middle and bottom of the stack trace.

console.error node_modules/react-dom/cjs/react-dom.development.js:88
    Warning: Cannot update a component from inside the function body of a different component.
        in Route (at App.tsx:37)
        in View (created by StackManagerInner)
        in ViewTransitionManager (created by StackManagerInner)
        in ion-router-outlet (created by IonRouterOutlet)
        in IonRouterOutlet (created by ForwardRef(IonRouterOutlet))
        in ForwardRef(IonRouterOutlet) (created by StackManagerInner)
        in StackManagerInner (created by Context.Consumer)
        in Unknown (created by Component)
        in Component (created by ForwardRef(IonRouterOutlet))
        in ForwardRef(IonRouterOutlet) (at App.tsx:36)
        in ion-split-pane (created by IonSplitPane)
        in IonSplitPane (created by ForwardRef(IonSplitPane))
        in ForwardRef(IonSplitPane) (at App.tsx:34)
        in NavManager (created by RouteManager)
        in RouteManager (created by Context.Consumer)
        in RouteManager (created by IonReactRouter)
        in Router (created by BrowserRouter)
        in BrowserRouter (created by IonReactRouter)
        in IonReactRouter (at App.tsx:33)
        in ion-app (created by IonApp)
        in IonApp (created by ForwardRef(IonApp))
        in ForwardRef(IonApp) (at App.tsx:32)
        in App (at App.test.tsx:6)

This issue was the closest I could find regarding this problem.

[samcooke98]

We found a specific pattern that causes this issue with mobx over in mobxjs/mobx-react#846

[milesj]

Maybe there should be an ESLint rule that warns against calling useState mutators within a render body?

[gaearon]

Calling setState of your own component during render is a supported pattern (although it should be used sparingly). It's setState on other components that are bad. You can't detect those statically.

[milesj]

It could theoretically be done using ts-eslint, assuming they are using the correct upstream React types, but yeah, probably more effort than it's worth.

[gaearon]

I don't think it could be done without some sort of effect tracking. As soon as you have one function in the middle, you lose the information.

[dalechyn]

I am also facing this issue with react@experimental+react-redux+redux.

Code looks like this:

import React, { Suspense } from 'react'
import { bindActionCreators } from 'redux'
import { connect } from 'react-redux'
import { Redirect } from 'react-router-dom'
import PropTypes from 'prop-types'
import { userActions, cabinetActions, tokensActions } from '../../actions'
import { CircularProgress } from '@material-ui/core'
import { api } from './api'

const Cabinet = ({ resource }) => {
	resource.read()
	return <h1>cabinet</h1>
}

Cabinet.propTypes = {
	resource: PropTypes.shape({
		read: PropTypes.func
	})
}

const CabinetPage = ({
	failedToLoad,
	tokensRefreshFailed,
	logout,
	loadCabinet,
	clearTokens,
	clearCabinet
}) => {
	if (tokensRefreshFailed || failedToLoad) {
		clearTokens()
		clearCabinet()
		logout()
		return <Redirect to='/login' />
	}

	return (
		<Suspense fallback={<CircularProgress />}>
			<Cabinet resource={loadCabinet()} />
		</Suspense>
	)
}

CabinetPage.propTypes = {
	loadCabinet: PropTypes.func,
	failedToLoad: PropTypes.bool,
	tokensRefreshFailed: PropTypes.bool,
	logout: PropTypes.func,
	clearTokens: PropTypes.func,
	clearCabinet: PropTypes.func
}

const mapStateToProps = ({
	alert,
	tokens: { tokensRefreshFailed },
	cabinet: { failedToLoad }
}) => ({
	alert,
	tokensRefreshFailed,
	failedToLoad
})

const mapDispatchToProps = dispatch =>
	bindActionCreators(
		{
			cabinetLoad: cabinetActions.load,
			logout: userActions.logoutWithoutRedirect,
			loadCabinet: api.loadCabinet,
			clearCabinet: cabinetActions.clear,
			clearTokens: tokensActions.clear
		},
		dispatch
	)

export default connect(mapStateToProps, mapDispatchToProps)(CabinetPage)

loadCabinet() dispatches an async action of three-phase rendering as concurrent docs say, and returns an object with read() prop.
However, I can't see any parent updates here.

For anyone else having this issue, I fixed it by moving redux action dispatching to a returned component. Here's how it looks like now:

const CabinetPage = ({
	alert,
	failedToLoad,
	tokensRefreshFailed,
	logout,
	loadCabinet,
	clearTokens,
	clearCabinet,
	clearAlert
}) => (
	<Suspense fallback={<MUIBackdropProgress />}>
		{alert.message && (failedToLoad || tokensRefreshFailed) ? (
			<MUIAlertDialog
				title={alert.message}
				text={errorText}
				onClose={() => {
					clearAlert()
					clearCabinet()
					clearTokens()
					logout()
				}}
			/>
		) : (
			<Cabinet resource={loadCabinet()} />
		)}
	</Suspense>
)

I like this warning because it forces you to choose right design patterns. Now everything works flawlessly!

[gaearon]

We fixed the cases where the warning was over-firing in 16.13.1. The remaining cases are legit and need to be fixed.

[finom]

@gaearon just upgraded and the error disappeared! Thank you guys for your work!

[vattcarter7]

@gaearon thanks. you just saved my day :-)

[cmakohon]

While upgrading did not resolve my issue, it did give me more information in the console to help find my problem. Thanks @gaearon !

[ntucker]

What if you dispatch an action that causes the other component to return the same state as last time? Is that considered bad? I would think it would short-circuit in that case.

[machineghost]

Can I just say that this while I totally understand the logic behind this warning ... it almost feels like a betrayal of what the React team has been telling the community, because it feels like the team has taught these important truths about how to code React:

1. keep your state as high as you need to in your hierarchy (no higher), and then pass data and setters down to child components

2. Function components are awesome! Forget that class noise, do your whole component in one function!

And now when people follow those two rules, and pass their state setters down to their function components, and call them in those components ... you pull the rug out and go "ha, but you can't actually do what we told you to do".

None of this changes anything about the technical needs here, and I'm not saying anything about this change is wrong or bad ... I just feel there's a messaging problem, in that you're not communicating good clear rules (like the two I just mentioned) for coding in this new world. If you're going to change the rules on us, I think it would be helpful to do so by first explaining best practices.

So, all I'm really asking is ... I think it would be more ideal if someone on the team were to write something (like an article) where they say "I know we gave you those two rules before: here are the new rules", and then add a link to what that article in every place in the docs/release notes that reference this new warning (so everyone googling "WTF is this?" can learn the proper way to code React, in the "new world").

[markerikson]

@machineghost : I think you're misunderstanding what the message is warning about.

There's nothing wrong with passing callbacks to children that update state in parents. That's always been fine.

The problem is when one component queues an update in another component, while the first component is rendering.

In other words, don't do this:

function SomeChildComponent(props) {
    props.updateSomething();
    return <div />
}

But this is fine:

function SomeChildComponent(props) {
    // or make a callback click handler and call it in there
    return <button onClick={props.updateSomething}>Click Me</button>
}

And, as Dan has pointed out various times, queuing an update in the same component while rendering is fine too:

function SomeChildComponent(props) {
  const [number, setNumber] = useState(0);

  if(props.someValue > 10 && number < 5) {
    // queue an update while rendering, equivalent to getDerivedStateFromProps
    setNumber(42);
  }

  return <div>{number}</div>
}

[machineghost]

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]

@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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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]

I think we’re getting to the point where this thread has outlived its usefulness.

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’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.

And while I appreciate your response, I was mainly just hoping for "rules of thumb", guidelines, best practices, etc

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.

"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 child component renders and changes state you'll have problems".

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]

As a small update (sorry for pinging everyone), I heard final-form/react-final-form#751 (comment) resolved a bunch of these errors for people who were using that library.