react: The minor release of 16.4 causes BREAKING changes in getDerivedStateFromProps

I’m going to close this. In conclusion:

• If this change exhibits bugs in your code in React 16.4, your code was already buggy in React 16.3 in more subtle and less deterministic ways.
• To support this claim, I refer you to five examples posted in this thread https://github.com/facebook/react/issues/12898#issuecomment-391781205, https://github.com/facebook/react/issues/12898#issuecomment-391847365, https://github.com/facebook/react/issues/12898#issuecomment-391946524, https://github.com/facebook/react/issues/12898#issuecomment-392001463, https://github.com/facebook/react/issues/12898#issuecomment-392048949 which are all already buggy in React 16.3 as I explained in my analysis in https://github.com/facebook/react/issues/12898#issuecomment-391782913 and https://github.com/facebook/react/issues/12898#issuecomment-392035163.

If you’re not convinced by my explanation, let’s take a demo from https://github.com/facebook/react/issues/12898#issuecomment-392080509.

Indeed, at the first glance seems like in the version using React 16.4 you can’t edit anything, but in the version using React 16.3 the code runs fine. So is the original code free of bugs?

No, it’s not. Here is a fiddle using React 16.3 where we re-render the parent component every second. You can see that the input resets every second:

That clearly wasn’t intended. And all examples in this thread that got broken by React 16.4 already have the same exact bug. Whenever a parent re-renders, the state gets unintentionally reset. And it’s pretty hard to debug because components are decoupled, and whether you see this bug or not depends on how “lucky” you are.

React 16.4 helped surface this bug in your code so you can fix it, instead of keeping the app broken in cases that reproduce less reliably.

As we mentioned, we’ll follow up with another post (edit: it’s live!) that describes when (not) to use getDerivedStateFromProps. I already shared some thoughts on component API design part of this in https://github.com/facebook/react/issues/12898#issuecomment-392035163.

But we maintain that making an existing bug in your app reproduce more consistently is not a breaking change, and is a helpful way to help you find and fix that bug. We are sorry for not being clear enough in the documentation and our guidance for using this method, and we’ll amend that by posting more in the coming week. (Edit: the post is live!)

Finally, I agree this method is kinda hard to wrap your mind around. I don’t think it’s the flaw of this API, it’s just that state-based-on-props was always hard to think about but this complexity was “hidden” due to imperative nature of componentWillReceiveProps. So even if bugs like this existed in your apps you might not have been able to reliably reproduce them every time.

The new method makes the complexity of what you’re trying to do explicit. The verbosity helps you realize that perhaps it’s not a good solution for your particular problem, and being more clear whether a value is controlled or uncontrolled, and lifting state up when necessary, will be better both from readability and correctness perspectives. Again, we’ll follow up with more guidance on that.

Thanks to everyone for the discussion! I hope this was helpful, and if I missed anything please let me know.

I believe the “before” code already had a bug (so 16.4 just surfaced it). Let me demonstrate it.

At this point, the documentId never will change.

It may be true that in your particular case this prop never changes. That’s a pretty fragile assumption from the component’s point of view—ideally it should be able to work with new props—but maybe it’s a reasonable intentional limitation of this component’s API.

Still, its parent component could re-render at any time (e.g. if it had setState call itself, or if one if its parents did). It could even be a parent component from a third-party library (e.g. a router that adds a setState call in a patch or a minor version).

It’s hard to tell whether this can happen or not without checking the source of every single component above in the tree. So even if this doesn’t happen right now in your code, the chances are high it might happen in the future when you’re working on a completely unrelated feature.

So why is that a problem? If any of the parents re-render, getDerivedStateFromProps would also get called again on your component. In your example, even with React 16.3, a parent re-render would reset the selection values though the document ID hasn’t changed. Your analytics call would also fire twice.

This is why the “before” code already had a bug, and the change in React 16.4 helped uncover it.

Even if none of the components above currently ever re-render, that’s an implicit assumption that makes every component with getDerivedStateFromProps fragile because such code relies on nothing ever updating above it. You can see this doesn’t really work with the promise of an encapsulated component model. People generally expect that it’s safe to add state to components above, or to move a component to a different tree.

That’s exactly the kind of broken assumption that led to bugs at Facebook, and led us to do this change. React 16.4 calls it more often which surfaces such bugs that already exist in your code.

Conversely, getDerivedStateFromProps implementations that don’t contain this bug work correctly both in React 16.3 and in React 16.4.

Note how in the 16.3 blog post we explicitly demonstrated you need to keep previous values in the state for use cases like this, just like you ended up doing in the “after” example. Then you wouldn’t have this problem now.

Note that additionally, getDerivedStateFromProps is intended to be a pure method. It’s not an appropriate place for side effects like the analytics call (use componentDidUpdate for them instead). You’ll get duplicate analytics events for every re-render. I’m sorry if this wasn’t clear from the docs. That wasn’t the main bug in the code but I figured I’d mention that.

Finally, there are also more subtle issues that would happen in async mode with code like this. Since the whole point of getDerivedStateFromProps was to migrate from async-unsafe patterns, it would be pointless to allow people to keep relying on it. If you didn’t fix this bug, you wouldn’t have gained anything from the componentwillReceiveProps migration.

Also I’d like to ask everyone to keep in mind that we’re not doing changes just to piss people off. We want what is best for apps in longer term, and we care about not adding extra boilerplate as deeply as you do. Again, we’re sorry for the churn this caused.

We’ll make another blog post about this next week. It seems like more people use getDerivedStateFromProps than we expected, and that is likely due to a misunderstanding about its purpose. For example, it might be that the code that relied on componentWillReceiveProps is always getting updated to getDerivedStateFromProps whereas memoization or lifting state up would have been more appropriate. We’ll show more appropriate strategies that don’t introduce the kind of boilerplate you’re concerned with.

But to continue this discussion we need code examples. You can see otherwise it immediately turns abstract and vague.

Semver is a social contract. I think it’s not as clear cut as it may seem. Here’s code that may randomly break and then “unbreak” on every single React release:

