[0006-static-lifecycle-methods] How does getDerivedStateFromProps access the current props? about rfcs HOT 23 CLOSED

I don't get it. I want to compute some state from the props, but unless I know what the current props are, I'm not sure how I can avoid performing a potentially expensive operation.

e.g. I have some data coming in as a prop (an array with many elements). I want to to normalize this data and then put it in the state.

getDerivedStateFromProps sounds like it would be perfect for this, however, if some other prop changes, I really don't want to run this normalization step again because my data hasn't changed.

Previously I could check if(this.props.data === nextProps.data) in componentWillReceiveProps but now I can't.

Why do I have to store two copies in my state now? One for the normalized data, and another one for the unnormalized data so that I can check against the unnormalized data in the props This seems like a common use-case that used to be easy and now is hard, and now I have this dangling unormalizedData state that I don't even care about.

from rfcs.

"Why can't I use prevProps" question already was asked many times. Because there is not previous props then component is initialized.

Couldn't prevProps just be null or undefined in this scenario? Why does there need to be an "additional check"? If developers want to ignore prevProps they can.

Your example will work AFAICT, it just feels like a downgrade from componentWillReceiveProps though. prevData shouldn't even be in the state IMO. Also, we can't defer setting the state with getDerivedStateFromProps, which means if normalizeData is async, we're out of luck, not even reselect will save us. With componentWillReceiveProps we could call this.setState whenever we were ready.

I guess the only choice is to use componentDidUpdate then? Seems strange to me to have to wait until after the component is rendered before I'm allowed to to start processing the new props.

e.g. This feels simpler to me but it has a silly caveat:

class MyComponent extends React.Component {
    state = {
      normalizedData: {}
    }
  
    async componentDidUpdate(prevProps, prevState, snapshot) {
      if (this.props.data !== prevProps.data) {
        this.setState({loading: true}); // <-- will immediately cause another render after we *just* updated
        const normalizedData = await normalizedData(this.props.data);
        this.setState({loading: false, normalizedData});
      }
    }
}

from rfcs.

It's better to discuss the "why no prevProps param" issue in #40- but I think there's a couple of misunderstandings in the above thread that are worth addressing separate from that argument.

From comment:

I used to store things that don't affect rendering under this. too, but since getDerivedStateFromProps is static, we can't really do that anymore either.

Assuming these things are derived from (or otherwise related to) props - storing them on the class instance is usually not safe. This is the main reason why getDerivedStateFromProps is static.

This section of the React docs explains the difference between "render" and "commit" phases as does this lovely diagram. Computing and storing values on the instance from a render phase method (like componentWillReceiveProps) is usually unsafe because React may invoke render phase lifecycles without committing at all (because of an error or a higher priority interruption). In this case, your instance values could be based off of the wrong props. React manages this.props and this.state for you though, and always resets them to the correct values before calling your component lifecycles.

From comment:

e.g. This feels simpler to me but it has a silly caveat:

Sync rendering behavior for setState calls made from componentDidMount/componentDidUpdate is actually a very important feature for libraries that need to do things like measure or position an element after rendering.

from rfcs.

// nextProps.keys = [1, 1, 2, 3] will never happen
// nextProps.keys = [1, 2, 3, 1, 4]; is valid and this.cache = [0, 1, 3, 6 ,7 ,11]

I don't understand this.

this.state.index = fn1(this.cache);

You should not directly mutate state like this. You should use setState. (I assume this may have been a typo and you may have meant to write nextState.index = ...)

And you suggest that I should move this.cache to this.state. Am I correct?

Maybe.

Since you're mutating cache, it would actually be safer to move what you're currently doing into componentDidMount/componentDidUpdate. If you were to move it to getDerivedStateFromProps (or leave it inside of componentWillReceiveProps) it's possible that you will mutate the cache unnecessarily. (This is explained in more detail in RFC 6 as well as my comment here.)

If you need to update cache before rendering, then a safer approach would be something like this:

class extends React.Component {
  state = {
    cache: [],
    index: 0,
    prevKeys: [],
  };

  static getDerivedStateFromProps(nextProps, prevState) {
    if (nextProps.keys !== prevState.prevKeys) {
      // Figure out 'i' by comparing nextProps.keys to prevState.prevKeys
      let i = 0;
      while (i < nextProps.keys.length && prevState.prevKeys.keys[i] === nextProps.keys[i]) {
        i++;
      }

      const cache = this.fn0(nextProps.keys, i);
      const index = this.fn1(cache);

      // Update state
      return {
        cache,
        index,
        prevKeys: nextProps.keys
      }
    }

    // Nothing to update in state
    return null;
  }

