Unsafe Lifecycle warning in React >= 16.3

[morlay]

Version

react-router@create-context-api
react@16.3 under React.StrictMode

Steps to reproduce

use react-router under React.StrictMode

Expected Behavior

without warning

Actual Behavior

warning for componentWillReceiveProps componentWillMount

[morlay]

For Router, we could write like this to fix the componentWillMount warning.
It works, but not sure is this a correct way.

https://github.com/ReactTraining/react-router/blob/react-context-api/packages/react-router/modules/Router.js#L79-L81

render() {
    const { children } = this.props;
    return (
      <RouterContext.Provider value={this.getChildContext()}>
        <HistoryListener history={this.props.history} onLocationChange={() => { this.setState(...) }} />
        {children ? React.Children.only(children) : null}
      </RouterContext.Provider>
    );
 }

class HistoryListener extends React.Component<{ history: History, onLocationChange: () => void }> {
  unlisten: any = null

  componentDidMount() {
    this.unlisten = this.props.history.listen(() => {
      this.props.onLocationChange()
    })
  }

  componentWillUnmount() {
    if (this.unlisten) {
      this.unlisten()
    }
  }

  render() {
    return null
  }
}

[pke]

This should be fixed in v3 too.

[timdorr]

@morlay No need to farm that out to another component. Instead, we can move to cDM and simply do a second check for if the location changed during that time.

Another option, if we wanted to go all-in on 16+, would be to use a setState callback function for the initial read from history.

[timdorr]

Easy next step: Upgrade to React 16.3 locally and add a failing test, like I did here: reduxjs/react-redux#918

[morlay]

@timdorr
In StrictMode,constructor static getDerivedStateFromProps and render will be double-invoked.

constructor -> static getDerivedStateFromProps -> render -> componentDidMount
There are no opportunity to do the second check. So i think to move history listener to another component.

Then we can have
Router#constructor -> Router.getDerivedStateFromProps -> Router#render -> HistoryListener#constructor -> HistoryListener#componentDidMount -> Others Components >Router#componentDidMount

logic in HistoryListener#componentDidMount will be same as old Router#componentWillMount.

I tried providing another callback method in context to omit location changes or proxy history push replace methods.
It only handle changes from children of Router and will miss others changes from history itself, like navigating by redux actions.

[timdorr]

That's fine. We can add checks to avoid double execution.

[grzegorzjudas]

Is there a PR to fix this?

[gustavo-depaula]

I opened a PR, but I need someone to help me test this.