Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

16.4 broke getDerivedStateFromProps() #12912

Closed
catamphetamine opened this issue May 26, 2018 · 3 comments
Closed

16.4 broke getDerivedStateFromProps() #12912

catamphetamine opened this issue May 26, 2018 · 3 comments

Comments

@catamphetamine
Copy link
Contributor

catamphetamine commented May 26, 2018

In your recent blog post you announced a "bugfix" for getDerivedStateFromProps():

getDerivedStateFromProps is now called every time a component is rendered, regardless of the cause of the update.

But in reality it broke a real-world library.

Now getDerivedStateFromProps() gets called on every re-render.

Suppose a component watches for a property change.
The recommended approach is to store the property value in state, like this.state.value.

Then, a user changes the input value, so both this.state.value and the property must be updated:

onChange = (event) => {
  this.setState({ value: ... })
  this.props.onChange(...)
}

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

The timeline is:

  • First both this.props.value and this.state.value are undefined.
  • Then onChange(event) handler triggers and calls this.setState({ value: ... }).
  • Because the state got updated getDerivedStateFromProps() is called in the new React 16.4.
  • props.value hasn't been updated yet, but state.value has, so the if condition triggers because props.value === undefined and state.value = .... Hence this.state.value gets reset by this if condition and becomes undefined (which is already a bug) because props are the "single source of truth" as per the official React recommendations.
  • Next, this.props.onChange(...) line executes which updates this.props.value which in turn calls getDerivedStateFromProps() again.
  • The if condition triggers again because props.value === ... and state.value = undefined and so this.state.value becomes ... again.

In my case it actually breaks the phone number input component:

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

The design requirement is to let a user set value externally which in turn must update the country flag icon.
It worked in React 16.3.
In React 16.4 though getDerivedStateFromProps() gets called on each internal state update resulting in the country flag being reset every time a user types a character (because an incomplete phone number can't be a source of a country flag, e.g. USA and Canada both start with +1, not to mention the whole NANPA region).

@DavidBadura
Copy link

DavidBadura commented May 26, 2018

You made a mistake in your code:

  1. getDerivedStateFromProps is a static method, so you should not call methods on this like setState
  2. You should return the changed state

The right way is:

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

  return null;
}

@aweary
Copy link
Contributor

aweary commented May 26, 2018

Please refer to #12898

@aweary aweary closed this as completed May 26, 2018
@catamphetamine
Copy link
Contributor Author

catamphetamine commented May 26, 2018

@DavidBadura true, corrected the code samples. the issue still persists though.
Anyway, I'll rewrite my code in some way so that it works with 16.4.

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

No branches or pull requests

3 participants