Discussion: componentWillReceiveProps vs getDerivedStateFromProps

[arieh]

Hello

We have found a scenario where the removal of componentWillReceiveProps will encourage us to write worse code than we currently do.

We currently consider props to be a valid form of input and state for our component. If a component received a prop, we do not duplicate it into our state as to maintain a single source of truth. This is especially true when using state management libraries where the state is externalized from the component but will also be true for many other scenarios.

However, with the removal of componentWillReceiveProps, react will force us to use setState for any prop we would like to monitor for change. Basically, any type of prop change that would also trigger a following action (be it internal one or an external one) would require us to duplicate the prop into the state.

To give a sort of clean example, let's imagine a dialog that has a visible flag and would like to report when it was hidden to a logging framework. Currently, we can do:

componentWillReceiveProps(newProps){
      if (this.props.visible === true && newProps.visible === false) {
           registerLog('dialog is hidden'); 
      }
}

With the new paradigm, I will need to do:

static getDerivedStateFromProps(nextProps, prevState){
        if (this.state.visible === true && newProps.visible === false) {
           registerLog('dialog is hidden'); 
       }
        return {
               visible : nextProps.visible
        };
}

Aside from already duplicating the props, in my opinion this will heavily encourage users to do

static getDerivedStateFromProps(nextProps, prevState){
        return {
           ...prevState,
           ...props
        };
}

Since it takes away a lot of the usefulness of props.

[alexkrolick]

This issue should probably be filed on the React repo or the RFC repo rather than docs, but... 🤥nvm

1. You can still use props in render, no need to copy into state unless you are computing a new state using new props and current state (e.g. resetting values after a page transition or showing a confirmation dialog).
2. You can return null from gDSFP if you want to perform side-effects based on prop changes without doing anything to state (your example could do this)
3. If you need access to the component instance (this), you should be able to use componentDidUpdate to perform side-effects (note: after the commit/render phase)

[SimenB]

Came to report the same thing. In our case, we use https://github.com/final-form/react-final-form, and wanted to clear the value of one field when another field changed (think 2 dropdowns, one with car manufacturer and one with model - when manufacturer changes, clear model). Then we had to compare old props with new props without putting it in state, same as OP.

We've since migrated to use https://github.com/final-form/final-form-calculate which allows us to do the same thing declaratively, but I still think it's a valid use case.

(I guess the answer, if we had not moved away from it, would have been "lift state up". Which is fair enough 🙂)

This issue should probably be filed on the React repo or the RFC repo rather than docs

The blog post encouraged opening issues here: https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#other-scenarios

[alexkrolick]

Alright, let's leave it open then

[timroes]

I added another use-case into (the duplicate) issue #729

[gaearon]

Can you explain why componentDidUpdate doesn't work for this?

Before:

componentWillReceiveProps(newProps){
      if (this.props.visible === true && newProps.visible === false) {
           registerLog('dialog is hidden'); 
      }
}

After:

componentDidUpdate(prevProps){
      if (prevProps.visible === true && this.props.visible === false) {
           registerLog('dialog is hidden'); 
      }
}

That’s the exact use case for componentDidUpdate and it’s more accurate than componentWillReceiveProps because it only fires when the prop has actually changed.

[danburzo]

I think in some cases moving a prop to state is the right solution.

Assume you need to perform an impossibly expensive computation based on a prop, and you want to make sure you only perform it when the prop has actually changed. And supposedly you need the computed value for rendering.

In this case, instead of doing:

class Component extends React.Component {

  constructor(props) {
    super(props);

    this.state = {
      computed_prop: heavy_computation(props.value)
    };
  }

  componentWillReceiveProps(newProps) {
    if (newProps.value !== this.props.value) {
      this.setState({
        computed_prop: heavy_computation(new_props.value);
      })
    }
  }
}

we can instead:

class Component extends React.Component {
  static getDerivedStateFromProps(props, current_state) {
    if (current_state.value !== props.value) {
      return {
        value: props.value,
        computed_prop: heavy_computation(props.value)
      }
    }
  }
}

(Of course it would be nice not to store this.state.value based on this.props.value — so I do wonder if there is another way to do this more elegantly)

[arieh]

@gaearon - I do not see a great difference other than a mind shift I guess. I would suggest in this case to add this transition to the docs maybe.

[arieh]

Since I see the title changed on this topic I'll add another use case that becomes harder (haven't interacted with react issues before so please let me know if I should open a new ticket instead :) )-

We currently use willReceive to do various pieces of logic in preparation for render (input parsing, sorting etc). Although technically getDerivedStateFromProps can be used for this, one issue that arrises is that if previously we could use class methods to organize our code, we now have to either have a lot of code in our getDerived or to use non-method functions for it. This feels like it's forcing us to take code that used to belong to the class now have to stay outside it and then code around these limitations (pass state around as arguments to functions, avoid using this/wrap/bind etc) due to the static nature of the new function.

[gaearon]

I don’t have a great answer for you here—indeed class methods like this tend to have code that’s unsafe for async which is why we want it to be static. So yes, you’d need to pull that out if you want the code to be async-friendly. One upside of doing this is that it should be easier to test because it’s separated from the rest of the class logic.