Connected components cannot use new 16.3 lifecycles

[byronanderson]

I know that there is still much consideration going into #890 so I don't mean to rush, but I thought you might be interested to know:

It looks like React 16.3 exposes the new lifecycles in a way that disallow for using the old componentWill* at the same time as new lifecycles.

Something that looks roughly like this:

class Form extends PureComponent {
  constructor(props) {
    super(props);
    this.state = {initialized: false};
  }
  static getDerivedStateFromProps(nextProps, prevState) {
    if (nextProps.data && !prevState.initialized) {
      return {initialized: true};
    }
    return null;
  }
  render() {
    return this.state.initialized ? "initialized" : "uninitialized"
  }
}
export default connect()(Form);

will get this warning:

which I didn't understand until I saw that the implementation details of connect use inheritance to add those lifecycles.

My workaround in this first case I found in my code is pretty easy to fix (I was just using the connect to get a dispatch method) but I bet people are going to start running into this as they upgrade to 16.3.

Sending my best to your efforts in #890 !

[markerikson]

I'm not sure I understand that example. The Form component contains neither cWRP nor cWU.

Can you put together a minimal repro of this issue in a CodeSandbox or something?

[byronanderson]

Sure thing...

[markerikson]

Hmm. Perhaps it's due to our use of hoist-non-react-statics ? Except that v2.5.0 added support for gDSFP, and @timdorr switched us over to use that a month ago. It looks like react-redux@5.0.7 ought to resolve this.

[byronanderson]

I was having a heck of a time reproducing with most recent versions of dependencies on codesandbox, so that would make sense...

I'll upgrade and check...

[byronanderson]

Yep, you are right, most recent patch version fixes this.

Thanks for the quick feedback, and sorry for wasting your time!

[swac]

Is this actually addressed? It seems like connectAdvanced is still injecting a componentWillUpdate function when in development mode: https://github.com/reactjs/react-redux/blob/c1b9d36e078836374fe833a6eb2a8f0de0ad1e30/src/components/connectAdvanced.js#L268-L291

If I understand React's warning correctly, this means the componentWillUpdate function in connectAdvanced won't be called for a component that's been migrated to use a new lifecycle method.

[markerikson]

The problem here is that getDerivedStateFromProps was being unnecessarily hoisted into the generated wrapper component. v5.0.7 fixes this, because it uses a newer version of hoist-react-statics that skips gDSFP.

We are still using a componentWillUpdate method in 5.0.7, but React isn't warning about that yet, and we're working it in #919 anyway.

[vctormb]

@markerikson is it correct to dipatch() something inside gDSFP or inside cDU?

[gaearon]

No, gDSFP must be pure and contain no dispatch calls.

Dispatching in cDU is okay-ish. Although it's still not that great from performance perspective to dispatch in response to re-rendering. But you were already doing it.

[vctormb]

@gaearon Actually, I already have it inside cWRP (if props change, I compare it and dispatch something to reducer).. but now that I've updated the react version I'm trying to migrate it. Humm.. is there a correct way to dispatch something when props change?

[timdorr]

@vctormb cDU is the recommended way: https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#side-effects-on-props-change

But I'd probably ask what your cWRP is responding to. Could that be responsible for dispatching the action? Could you push the state transformation back to the store itself? There is probably a larger refactor that will actually put you in a better position not just for React async stuff, but as your app changes over time.