Expanded DEV-only warnings for gDSFP and legacy lifecycles

[bvaughn]

After a lot of discussion and careful consideration, the team has reaffirmed the initial decision to not call unsafe legacy lifecycles componentWillMount, componentWillReceiveProps, and componentWillUpdate for any component that defines the new static getDerivedStateFromProps lifecycle. (This applies to the "UNSAFE_" aliases as well.)

The reason for this decision ultimately boils down to support for the react-lifecycles-compat polyfill and concerns about consistent, predictable behavior for future version of React.

As @jquense pointed out with issue #12411, conditional lifecycle behavior is a new thing, and it is not intuitive. For this reason, we needed to improve the granularity and wording of the DEV warning. This PR does that, first by expanding the warning conditionals to explicitly check for componentWillMount and componentWillUpdate as well (rather than just componentWillReceiveProps) and secondly by explicitly stating that "Unsafe legacy lifecycles will not be called for components using the new getDerivedStateFromProps() API".

The entire new DEV warning is:

Unsafe legacy lifecycles will not be called for components using the new 
getDerivedStateFromProps() API.

<YourComponentName> uses getDerivedStateFromProps() but also contains 
the following legacy lifecycles:
  componentWillMount
  componentWillReceiveProps
  componentWillUpdate

The above lifecycles should be removed. Learn more about this warning here:
https://fb.me/react-async-component-lifecycle-hooks

Tests have been updated as well to verify the new behavior.

Resolves #12411

[aweary]

@bvaughn will this also apply to other new lifecycle methods, like getSnapshotBeforeUpdate?

[bvaughn]

I have no current plans for that to be the case, no.

[bvaughn]

I've updated the PR description to more clearly reflect the considerations that went into this decision. Please let me know if there are any concerns about vague wording or if I omitted anything.

[aweary]

@bvaughn thanks for clarifying! Can you expand on why this same behavior (conditional lifecycles) won't extend to other new lifecycle methods? What is special about gDSFP?

[aweary]

Using "Unknown" as a proper noun feels kind of confusing, but I can't think of something better.

[bvaughn]

Agreed. I wonder if there's a way I could improve this by reformatting the message slightly.

[aweary]

Maybe "An unknown component uses..."?

[bvaughn]

That makes it seem like React doesn't know which component, when really- it just doesn't know what the name fo the component is.

Maybe "An unnamed component" ?

[bvaughn]

Actually I think just plain "Component" might be okay, and more inline with other places.

[aweary]

In what cases does React not know the name of a component? "An unnamed component" sounds better than "Unknown" to me 👍

[bvaughn]

create-react-class case

[bvaughn]

@bvaughn thanks for clarifying! Can you expand on why this same behavior (conditional lifecycles) won't extend to other new lifecycle methods? What is special about gDSFP?

The biggest reason is that the react-lifecycles-compat polyfill relies on unsafe legacy lifecycles componentWillMount and componentWillReceiveProps to work. We know there will be shared components that intentionally define both the old and new lifecycles- (via the polyfill)- and double executing that code could be slow and would likely be confusing as well.

A second argument is that, conceptually, it doesn't make sense to combine the new and old portions of the component API (outside of the case of the polyfill). The new API exists as a safe alternative to the old, not as a compliment.

[aweary]

@bvaughn to clarify, I'm asking for clarification on your response to my question:

will this also apply to other new lifecycle methods, like getSnapshotBeforeUpdate?

I understand the argument for prevent users from mixing new and old APIs, but I'm curious why this won't apply to other new lifecycle methods, like getSnapshotBeforeUpdate? It seems like the same argument would apply to any new lifecycles.

[bvaughn]

Let's move this conversation over to the getSnapshotBeforeUpdate RFC/PR?

As I think more about how I would implement getSnapshotBeforeUpdate support in the polyfill, I think it might make sense to carry this behavior over to that new lifecycle as well. But that's not the focus of this PR. 😄

[bvaughn]

FYI @aweary, I added a polyfill section to the other RFC.

[tofrankie]

I have the same problem.

Can getDerivedStateFromProps not be used at the same time with unsafe lifecycle functions such as UNSAFE_componentWillMount?

Warning: Unsafe legacy lifecycles will not be called for components using new component APIs.

LifeCycle uses getDerivedStateFromProps() but also contains the following legacy lifecycles:
  UNSAFE_componentWillMount

The above lifecycles should be removed. Learn more about this warning here:
https://fb.me/react-unsafe-component-lifecycles

When I remove UNSAFE_componentWillMount there will be no warning.

Examples are as follows:

import React, { Component } from 'react'

export default class LifeCycle extends Component {
  constructor(props) {
    super(props)
    this.state = {}
  }

  static getDerivedStateFromProps() {
    return null
  }

  UNSAFE_componentWillMount() {}

  componentDidMount() {}

  render() {
    return (
      <div>React Life Cycle</div>
    )
  }
}

[gaearon]

Can getDerivedStateFromProps not be used at the same time with unsafe lifecycle functions such as UNSAFE_componentWillMount?

No. That's what the warning says.