Props shouldn’t be overwritten

[timneutkens]

I'm not sure what the correct functionality is, either providing the new props vs the old ones / merging them etc. But the change made in 4.1.0 is breaking and should be part of a major version.

It also seems weird to me that componentWillUpdate is called from react-hot-loader 🤔

Fixes vercel/next.js#4299

testProps is the old way they were calculated in 4.0.0/4.1.0

nextProps are the new way they were calculated in 4.1.0+

My change brings back the result of testProps rather than nextProps.

[timneutkens]

Another solution to the problem we're having with styles-jsx is to remove the extra componentWillUpdate

[codecov-io]

Codecov Report

Merging #967 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #967   +/-   ##
=======================================
  Coverage   88.99%   88.99%           
=======================================
  Files          30       30           
  Lines         727      727           
  Branches      166      166           
=======================================
  Hits          647      647           
  Misses         66       66           
  Partials       14       14

Impacted Files	Coverage Δ
src/reconciler/hotReplacementRender.js	89.8% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b22f37e...a6bbd40. Read the comment docs.

[theKashey]

It also seems weird to me that componentWillUpdate is called from react-hot-loader 🤔
This is how React-Redux got signal to update wrapped component.

As far as I know - the logic around merging props should correct.

const Switch  = ({flag}) => flag ? <button1 /> : <button2 />;

<Switch flag={0}
// and then
<Switch flag={1}

React will re-render something with flag={1} and that means that we have to work with flag={1}, with a new prop, not the old one.

Look like, you dont need this change. You might need remove side effect on props. So - store the original props, and restore them just after render.

in the same time, I am merging old tree and a new tree. Probably I should use the values used to build existing tree, ie "the old values".

I have to double check.

Meanwhile - could you double check that

const next = instance => {
      // copy over props as long new component may be hidden inside them
      // child does not have all props, as long some of them can be calculated on componentMount.
      const realProps =  instance.props; // <<-------
      const nextProps = {
        ...instance.props,
        ...(child.nextProps || {}),
        ...(child.props || {}),
      }

      if (isReactClass(instance) && instance.componentWillUpdate) {
        // Force-refresh component (bypass redux renderedComponent)
        instance.componentWillUpdate(nextProps, instance.state)
      }
      instance.props = nextProps
      hotReplacementRender(instance, stackChild)
      instance.props = realProps; // <<-----
    }

Fixes your problem. Might be this change should be added.

[timneutkens]

Meanwhile - could you double check that

Since:

if (isReactClass(instance) && instance.componentWillUpdate) {
        // Force-refresh component (bypass redux renderedComponent)
        instance.componentWillUpdate(nextProps, instance.state)
      }

is still called the result is the same

[timneutkens]

The code is located here: https://github.com/zeit/styled-jsx/blob/master/src/style.js#L26-L28

[theKashey]

Why changing the props merge order (and why you are getting new values?) fixes the problem?

[theKashey]

Ok. So what I found:

1. The merge order was right. There is no difference between child.props and nextProps, they are both is props of Component, just from different sources (lets call it so)
2. That might be not correct in real, but this is how v4 works and this is how we build out tests.
3. Your problem is not due to props, but due to how componentWillUpdate called. As long "we are not touching lifecycle events", probably we should call this method with unchanged props.
4. There were a side effect with soiling component props, I noticed with React 15. The patch will fix it.

See #968

[timneutkens]

Awesome @theKashey 🏆 ❤️

probably we should call this method with unchanged props

Yeah so this is what was done before (in rhl 3 / 4.0.0/4.0.1), I'll check your branch 👍

[timneutkens]

#968 fixes this issue.