if (ReactDOM.render.toString().length > 330) {
  throw new Error('oops');
}

Does it mean we can never change render() implementation because any possible change would break semver? Probably not.

It’s a silly example of course. What about code like this?

<div onGotPointerCapture ={() => { throw new Error('no'); }} />

Technically it doesn’t do anything in React 16.3, but throws an error in browsers that support in React 16.4. Does that mean that adding support for pointer events was a breaking change?

This code also looks silly although maybe this version is a tiny bit less far-fetched:

const handlers = {
  onGotPointerCapture() {
    const x = somethingThatReturnsNull();
    x.foo();
  }
}

// somewhere
<Foo handlers={handlers} />

// ...
//  in Foo:

<div {...this.props} />

Maybe somebody could write something like this assuming that pointer events were already supported, and didn’t notice the warning. This code would break with 16.4 release.

I think it’s still implausible but we can argue that for any kind of change, there is a chance that there exists some code that happens to rely on the old behavior. If we wanted to respect semver in the strictest (and almost mathematical) sense then we’d have to make every single release major. Which of course defeats the purpose of semver.

So, coming back to my original point, semver is a social contract. We try hard not to break things that weren’t broken. But given the choice between “break already broken code more strictly and ensure more broken code doesn’t get written” and “keep broken code working in some cases, but allow more broken code to be written” it’s less clear cut. In this case, we decided that it’s worse for the ecosystem to endure the pain of fixing it later (and in the meantime live with bugs that are hard to reproduce) than make existing hidden bugs apparent. And this change does fix other categories of bugs by itself as well, so it’s not just a “make existing bugs more prominent” scenario.

I can see arguments for doing it the other way but I hope you can see ours too.

Update: see my comment at the bottom of this thread for the conclusion.

TLDR: if this change broke your code when you moved from 16.3 to 16.4, your code was already buggy in subtle ways. The change in React 16.4 made these bugs in your product code occur more often so you can fix them. We don’t consider making existing bugs in your code reproduce more reliably to be a breaking change.

If you’re coming from an issue in some third-party library that works with 16.4, but doesn’t work in 16.3 — you’re hitting the React bug. This issue is a complaint about this bug getting fixed in 16.4. However, we think fixing it was the right choice. We recommend everyone to upgrade to React 16.4 or later.

Again, for details, walkthroughs and demos, see my comment at the bottom of this thread.

---

I’m sorry this broke things for you. Can you show the relevant code please?

We’re aware this change can cause issues in rare cases, as documented in the blog post. Unfortunately not making this change also leads to buggy behavior, although less obvious and deterministic so maybe you didn’t bump into it yet. I can see your point that fixing it is “breaking” in your case, but I hope you can also see that fixing any bug can be considered breaking for the code that relied on it.

If your code relied on the old behavior but doesn’t work with the new one, it worked by accident. It’s our fault for overlooking this in the 16.3 release but it took about a month for us to discover this problem from bug reports at Facebook. The function takes two arguments: props and state. But it only runs when one of them updates. This didn’t really make sense and was an oversight in the initial implementation.

When we fixed that issue at Facebook (which, as we learned, was the reason behind numerous bugs), one component that relied on the buggy behavior broke (just like in your case). This was just one component out of a thousand or so. We decided that it’s better in the long term to make the fix than to allow more components to be written that rely on the bug.

Again, I’m sorry this caused issues for you. Seeing your code would be helpful.

Here’s an updated example with 16.4 that works: https://codesandbox.io/s/m4km18q38j

My changes (compared to the broken 16.4 version you posted are):

• Removed unnecessary state and getDerivedStateFromProps from ButtonSelectable. It’s now fully controlled.
• Removed unnecessary getDerivedStateFromProps from EditObject. Instead of mirroring object in state, I separated this.props.object (coming from above), and this.state.overrides (locally edited fields). I don’t attempt to “sync” them. Instead, I merge them in the render method: {...this.props.object, this.state.overrides}. When I want to confirm edits, I send the merged objects upwards, and the parent component replaces its state.
• I need to reset EditObject’s overrides when we switch between “different” objects in the parent. However, as mentioned in the blog post, it is brittle to do so by comparing next and previous props. Even with componentWillReceiveProps this solution doesn’t work very well (what if you need to update the object above for unrelated reasons but not reset the form state?). Instead, I added an id field to the objects. Now I can reset the overrides state using the “uncontrolled component with a key” approach: <EditObject key={this.state.object.id} />.

Hope this is helpful!

@kohlmannj Regarding the pattern you described above, I wanted to reiterate a concern in case it’s helpful. (This isn’t exactly related to getDerivedStateFromProps but more about the overall pattern.)

You described your component as:

• The component exposes this.props.scaleX and this.props.scaleY, which are used to derive initial this.state.scaleX and this.state.scaleY values
• Incoming number-type props changes to this.props.scaleX or this.props.scaleY should replace the corresponding this.state value
• This supports the need for an interactively rescaleable scroll view which can either be used in “uncontrolled”, standalone form; or, in the future*, as controlled by a parent component

My concern is that mixing controlled and uncontrolled behavior like this leads to a confusing API and/or confusing runtime behavior. At a high level, it’s not clear what the source of truth is when props and state values disagree. More practically speaking, there are two variants this pattern usually takes- and each has downsides.

---

Variant 1: State is always reset to props when the component is re-rendered

The recent getDerivedStateFromProps change highlights one downside of this approach, but the problem also existed for version 16.3 as well as older versions built on top of componentWillReceiveProps.

The problem is this: Unexpected re-renders override your component’s state unintentionally. This includes changes to unrelated props like callback functions or inline styles. These are often re-created inline during render, and so bypass PureComponent or shouldComponentUpdate purity checks.

---

Variant 2: State is only reset when props value changes

This pattern compares incoming props to previous props and only updates state when the value changes externally. (This is done to avoid the unexpected re-render problem I mentioned above.)

However this also has a downside: There’s no good way for the parent to reset a property to the previous value.

Here’s an example scenario:

