hmr + redux @connect() class decorator issue

[rickbrenn]

Description

When using babel's @babel/plugin-proposal-decorators with redux's connect HOC and you make changes to the file the app updates but displays the old code.

Here's the normal way which works as expected when you change the file:

class Test extends Component {
  render() {
    return <div>Change me</div>
  }
}

export default connect()(Test)

If you use the class decorator it no longer renders new code:

@connect()
export default class Test extends Component {
  render() {
    return <div>Change me</div>
  }
}

I've tested this with other HOC like react-router's withRouter and the class decorator works fine. I'm unsure if this issue has to do with babel, hmr, or maybe I'm missing something obvious. I also transpiled the code with babel cli manually and tested the code it produces but that works fine, which is weird.

Expected behavior

The file should hot reload successfully whether you're using the standard method or class decorator.

Actual behavior

When using redux connect as a class decorator old code gets rendered as you change the file.

Environment

React Hot Loader version: 4.3.12
Node: v8.12.0
npm: 6.4.1
@babel/core: 7.2.0
@babel/plugin-proposal-decorators: 7.2.0
Operating system: macOS Mojave 10.14.1
Browser and version: Chrome 70.0.3538.110

Reproducible Demo

Here's an example repo

[theKashey]

Sorry, was quite busy with a new release. Could you check your problem with version 4.6.0?

[rickbrenn]

No problem. I updated the test repo to 4.6.0 but unfortunately the issue persists.

[Wedvich]

I'm seeing the same issue with using connect as a decorator on 4.3.12, and the issue is still there after upgrading to 4.6.0. So I tried adding hot to my connected component too, but it still fails:

@connect()
class Thing extends Component { /**/ }
export default hot(module)(Thing);

So this seems to suggest it's an ordering problem of some sorts: calling hot with the connect'ed component as input fails. Using a helper function that flips the order works, even as a decorator:

const hotConnect = (...connectArgs) =>
  (component) =>
    connect(...connectArgs)(hot(module)(component));

@hotConnect()
export default class Thing extends Component { /**/ }

I'm a bit confused myself as to what's the best practice - if hot should be used on every connected component, or if it's sufficient to only apply it to my root component. When using connect as a decorator only the former works - however, when not using decorators, it works fine with the latter.

[theKashey]

You may have one hot for your top-level component, just don't use it in the same file you are creating redux store - put it one module below.

[Wedvich]

Allright, good to know!

Just for the record, using the new root/hot from 4.6.0 on my root component had no effect, and I still need my hotConnect workaround.

[theKashey]

I'll take a look on your case later today. Please don't fix it.

[theKashey]

This is React-Redux v6 issue.From our side it's tracked as #1001
The problem - React-Redux creates methods via factories to render your Component. That WrappedComponent is stored in closure and RHL has no power on it.

this.selectChildElement = makeChildElementSelector()

During the class update we consider updating of the factored method as unsafe, and NOT doing it.

Result - you are rendering the old WrappedComponent.

Solutions:

• find a way to regenerate methods like this.selectChildElement. Still no idea how
• ask react-redux to make that method a class method, why it should be an extrnal function?
• ask react-redux to render Component.WrapperComponent, which we cound update, not FinalWrappedComponent

Whys:

• v5 works as long as it was a magic compoenntWillUpdate methods, specially designed to HMR.
• "separation" works as long you have Class as a registered variable, and we could inject a new version instead of the old one. That's always was the easiest way to make RHL work, but variable extracting is not quite cool stuff.

@markerikson - could you take a look at the problem? I would really appreciate if Redux and RHL would be besties 🤝

[markerikson]

I've got other priorities at the moment, but if someone wants to investigate this further and file a PR, we can look at it.

[theKashey]

This would fix the problem for now - reduxjs/react-redux#1137