  fn0(keys, max) {
    // Don't mutate cache
    const cache = this.cache.slice(0);
    for (let i = 0; i <= max; i++) {
      cache.push(cache[cache.length - 1] + keys[i]);
    }
    return cache;
  }
}   

That being understand, I don't really understand what the partial code snippet you shared is doing so I'm just taking a guess.

from rfcs.

As for the answer 1, nextProps and prevProps will become redundant. I want to know whether it is a valid statement? If so, I hope you guys consider to remove it in the future.

As for the answer 2, I am not directly mutating an array from props. Here is an example

class extends React.Component {
  constructor(props) {  // props.keys = [1, 2, 3]
    super(props);
    this.cache = [0];
    this.fn0(props.keys); // this.cache = [0, 1, 3, 6]
    this.state = {};
    this.state.index = 0;
  }

  fn0(keys) {
    for (let i = 0; i < keys.length; i++) {
      this.cache.push(this.cache[this.cache.length - 1] + keys[i]); // this.cache[n] = this.cache[n - 1] + keys[i]
    }
  }

  componentWillReceiveProps(nextProps) { 
    // this.props.keys will keep growing and existing elements will never change
    // nextProps.keys = [1, 1, 2, 3] will never happen
    // nextProps.keys = [1, 2, 3, 1, 4]; is valid and this.cache = [0, 1, 3, 6 ,7 ,11] 
    let i = 0;

    while (i < nextProps.keys.length && this.props.keys[i] === nextProps.keys[i]) {
      i++;
    }

    fn0(nextProps.keys.slice(0, i));
    const nextState = Object.assign({}, this.state);
    this.state.index = fn1(this.cache);
    this.setState(nextState);
  }
}    

And you suggest that I should move this.cache to this.state. Am I correct?

Anyway, this is what I have observed you guys try to force us to get rid of all data members and move them to the state with that small static method.

from rfcs.

If the goal is not necessarily handling state but wanting to reduce the performance cost of an expensive one-way computation ny not executing it when data has not changed. Perhaps rather than using getDerivedStateFromProps a better fit for this use case is something like reselect's memoized selectors in render.

from rfcs.

@wood1986 current props cannot be accessed in getDerivedStateFromProps lifecycle. It's pure function. You just need to store new prop in state.
You can read this thread for more information.

from rfcs.

@TrySound this sounds odd because inside the component there will be two versions of props
state.props and this.props Yes, as long as you know what you are doing, this is fine. But I do not feel well with two versions of props

from rfcs.

@wood1986 You have two version of props when you use componentWillReceiveProps and componentDidUpdate. It's not different, except you save that data explicitly by yourself.

And what is wrong for you to keep previous props in state? Do you think it may introduce performance regression or something else?

from rfcs.

I think my issue is not about the performance. It's main about the code migration from 16 to 17 and the code consistency.

In the Basic example, the comment inside gDSFP says

Called after a component is instantiated or before it receives new props.

Regardless of *context, does it means starting from 17
shouldComponentUpdate(nextProps, nextState) -> shouldComponentUpdate(nextState)
componentDidUpdate(prevProps, prevState) -> componentDidUpdate(prevState)?

If so, I am totally fine. If no, then this is odd part because shouldComponentUpdate and componentDidUpdate still hold nextProps and prevProps. And this is the inconsistent part.

from rfcs.

@wood1986 The problem with prevProps in gDSFP is that is does not exists on first call. So you need to extra check if it's not null and one of it's prop is not changed. This is overhead. State solves that problem via initial value.

from rfcs.

I get this part. If this.state will have props information via the implementation ofgetDerivedStateFromProps, what is the point of having nextProps and prevProps in shouldComponentUpdate and componentDidUpdate respectively?

from rfcs.

They just wasn't touched yet.

from rfcs.

Hey @bvaughn,

1. Could you tell us that if we have to put the props into state via getDerivedStateFromProps, do you need the argument nextProps and prevProps in shouldComponentUpdate and componentDidUpdate?

2. How can I update the data member inside the getDerivedStateFromProps before giving the next state which depends on updated data member?
   Currently I have a scenario that I need to re-compute some elements of an array which is the data member of the React.Component on nextProps is received in componentWillReceiveProps. The next state depends on the result of this array. this.state.index is for accessing the result of this array. I do not want to put the whole array into the state and this array cannot be static.