1. A ScrollView component like yours is used to render a lot of form fields and underneath of them, there’s a “submit” button.
2. A user fills out the fields and tries to submit, but there’s an error with one field.
3. The application renders <ScrollView scrollTop={posOfErrorField} {...props} /> to automatically scroll to the field with an error so the user can see it.
4. The user modifies the field, then scrolls down and submits again.
5. There’s still an error with this field.
6. The application renders <ScrollView scrollTop={posOfErrorField} {...props} /> to scroll back to the problematic field so the user knows it still has an error.
7. Nothing happens. (State is not updated, because posOfErrorField hasn’t changed.)

This would be confusing for both the user and the developer trying to debug this problem. And the only work arounds are:

• Set a new key to recreate the component entirely. (This works okay for small components like form fields, but can be expensive for components that contain a lot of children- like our ScrollView.)
• Re-render twice, once with e.g. a null prop and then the second time with the original prop- in order to force the inner component to reset its state.

---

We recommend designing your components to be either fully controlled or fully uncontrolled to avoid the above cases.

• A fully controlled component avoids both of the above problems, because there’s no state to be reset.
• A fully uncontrolled component can be lighter weight to use (since it generally requires fewer callbacks). In this case, props can be used to initialize default state values, but they never update state afterwards- for reasons like the ones I’ve outlined above. In this case, we also suggest prop names that are clear like defaultScrollTop or initialScrollTop etc. If you need the ability to forcefully reset an uncontrolled component, you could add an instance method (to be called via a ref).

Am I the only one who now just can’t grasp the naming and the param names of this method? It’s called getDerivedStateFromProps and the params are (nextProps, prevState), and it totally made sense in 16.3 - we were literally getting derived state from props, on the new props, thus having nextProps and prevState as params. Yeah, it was kind of a new componentWillReceiveProps, but at least the naming made sense, particularly when I was going through this diagram two days ago. Now when the same method will fire on setState() and forceUpdate(), I am totally confused. Why do I need it on setState()? How do I get the newState in this case? How is a prevState useful in this case? What am I deriving from what? And why do I need it on forceUpdate()?

@catamphetamine

I already asked you to refrain from name-calling about a month ago in another thread. Please stop this.

I am not “blaming” other people for their code being buggy. I’m sorry if I came across this way. I am pointing out that the bug you’re running into already existed in your code and is not a new one. I have analyzed it in detail in this thread, including providing a demo.

This is not a value judgment about you or your code. If we want this discussion to be fruitful, I think it might help to separate the technical details from emotions.

The same is true for your snippet in https://github.com/facebook/react/issues/12912 with React 16.3. Your phone input would completely reset if onChange callback doesn’t immediately set the same state, and the parent component re-renders for another reason. If you turn this into a fiddle I’ll be happy to demonstrate that.

Again, I’m sorry that this bug was surfaced by the change (and it frustrated you) but it already exists in the code you wrote with React 16.3. I’m not trying to blame you, or make you feel bad. I’m only stating the fact about the code. And, as I mentioned earlier, we don’t consider making existing bugs in your code occur more often to be a breaking change.

But we definitely could do better at communicating how to use getDerivedStateFromProps without introducing bugs, and the blog post we plan to write will hopefully help that.

Peace.

I’m not sure I understand the benefit or reasoning to have this method called on every single change.

We had to go back and write much more logic to handle the internals of all the getDerivedStateFromProps method calls after this change that feels more like boilerplate code the framework should be (and was before 16.4) handling. It just add more noise and bloat to our components.

The 16.3.2 version acted more in lines with the former componentWillReceiveProps , which this method replaced. In the pre-16.4 behavior, it was much easier to migrate from the deprecated componentWillReceiveProps to the new getDerivedStateFromProps.

The blog post provided by @chase states:

The new static getDerivedStateFromProps lifecycle is invoked after a component is instantiated as well as when it receives new props.

This is exactly what we expected and how it operated in 16.3. That is much cleaner in my opinion.

As a side note, and on a personal level, I never liked the deprecation of componentWillReceiveProps and have complained about it internally to many of my coworkers. However, I understand the side-effects that are trying to be edged out by changing methodologies on how the lifecycle works. That being said, if the React group feels that the new behavior introduced in 16.4 is how they wish it to work, I would like to suggest another lifecycle method is introduced more in line with how componentWillReceiveProps worked, or undeprecating that method and putting more protections around it to try and edge the issues you intended to prevent. Of course, I am not an architect, nor an employee of Facebook, or a member of the React development team, but the changes around the lifecycle in this particular area are extremely frustrating as a developer.

The given explanation for why the change was made is reasonable, it seems, but that’s a separate issue from the fact this change was released without a major semver bump. 16.4 breaks what was generally understood to be the correct behavior in 16.3. I hear that there was some intentional ambiguity in the spec, but that doesn’t change how folks understood gDSFP to work.

I’m glad none of my own code broke, but for a project as widely adopted as React it seems reckless to ship a breaking change under the guise of a “bugfix.” I understand it may be inconvenient and out-of-step and bad marketing to release a new major version all of a sudden, but on the flipside this makes React’s versioning less reliable and it makes me a bit more hesitant about upgrading to new React versions.

@royalhunt Please don’t use this thread to discuss a completely unrelated problem. If you believe it’s a bug please file a new issue with a reproducing example.

@goyney Every bug fix has the potential to break your code if you were relying on the buggy behavior. It’s true that this bug fix is a little more likely than most to break code, but there isn’t a hard line between “bug fix” and “breaking change”; it’s much more of a spectrum. When we looked at this we felt it was closer to a bug fix.

@goyney

I couldn’t help noticing you downvoted my last comment. Is there something in my explanations that’s not clear or that seems wrong? I tried my best to explain why the change uncovers existing bugs (making existing broken codepaths more reproducible), and why code that didn’t have bugs with the old behavior also doesn’t have them with the new behavior (so it’s not a “breaking” change). Is something still unsatisfactory and could be better? What can we do to improve aside from publishing a blog post (as we plan to)?

I understand it’s frustrating when a version update leads to seeing bugs. But if those bugs already existed in your app, perhaps it’s a consolation that now you see them clearly every time, and have an opportunity to fix them?

