Fix componentWillReceiveProps when calling setState/forceUpdate

[jamesb3ll]

Fixes #2185

componentWillReceiveProps doesn't get called when calling setState or forceUpdate and receiving props.

componentWillReceiveProps never gets called as this.setState/forceUpdate will set component._force = false/true, so _force will not be null or undefined as expected in diff/index.js:

if (
  newType.getDerivedStateFromProps == null &&
  c._force == null &&
  c.componentWillReceiveProps != null
) {
  c.componentWillReceiveProps(newProps, cctx);
}

Instead of using the _force flag, I've decided to check the equality of oldProps and newProps to check whether props have changed. I've added a test for this case too. It would be great to know if I'm missing an edge case with this fix?

This fix also drops the size by -6B 🎉

[coveralls]

Coverage remained the same at 100.0% when pulling afb335f on jamesb3ll:fix-cWRP into 59c036c on preactjs:master.

[JoviDeCroock]

I think this is safe to do, do note that oldProps and newProps will never be referentially equal due to https://github.com/preactjs/preact/blob/master/src/create-element.js#L12

[marvinhagemeister]

@JoviDeCroock This is not always true. For the component where setState/forceUpdate will be called on, the props will be referentially equal. The whole subtree will have newly created props objects due to the line you linked to tough.

[jamesb3ll]

Thanks @JoviDeCroock 🙂I was also confused with that, but with the debugger I saw that the parent component (setting state or forceUpdate) kept the same equality for newProps and oldProps and that way we can avoid that component calling componentWillReceiveProps

I've been playing around in this codesandbox here: https://codesandbox.io/s/preact-x-cwrp-b1inz

Edit: Exactly, thanks @marvinhagemeister 😃

[marvinhagemeister]

This test is so good 😍

[marvinhagemeister]

This PR is awesome! The more I look at it, the more I'm convinced that the solution here is pretty genius!

The issue arises because both the Inner and Outer components are enqueued as part of the same commit. With each queued component we mark the update type with the _force flag that is stored on the component class. Trouble arises because we expected the queued component to always be the root of the subtree it updates, never as one of the children of an already queued component. Thus our logic broke, because we checked each component for the _force flag during diff, instead of just the one at the root of the update.

This solution here is a stroke of genius, because it leverages the fact that, the props of a root component of any sub-tree will have referentially equal props. Every other vnode below that will have a newly created object for props like Jovi mentioned. Therefore checking for props equality is the smallest and easiest way to check for a root and assert if cWRP should be called 🎉

Love it, thank you so much for the PR 👍 💯

[marvinhagemeister]

FYI: We'll likely publish a new release in a couple of hours 👍