Should getDerivedStateFromProps suppress 'state not initialized' warning?

[Jessidhia]

Do you want to request a feature or report a bug?

Either a bug or a feature, depending on how the team sees it 🤷‍♀️

What is the current behavior?

The presence of static getDerivedStateFromProps does not suppress the warning about state not being initialized, even if it is always called, even on first render.

class Foo extends React.Component {
  static getDerivedStateFromProps(props) {
    return { a: props.a }
  }

  /* // uncommenting the constructor silences the warning, but is redundant work
  constructor(props) {
    super(props)
    this.state = { a: props.a }
  }
  */

  render() {
    // this is slightly silly, but is an example
    return this.state.a
  }
}

What is the expected behavior?
getDerivedStateFromProps is always called, even on initial render, so maybe it should be considered as having initialized the state. This means that the second argument to it should be allowed to be undefined, as it would be on the first call if state is not initialized by the user. This change would not be a breaking change, as it depends on the user breaking from the pattern of initializing state in the constructor, although it brings memories of createClass's getInitialState.

Alternatively, it should not be called on initial render, and the class implementer would call its own getDerivedStateFromProps and spread its result on its initial state definition. This is clearly not possible anymore now that 16.3.0 is out.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

16.3.0

[TrySound]

@Kovensky Yep, that's the case. The idea is to have the second argument always available what is not possible with passing previous props. So you should always initialize state and it will be passed to gDSFP so you could add only necessary check without checking existence. Also gDSFP can return null so the state can be uninitialized in this case.

[Jessidhia]

It's unfortunate if you are using types; especially if the initialization logic is the same as the getDerivedStateFromProps. You end up having to execute it twice on the initial mount, the first time for the initial value of state so it can be given to getDerivedStateFromProps.

It's logically fine because it's supposed to be a pure method, at least.

[TrySound]

@Kovensky Not quite. Initial state can usually contain empty values which will be "filled" with gDSFP

class Foo extends React.Component {
  state = {
    prevMyList: [],
    list: []
  }

  static getDerivedStateFromProps(props, state) {
    if (props.myList !== state.prevMyList) {
      return {
        prevMyList: props.myList,
        list: props.myList.map(d => d.name)
      }
    }
    return null;
  }

  render() {
    return this.state.list
  }
}

[gaearon]

Yep, that's what I'd recommend. 👍

[pke]

However in my code, state is null on the first call of gDSFP. How can that be?
State is initialised like in your example, yet its null.

[gaearon]

@pke Provide a reproducing case please?

[pke]

Can't provide a case (closed source) just my observations in that code:

First I have to slightly correct myself, the initial state handed into gDSFP is not null but an empty object.
When there no ctor defined in the class gDSFP function is called with an empty prevState

When I add a ctor its called before gDSFP (twice because of React.Strict wrapping) and the prevState is then correctly initialised.

[gaearon]

It would really help if you could reduce this to a minimal example and file a new issue. Thanks!

[pke]

Set up a repo to reproduce this but couldn't. Need to figure it out in my main project.

[pke]

Found the problem. React told it all the time in the console:

React-Router and styled-components ThemeProvider and withTheme don't use the new API yet and this screws everything up. I guess I have to wait for those to catch up. And it might be a good time to get rid of react-router and replace it by something simpler and sane :)

So when I repace componentWillReceiveProps with componentDidUpdate and calling setState in it the problem goes away for now and everything just works.

[gaearon]

Hmm. I think the issue is not that they screw it up for you, but rather that they might be copying getDerivedStateFromProps() onto their wrapper components because it's static, and that causes the weird behavior. We had a similar issue in React Redux: reduxjs/react-redux#879. The fix is for these packages to update their hoist-non-react-statics dependency (or equivalent code) to the version that's aware getDerivedStateFromProps() is a special method and shouldn't be copied. I encourage you to file an issue with those projects and describe the problem there.

[alecmev]

@TrySound

The idea is to have the second argument always available what is not possible with passing previous props. So you should always initialize state and it will be passed to gDSFP so you could add only necessary check without checking existence.

I understand the reasoning, but why is "without checking existence" important? Consider this synthetic TypeScript snippet (sorry, don't know Flow well enough):

class Foo {
  bar = " bar ";
}

interface Props {
  foo: Foo;
}

interface State {
  foo: Foo;
  trimmedBar: string;
}

class A extends Component<Props, State> {
  static getDerivedStateFromProps(
    nextProps: Props,
    prevState: State | null
  ): State | null {
    if (prevState && nextProps.foo === prevState.foo) return null;
    return {
      foo: nextProps.foo,
      trimmedBar: nextProps.foo.bar.trim(),
    };
  }

  render() {
    return <div>{this.state.trimmedBar}</div>;
  }
}

And this fix:

function propsToState(props: Props): State {
  return {
    foo: props.foo,
    trimmedBar: props.foo.bar.trim(),
  };
}

class A extends Component<Props, State> {
  state = propsToState(this.props);

  static getDerivedStateFromProps(
    nextProps: Props,
    prevState: State
  ): State | null {
    if (nextProps.foo === prevState.foo) return null;
    return propsToState(nextProps);
  }  

  render() {
    return <div>{this.state.trimmedBar}</div>;
  }
}

Extra cognitive load (of course, an exaggeration, for this example; I'm talking about the "two places where the state is touched, instead of one"), at no gain, from my POV.

There's an alternative: letting foo and trimmedBar be undefinable, and initializing state with {}, but that's a whole new can of worms, because now TypeScript will think that they can actually be undefined, throughout the component, at any time, while the programmer knows that the window during which they're undefined is very narrow.

This pattern - "do [almost] the same thing both on mount and on prop updates" - is, AFAIK, wide-spread, and it's more of an exception than a rule to have significant differences.

Now, having written the above, I'm not so sure anymore that significant differences between state initialization and derivation are that uncommon, so I wouldn't suggest going as far as recommending people to cram everything state-related into gDSFP.

However, I still think that relaxing the warning is worth it. I'd replace it with something along the lines of "if a custom gDSFP is supplied and this is the first invocation of it and prevState is null and it returns null, then print a warning". And I would document that prevState can, indeed, be null.

Or am I missing a footgun here?