So how can you make this pattern work? Really, the best way is to avoid it, and instead explicitly make the component fully controlled (so that the state can be managed by the component above).

But if you must do something like this, you can accept a prop like current item ID, and keep the previous ID in state. Then in getDerivedStateFromProps() you know the current item has changed and you need to reset the state. Just having the current value changing from above doesn’t really tell you enough to make a decision.

Another option is to not implement getDerivedStateFromProps at all – meaning your child component won’t reset when you move to the next item – but then specify a key on the child that the parent determines using the ID of the item. That will mean that when the key/ID changes, the child is completely remounted. Note that this may be worse for performance in some cases but is quite possibly the semantics you want. (It depends on the use case though.)

Essentially, it boils down to this: When you use getDerivedStateFromProps (or componentWillReceiveProps), your component is guessing at what has happened based on the old and new values. Even if you store the old and new values and compare them properly (as we’ve been suggesting in this thread), you’re making a guess at what the parent wanted you to do. In many cases this won’t cause problems – for any parent–child pair, it may be that the child will always guess correctly because the parent may only change its state in certain ways. But for a fully reusable child, it’s not a good idea to use these methods. Because when you pull them into a new parent that behaves differently, they might do the wrong thing.

@Tolgeros in https://github.com/facebook/react/issues/12898#issuecomment-391946524

Also, would this be a sensible way, in general, to mimic the 16.3 behavior in 16.4?

Yes, I think this looks equivalent to me. I don’t recommend writing code like this though. It is fragile. Let me expand on that.

It needs a copy of ‘value’ kept in its local state, for user experience reasons, but if changes come from above it needs to be able to respond to it.

Just to clarify we’re on the same page: do you realize that just re-rendering parent for any reason whatsoever counts as “change from above”?

For example, if you wrap your component in some popup container that passes closePopup as a prop to its child, and that function is different on every render (so equality checks in Redux containers won’t stop it), and the popup calls setState, your component will re-render and blow away the user input.

I think any code that tries to distinguish “change” from above by comparing whole props objects instead of individual props (or, in the past, just relying on when the method gets called) will lead to bugs like this.

To implement something like you described, it would make sense to me if you at least determined the change by comparing some explicit marker coming from Redux store (e.g. an ID or version field) that explicitly gets incremented when you intend to blow away the local state.

Do you see what I mean?

@dubbha in https://github.com/facebook/react/issues/12898#issuecomment-391990732

Yes, it helps, at least it is more obvious that what I am getting as the second argument in case of setState() is the current, newState, not the prevState instead.

Well yes, the only reason we called it prevState in the docs is because you are returning some state already. So the input state is definitely “older” than what you are creating right now. I was worried people would attempt to mutate the state object if it was just called state.

When asking the “why do I need it” question I was mostly trying to think of the new use cases for the method.

Maybe I’m wrong but I don’t think the change is intended to cover new use cases. It is intended to uncover dormant bugs in existing use cases, as we’ve seen already three times in this thread:

• https://github.com/facebook/react/issues/12898#issuecomment-391847365
• https://github.com/facebook/react/issues/12898#issuecomment-391782913
• And the issue described in this comment

So the use cases are the same, and if you use the method in supported ways, you don’t need to make adjustments. If your code has bugs, they became more prominent, and you need to fix the code to follow the supported patterns (like in the documentation and blog posts).

@andrewBalekha in https://github.com/facebook/react/issues/12898#issuecomment-392001463

For example, in this case, getDerivedStateFromProps triggers in each setState, thus I have always default value, which is prop value (15).

Your code also has the same problem as in three cases above that I described, even in React 16.3.

If the user inputs something but then parent component re-renders for a different reason, you will blow away user input (because it won’t match the state).

For every component you need to decide if some value is controlled or uncontrolled. This is part of your component’s API design.

If it is controlled then you should use the value from props, and not attempt to mirror it in state. When you want to change something, you need to ask the parent to do so. This is called “lifting state up” and is described in detail in documentation. In this scenario you don’t need getDerivedStateFromProps.

If it is uncontrolled then you can take a prop like defaultValue for the first render. But from that point you should use local state to keep the value, and re-rendering from above shouldn’t reset it. In this scenario you don’t need getDerivedStateFromProps either.

Combining controlled and uncontrolled behavior (using getDerivedStateFromProps or other means) is possible but extremely error-prone. I strongly suggest against it. You can see from my three previous bug descriptions in this thread that this leads to the same mistake: a completely unrelated parent re-render will blow away your state.

So what do you do instead if you want to blow away the state when something changes in the parent? You need to figure out what exactly should serve as a trigger for blowing away the local state. For example maybe in some parent there is a state like currentItemID that changes. In that case, you can either:

• Pass that state down to the child so it can use it in getDerivedStateFromProps and decide to blow away the existing local state.
• Or you can give key={this.state.currentItemID} to the child so that when the ID changes, it is forced to be unmounted and then mounted again with new props. This will ensure the “clean slate” without getDerivedStateFromProps but will blow away existing DOM. And in fact that might be exactly what you need if conceptually the “current item” has changed and you don’t want to preserve things like focus, uncontrolled input state, or scroll positions.

Does this explain the problem a little bit? I’m not sure if my explanations are clear enough because I keep describing the same problem in the application code but every new example in this thread demonstrates it again as if it was desired behavior.

@gaearon I don’t have any code examples (and I’ve not even installed 16.4 yet) but I just wanted to comment on what you said earlier…

It seems like more people use getDerivedStateFromProps than we expected, and that is likely due to a misunderstanding about its purpose. For example, it might be that the code that relied on componentWillReceiveProps is always getting updated to getDerivedStateFromProps whereas memoization or lifting state up would have been more appropriate.

I’m surprised you’re surprised about this. I fully expected people to use this method simply because core methods on which, I’m sure, many relied upon previously are being removed. Namely those that allow us to tell when props change externally. I, personally, got the impression that this is almost the new componentWillReceiveProps from reading the docs.