from rfcs.

1. Could you tell us that if we have to put the props into state via getDerivedStateFromProps, do you need the argument nextProps and prevProps in shouldComponentUpdate and componentDidUpdate?

We have no current plans to change the signature for methods like shouldComponentUpdate and componentDidUpdate with regard to next/prev props.

2. How can I update the data member inside the getDerivedStateFromProps before giving the next state which depends on updated data member? ...

I don't really understand your question. Can you provide an example?

What inputs (in your current componentWillReceiveProps) determine whether you "re-compute" elements or not. Can you just mirror those inputs in prevState so that you can access them in getDerivedStateFromProps (as shown here)?

It sounds like you may be mutating an array (or objects within an array) that's passed in via props. This is not particularly safe. Generally we would suggest you store a filtered or mapped version of that array in your component's state and work with it.

from rfcs.

Yes you are right. It was a typo. It should be nextState.index = this.fn1(this.cache)

Anyway, thanks for your answer! I need to prepare for that change. I hope I can share the real code with you in the future because it is related to “react-virtualized”.

from rfcs.

You are welcome. Good luck!

from rfcs.

Hey @mnpenner, I am not giving you an advice. I just want to share my experience and I am not saying they make a good or bad move.

In my scenario, props.data is a simple int array and is used to compute a lookup table.

Before 16.3, I stored this lookup table in this instead of this.state. The reason of putting it in this was any changes to this lookup did not need to trigger a rendering immediately. I had a theory was any parameters affecting the rendering should be in this.state. Here is the sample and this.lookup also has contained the props.data.

this.props = [1, 2, 4];
this.lookup = [
  { "key": 1, "value": 1},
  { "key": 2, "value": 3},
  { "key": 4, "value": 7}
]

As I am doing the 16.3 migration for my lib, I have to move this.lookup table to this.state in order to have a similar 16.2 comparison between prevProps and nextProps. Luckily, this.lookup has contained the props information and my comparison has not been a simple reference check at the very beginning (before 16.2). So I still survive.

I guess your normalization process from this.props.data to normalizedData is irreversible (my scenario is reversible). This means if you only have normalizedData, you cannot get the original this.props.data. If it is reversible, you should be able to do the comparison without introducing the this.state.unormalizedData. But the downside is you need to have some special logic for the comparison.

from rfcs.

@wood1986 Even if I could denormalize it, it would be nearly as expensive as the normalization algorithm. There would be no point, I'd just normalize every time.

I used to store things that don't affect rendering under this. too, but since getDerivedStateFromProps is static, we can't really do that anymore either.

from rfcs.

@mnpenner You don't have to store two copies of your state. You just store references for the next comparing. And it's do not introduce to you any cost except probably more characters for it. For me this api is much better than any other because it's explicit and allows to type code well.

"Why can't I use prevProps" question already was asked many times. Because there is not previous props then component is initialized. Passing any value will require additional check which makes code less readable. With state you have defaults in initial state which you may compare with new props. So the problem is solved.

There is not any special code here. It's dead simple I'd say.

class MyComponent extends React.Component {
  state = {
    prevData: [],
    normalizedData: {}
  }

  static getDerivedStateFromProps(props, state) {
    let state = null
    if (state.prevData !== props.data) {
      state =  {
        ...state,
        prevData: props.data,
        normalizedData: normalizeData(props.data)
      }
    }

    return null
  }
}

from rfcs.

@mnpenner If normalized data is async then you should use componentDidMount and componentDidUpdate (you have nextProps and prevProps here), which are safe place for side effects in opposite to componentWillReceiveProps.

You will always have to do null check. You do not get anything helpful here, just more arguments with harder types and runtime check which may be omitted with state.

class MyComponent extends React.Component {
  static getDerivedStateFromProps(props, prevState, prevProps) {
    let state = null
    if (!prevProps || prevProps.data !== props.data) {
      state =  {
        ...state,
        normalizedData: normalizeData(props.data)
      }
    }

    return null
  }
}

from rfcs.

Just an FYI, somebody opened an RFC to add prevProps to the getDerivedStateFromProps API over at #40.

They're looking for real use cases to justify adding it, so perhaps you can share over there if interested.

from rfcs.

I am sorry if I have overlooked this, but to summarize, reading through this and linked comments, what I understand is that main reason for not having the current props as the third argument is that it will be a potential memory consumption/leak (and prevProps may even be included from other lifecycle methods for some reason, not sure why though). Is there any other reason?

from rfcs.