As for me, my use case for getDerivedStateFromProps are components that receive objects to be edited from external sources (usually from Redux via mapStateToProps) and store that data in the local state (which has to be done as you can’t edit property values directly). They also usually some onSave(newData) type callbacks (mapped via mapDispatchToProps) that are triggered when the updated data needs to be persisted. Those callbacks result in new data being pushed into the current component’s props and local state having to be updated.

Before getDerivedStateFromProps existed I had to check for presence of and changes to this.props.something in both - constructor and componentWillReceiveProps - because data could be available during the construction, or alternatively it might’ve been necessary to load it first, meaning it would become available through one of the eventual componentWillReceiveProps calls.

getDerivedStateFromProps is obviously a godsend in that regard and makes the code so much cleaner and neater and I personally can’t see any other way to implement behaviour similar to what I described above. It would definitely be interesting to see how memoization or lifting state up would have been more appropriate as you said. I’m pretty sure I looked into those options but couldn’t find a solution that would work for these use cases. componentDidUpdate can be used in some occasions but sometimes it’s too late and things need to be checked before something gets changed and not after.

You broke semver, plain and simple. I understand your explanations and I understand your reasoning and do not disagree with them. But, when I yarn and get the latest minor release, my stuff shouldn’t stop working because of a minor or patch version dependency change, plan and simple.

While the changes were good intended, that is my major sticking point in all this.

Assign Mark a few code reviews at least!

That’s a good point, @codeaid. It’s can be difficult to strike the right balance between too much and too little detail, but I think our current reference section definitely needs some work.

We hope to eventually create a “recipes” section on the site that shows common tasks and our suggested patterns. Unfortunately there are only a couple of us though, and this sort of thing is pretty time consuming.

But I appreciate the feedback. We’ll try to keep it in mind as we make small edits over the next couple of weeks.

The documentation on the previous blog post seems to completely contradict the changes made in v16.4. Perhaps a note should be added to prevent confusion?

@klimashkin

Then react team should make components stateless and thisless (get rid of classes) to force that approach. If last is an option that react team is considering now (and getDerivedStateFromProps is an early bird), just let us know so we can change mindset in advance.

While we’re always considering alternatives to classes, as we have done for the past four years, local component state is an essential feature of React (pretty much the most important one), and it’s not going away anywhere. I think you might be taking my comment to an extreme I didn’t intend.

But on the other side I don’t understand purpose of getDerivedStateFromProps if you say ‘just lift your state up’ (I assume it doesn’t matter if that upper state holder is another component or redux selector). If you follow ‘just lift your state up’, you don’t need local component state anywhere at all.

I didn’t mean you should always lift it up as high as possible. I meant that it’s bad to have duplicate sources of truth that disagree with each other about any particular value. Lifting state up can solve that problem: basically, it forces you to choose one source of truth, and get the data flowing down from there. I didn’t say “lift all the way up“, just enough to solve the practical issue.

Even if you lift it up to their shared ancestor, it’s still local state in the component that owns it. But if you have two local states in two components that really are supposed to represent the same conceptual value, and “syncing” them is a pain, this is a sign that lifting state up to their common ancestor (but no higher than necessary) can resolve the ambiguity and the bugs it causes.

Lifting state up isn’t a panacea either. I’m only suggesting it as a solution when you’re literally trying to sync two values. I think that if a prop and state field are named the same way, it’s a good rule of thumb that getDerivedStateFromProps might not be an ideal solution because it’s just a symptom of a duplicate source of truth.

But there are legit cases for getDerivedStateFromProps that are not solved by lifting state up.

Sometimes state really is both local and derivative. For example, you may want to reset displayed suggestion list when the user changes a field. The suggestions themselves might be an implementation detail of a component, and you might not want to lift them up (thus causing every parent to manage them). I think this would be a legitimate case for using getDerivedStateFromProps: you reset some local state in response to a prop change, and the parent doesn’t need to be aware the derived value even exists.

If I were to come up with a checklist for when using getDerivedStateFromProps makes sense, it would be something like:

• The derived state is truly local to the component: it doesn’t directly map to any data the parent is already managing.

• The state is only re-derived on specific prop changes. Re-rendering a parent at arbitrary times doesn’t blow away the child state.

• Effectively, this means one of two options. Either this state is accumulated from prop changes (e.g. scrollDirection state accumulated from changes to scrollTop props). Or it’s accumulated “per prop value” (e.g. autosuggest items displayed for current field, which need to reset whenever the field changes).

Probably getDerivedStateFromProps could be renamed to just getDerivedState to stop confusing it with componentWillRecieveProps and emphasize that it doesn’t depend on props change only. Because it literally derives state on each render, not just on props change.

Would it help to say that arguments are just called props and state and represent current props and state?

I thought prefixes would be helpful but I see now they’re more confusing.

Why do I need it

I don’t understand this question. The use cases haven’t changed. If you needed this method in 16.3 and your code doesn’t have bugs (such as the one described above), it should also work in 16.4. So if you “need it” it’s for the same reason you “needed it” in 16.3.

If you don’t need it that’s cool too—it’s not intended to be commonly used. As I already noted a few times in this thread it’s hard to say whether you need it if you don’t show a particular snippet of code.

@bvaughn Looking forward to the article. One thing I’d suggest, and it’s a comment that a few of my colleagues have expressed - lots of the important documentation actually seems to be in blog posts and not the API pages themselves. I would personally prefer to see documentation expanded and blog posts just linking or quoting parts of it. As it stands, there are things (logic, suggestions, exceptions) which are only available in blog posts.

I hear that there was some intentional ambiguity in the spec, but that doesn’t change how folks understood gDSFP to work.

Our original implementation was flawed. For what it’s worth, the RFC spec for getDerivedStateFromProps did make mention of this:

Note that React may call this method even if the props have not changed. If calculating derived data is expensive, compare next and previous props to conditionally handle changes.

And our examples/recipes recommended comparing new and previous prop values before updating state- but we failed to community this warning clearly enough in the API reference docs, and I apologize for that.

---

I, personally, got the impression that this is almost the new componentWillReceiveProps from reading the docs.

We need to do a better job of communicating when getDerivedStateFromProps is appropriate and when other techniques would be better. (See this tweet for an example.) We plan to publish another blog update in the next week or two that covers this in more detail.

It would definitely be interesting to see how memoization or lifting state up would have been more appropriate as you said.

We will be sure to include several examples in the upcoming update about this.

@bvaughn Thanks for writing this. I love how this thread helped crystallize these problems in my head. I’ve been vaguely aware of them but it was hard to articulate what exactly was broken in such APIs. Looking forward to the blog post!

Semver is a social contract and it is a contract a company like Facebook should probably be following, especially with such a huge community of developers utilizing a, dare I say, standard in front-end development at this point in time.

You had functionality in a non-alpha/non-beta release out in the wild for two months. You had cited documentation stating the lifecycle’s behavior. You made claims of componentWillUpdate deprecation and suggested developers start moving to the new modal as soon as possible to prepare for the eventual removal of the deprecated code.

With all of this, I would assume this means you had proper unit and contract testing for each piece of the lifecycle, including getDerivedStateFromProps. I can almost bet that at some point during 16.4’s development tests were added or changed because the behavior changed here. In fact, you can see test file changes in both https://github.com/facebook/react/pull/12802 and https://github.com/facebook/react/pull/12600.

That should have been a red flag that a breaking change was being made. A change to the fundamental lifecycle of React is a big deal.

Was it really a bug? Yeah, I think based on the discussions in this thread and your explanations, it should be classified as an unintended behavior. But this wasn’t a launch-day slip up. This was two months into the release of 16.3. This is a breaking change. It warranted more of a heads-up to the community or it should have been saved for 17.0. This one change wasted the collective community probably several hundred hours of productivity time to figure out why their code just stopped working. I know it took two developers at least two hours to track down why everything just quit working since this specific change omitted no errors.

I also understand that sometimes accidents happen, when a breaking change is pushed unintentionally. But everything you’ve stated in this thread indicate that this was intentional and you knew what you were doing. To me, that is extremely unprofessional and good way to upset your base of users. Something like this, especially if it happens more than once, can easily push someone to a competing framework.

Listen, I like React. It took a long time for me to ramp up on it and feel really comfortable with it. But I like it. I’m not going to go bashing it because of stuff like this. It’s small, it’s petty, and frankly, I have code I need to be writing and arguing with people over the interwebs is not something I like to do.

What I am asking you is to be more vigilant on how and when you release breaking changes in the future.

@kohlmannj For me it is not a bug in react in any case, it’s just these two pieces of code are just not equivalent

public componentWillReceiveProps(nextProps: IScrollViewProps) {
    if (nextProps.scaleX !== this.props.scaleX && typeof nextProps.scaleX === 'number') {
      this.setState({ scaleX: nextProps.scaleX });
}

and

 public static getDerivedStateFromProps(nextProps: IScrollViewProps, prevState: IScrollViewState) {
    if (nextProps.scaleX !== prevState.scaleX && typeof nextProps.scaleX === 'number') {
      return { scaleX: nextProps.scaleX };
}

In cWRP you are comparing old prop value with the new one and in gDSFP you are comparing it with the derived value, that is going to change inside you component. I think now with gDSFP you just have to save the old scaleX value in state, too, to be able to compare new prop against it. Am I correct?

if(nextProps.scaleX !== prevState.oldScaleX && typeof nextProps.scaleX === 'number') {
    return {oldScaleX : nextProps.scaleX, scaleX : nextProps.scaleX}
}

Thanks for elaborating! I think we don’t disagree that much. This rings true to me:

It warranted more of a heads-up to the community

We’ll take this feedback to the heart.

I stand by saying that this is a poorly communicated bugfix for an (also poorly communicated) feature rather than a semver-major change. But we could definitely have explained it better in retrospect, and it’s our fault.

For what it’s worth we did mention in the blog post that unsupported patterns that worked in some cases (but not others) by accident would break more consistently.

I see why you’re saying it was annoying to debug, but it’s not like we made this change silently. In fact a large part of the text in 16.4.0 blog post is dedicated to this exact change. If you saw breakage after upgrading, it would make sense to search your code for getDerivedStateFromProps (and patterns described in the post) rather than try to find a bug for two hours without that context. I’m not sure whether this is what happened, and I’m definitely not saying it’s your fault, but we did make effort to explain what issues you may run into.

Above all I appreciate everyone’s willingness to engage in this conversation. Again, we’re sorry we let the original version stay out for enough time for you to start depending on the buggy behavior. In fact the biggest reason we scrambled to get 16.4 out sooner was because we realized the mistake a few weeks ago (and we’ve been testing the impact of the fix for these past two weeks, which gave us confidence that the change is scoped enough and the existing bugs it uncovered were important enough to ship the fix now).

you edit stuff, trigger an action, make an API call and get new data from Redux that has to replace the current state

How do you determine that data coming from above is “new”? I don’t understand this part.

Judging by your description, you don’t compare any IDs or anything. So you’re just hoping that your components never get re-rendered unless something specific happens on the Redux side (e.g. a data fetch). This is a very fragile assumption.

As far as I can tell adding a single always-changing value to the mapStateToProps result (e.g. something: {}) will cause your component to always re-render on any action (not just the ones you care about), and always reset the state. Isn’t that bad? Adding a property or selecting more data from Redux (that might change more often than “top-level” data you’re selecting) shouldn’t break your component.

In other words, you seem to be relying on a performance optimization (React Redux currently bails out of rendering shallowly equal props) for correctness. This sounds like it will become a bigger problem later on (e.g. if React Redux stops using shallow equality checks everywhere).

It’s hard for me to say more without seeing the code but that’s the basic idea.

How do you solve this issue correctly? I believe I gave some pointers in https://github.com/facebook/react/issues/12898#issuecomment-392035163.

If you need this kind of behavior, you need some explicit marker that tells you the data is actually “new” (e.g. ID or version field is different) so it needs to be replaced. Your components should be resilient to being re-rendered.

Again, seeing a sandbox with a minimal code example would help.

@RoyalHunt

Actually I have some breaking problems as well with using apollo 1.

Looking at your “before” code in https://github.com/facebook/react/issues/12898#issuecomment-391847365, it has the exact same bug as I described in https://github.com/facebook/react/issues/12898#issuecomment-391782913.

So here, too, React 16.4 didn’t break your code. It helped you find the bug that already existed (previously unnoticed) in your code and manifested itself less deterministically.

Sounds like a good thing to me.

No, you’re not the only one, @dubbha!

Andrew and I were talking about this earlier this week and decided to update the docs to just be props and state like in the most recent blog example but I guess neither of us have made this change yet. I will do it now!

Edit: Docs updated via reactjs/reactjs.org/pull/910

This didn’t really make sense and was an oversight in the initial implementation.

@gaearon can you expand on this? The spec’d behavior in the RFC never mentioned this, so it seems like it was an issue with the initial API design and not it’s implementation?

As explained both in API reference for componentDidUpdate and blog post, you should always make a comparison in componentDidUpdate to decide whether to do something.

componentDidUpdate(prevProps, prevState) {
  if (prevProps.something !== this.props.something) {
    // do something
  }
  if (prevState.somethingElse !== this.state.somethingElse) {
    // do something else
  }
}

Here is a few smaller examples (with a counter) if someone prefers to see the code.

Before (using componentWillReceiveProps): https://codepen.io/ismail-codar/pen/gzVZqm?editors=1011

Demonstration of why this code already has a bug: https://codepen.io/anon/pen/jxgXgK?editors=1011. Click “demonstrate bug” and you’ll see that completely unrelated setState in parent blows away the child state.

A correct solution that needs neither componentWillReceiveProps nor getDerivedStateFromProps, and instead lifts state up to the parent so it can decide what to do in either case: https://codepen.io/anon/pen/erqXpz?editors=1111. Note how I am now forced to deal with the complexity of this UI. For example, I had to explicitly decide what needs to happen if I edit the “initial value” field after interacting with the counter. In my implementation I ignore these edits, but you can do something else. The important part is that this is now explicit.

I want to add a note to https://github.com/facebook/react/issues/12898#issuecomment-392266811 before I forget about it again.

Variant 2: State is only reset when props value changes

This pattern compares incoming props to previous props and only updates state when the value changes externally. (This is done to avoid the unexpected re-render problem I mentioned above.)

However this also has a downside: There’s no good way for the parent to reset a property to the previous value.

Here is another scenario where this is a bad solution. Often people want to keep edits to a form in local state like state.value (and “commit” them to some place above, such as a Redux store, on an event like button click).

Let’s say we have a UI like this:

[button: View Previous]
[button: View Next]

Edit value: [input: Value]

[button: Save] 

You can navigate items with Previous and Next buttons (and the text input should reset to the current values for them), and you can click Save to save your edits.

To implement this, people sometimes try to use componentWillReceiveProps or (now) getDerivedStateFromProps. The logic goes like this: we initialize state.value to props.value. And if props.value changes “from above“, we want to throw away local edits (we assume the change from above comes due to Previous or Next click). However, this logic is buggy.

Consider the case where we navigate between two items with Previous and Next buttons. If their values are different, sure, a re-render will reset the field.

But what if two neighbor items have the same value? Then the condition you’re using to determine it (something like props.value !== state.previousValueFromAbove) will be false, and you’ll keep local edits even though the current item has changed, and they’re not valid anymore!

On the other hand, if your condition looks like props.value !== state.value (and compares to current input value), apart from your code breaking in 16.4, in 16.3 you would also get buggy behavior I described in earlier comments (every parent re-render would blow away your state).

So how can you make this pattern work? Really, the best way is to avoid it, and instead explicitly make the component fully controlled (so that the state can be managed by the component above).

But if you must do something like this, you can accept a prop like current item ID, and keep the previous ID in state. Then in getDerivedStateFromProps() you know the current item has changed and you need to reset the state. Just having the current value changing from above doesn’t really tell you enough to make a decision.

@bvaughn Excellent notes — thank you very much. As it happens, I tentatively considered a change to fully uncontrolled in https://github.com/kohlmannj/limina/commit/d68e23c7bcb8cbbb872fd4d714848e5826673ed1. Based on this discussion, I will work on entirely separating controlled and uncontrolled behavior as I continue development.

Thanks. Btw I’d be happy to look at the issue in your input component but no earlier than Tuesday (due to holidays).

@gaearon Ok, thx, I removed my comment. I have been heard which means we’re not being opressed here. I had to double-check that. It’s good that React is having a renaissance and is evolving again instead of just “dying off” like yet another used-to-be-so-hyped framework. Your changes are controversial but I’m fine with that.

@karolk Did you have a chance to read my reply or the sections of Dan’s reply that I highlighted? (Sorry, our comments were posted at the same time. 😄)

It seems like you’re aiming for an uncontrolled component, in which case we strongly recommend that you don’t try to mirror props in state like your component is doing. Instead, you can initialize state with a default value from props like so:

class Test extends React.Component {
  constructor(props) {
    super(props);

    // Read default text from incoming props (store),
    // But then don't try to mirror it to state.
    this.state = {
      text: props.defaultText
    };
  }

  changeText(newText) {
    this.setState({ text: newText }, () => {
      // Notify store of text change if some criteria is met.
      if (this.state.text.match(/\d+/g) === null) {
        dispatch(this.state.text);
      }
    });
  }

  render() {
    return (
      <input
        onChange={({ target }) => this.changeText(target.value)}
        value={this.state.text}
      />
    );
  }
}

Even for React 16.2 (before there was getDerivedStateFromProps) mirroring props and state in the way your fiddle shows was not recommended because it can result in a confusing API or confusing runtime behavior:

• What’s the source of truth when the two disagree?
• Unexpected re-renders or changes to unrelated props like callback functions or inline styles (which are often re-created inline during render) could blow away state unintentionally.
• If state is reset to props every time the component re-renders, it is entirely unnecessary and you should just use the props value instead.
• If state is only updated when the specific field in props changes, then the parent component has no way to forcefully reset to the previous props value without rendering twice (first to change the value arbitrarily and again to change it back to the target).

For example, in this case, getDerivedStateFromProps triggers in each setState, thus I have always default value, which is prop value (15).

class RangeInput extends React.Component {
  state = {
    value: this.props.value,
  }

  static propTypes = {
    value: PropTypes.number.isRequired,
  }

  static getDerivedStateFromProps(nextProps, prevState) {
    if (nextProps.value !== prevState.value) {
      return {
        value: nextProps.value,
      }
    }

    return null
  }

  onChangeRange = e => {
    this.setState({ value: parseFloat(e.currentTarget.value) })
  }

  render() {
    const { value } = this.state

    return (
      <div>
          <input
            type="range"
            min="0"
            max="100"
            step="0.5"
            value={value}
            onChange={this.onChangeRange}
          />
        </div>
    )
  }
}

ReactDOM.render(<RangeInput value={15} />, document.querySelector("#app"))

is here another workaround?

Am I correct in understanding that an updated version of this diagram would have gDSFP stretching to the right, such that setState will now also trigger it (but not forceUpdate)?

Also, would this be a sensible way, in general, to mimic the 16.3 behavior in 16.4? As in, I don’t want gDSFP to do anything if the props did not change. Can I just do a basic object reference equality comparison of the props object (rather than comparing specific props) to solve the problem in a general sense?

Before:

static getDerivedStateFromProps (nextProps, prevState) {
   return {
      value: nextProps.value
   }
}

After:

static getDerivedStateFromProps (props, state) {
   if (state.prevProps !== props) {
      return {
         value: props.value,
         prevProps: props
      }
   } else {
      return null
   }
}

To add some background, these methods would be used by components that will do setState of value and call a props.onChange method in the setState’s callback parameter to bubble up the change to the real source of truth (which may be some top level state or Redux store). It needs a copy of ‘value’ kept in its local state, for user experience reasons, but if changes come from above it needs to be able to respond to it.

_handleChange = (event) => {
   this.setState({value: event.target.value},
      () => this.props.onChange(this.state.value)
   );
}

@gaearon No, no confusion, just poor copy/pasting. I immediately edited it. Thanks.

The RFC does say:

Note that React may call this method even if the props have not changed.

We didn’t want to over-specify when exactly this happens because there might be multiple reasons. setState was meant to be one of them but we missed this ourselves (in part because we left it vague in the RFC) and only realized the omission after seeing bug reports caused by this.

Again, the new behavior is critical to making getDerivedStateFromProps safe for async mode. If we didn’t do it, migration from componentWillReceiveProps would have largely been pointless.

None of the intended usage examples we provided in RFCs, blog posts, or docs would break with the new behavior.

Updated to:

TLDR: if this change broke your code when you moved from 16.3 to 16.4, your code was already buggy in subtle ways. The change in React 16.4 made these bugs in your product code occur more often so you can fix them. We don’t consider making existing bugs in your code reproduce more reliably to be a breaking change.

If you’re coming from an issue in some third-party library that works with 16.4, but doesn’t work in 16.3 — you’re hitting the React bug. This issue is a complaint about this bug getting fixed in 16.4. However, we think fixing it was the right choice. We recommend everyone to upgrade to React 16.4 or later.

I think this should be clearer?

Here’s the blog post!

https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html

@gaearon

While we’re always considering alternatives to classes, as we have done for the past four years, local component state is an essential feature of React (pretty much the most important one), and it’s not going away anywhere.

Thanks for stating this, it reassures me a lot. I’m not a functional purist - I use local state a lot, combined with MobX observables. Just know that there are people like me out there 😃

@catamphetamine The example code you show above looks more like a controlled component to me. (It does not store the current phone number “value” in state; it gets it from props.)

In the example you show above, you don’t actually need to use component state at all. The core of what you’re doing is deriving something (which flag to show) from props in a way that performs well. All you need to do this sort of thing is a memoization wrapper.

If you think otherwise you can answer. But don’t write lengthy comments.

👍

@gaearon We didn’t knowingly upgrade. I wasn’t even aware 16.4 had dropped until someone on my team said, “Hey, look at the React version, it changed.”

Our company’s policy is ^, so we automatically got the latest version on our next fresh pull, and subsequent yarn, of the codebase, which just happened to be that same night. Under normal circumstances, we may not have discovered the issue for a few days. By that point in time, I’m sure one of us would have saw the release announcement and would have attacked things in a different way.

@bvaughn I went for a combination of parent controlled and state but this seems to depend on implementation detail. I don’t think there’s anything I don’t get in @gaearon’s explanation. Thank you both for your time, I appreciate quick response here and on Twitter.

Simpler example of the issue: https://jsfiddle.net/6fme1y69/2/ It seems like coming from componentWillReceiveProps in 16.2 to getDerivedStateFromProps in 16.3 it was very easy to refactor the code to something that stopped working properly in 16.4.

@gaearon, thanks I’ve got my problem now.

Here is an example where my code is failing in 16.3 as well https://codesandbox.io/s/j809y1llv

Actually I have some breaking problems as well with using apollo 1. But I solved this with the next code:

static getDerivedStateFromProps(nextProps, state) {
    const savedArticles = nextProps.data.articles
    if (savedArticles && !state.firstRender && savedArticles !== state.savedArticles) {
      return {
        savedArticles,
        firstRender: true
      }
    }
    return null
  }

Before it was:

static getDerivedStateFromProps(nextProps) {
    const savedArticles = nextProps.data.articles
      return {
        savedArticles,
        firstRender: true
      }
  }

The 16.3.2 version acted more in lines with the former shouldComponentUpdate, which this method replaced. In the pre-16.4 behavior, it was much easier to migrate from the deprecated shouldComponentUpdate to the new getDerivedStateFromProps.

Sorry, but this seems like a significant confusion on your side. shouldComponentUpdate was never deprecated. It also has nothing to do with what getDerivedStateFromProps is for.

(That was in response to a typo, now edited)

yeah i was also surprised here, the old behavior was more waht i’d have guessed the behavior would be from reading through the RFC