Support decorators

[zdavis]

(This may or may not be something you want to address with v3; if not, please just close this)

Our project uses decorators to connect components to the redux store. I think this is fairly common. For example, you see it a lot in erikras/react-redux-universal-hot-example, which uses Babel 6 and the "transform-decorators-legacy" Babel plugin, since Babel has decided to postpone decorator support until its more mature. For example:

https://github.com/erikras/react-redux-universal-hot-example/blob/master/src/containers/Chat/Chat.js#L4

This worked fine with v2, but with v3 components that are decorated like this seem to lose their internal state when they're hot-reloaded.

If you want to look at this, let me know and I'll put together a sample repo that shows the problem. Thanks so much for your work on this! It's great to see all the activity on this project right now.

best,
Zach

[gaearon]

If you want to look at this, let me know and I'll put together a sample repo that shows the problem

Please do! Also make sure you’re using react-hot-loader/babel rather than react-hot-loader/webpack if you want “connected” classes to be found by it.

[zdavis]

Ok, the "decorators" branch in zdavis/react-hot-boilerplate shows the problem pretty clearly. See zdavis/react-hot-boilerplate@7b510aa.

This commit adds a new component called "DecoratedCounter". The decorated counter is decorated with @connect, which just wraps a component around the decorated component.

The Counter component is also wrapped with connect, but without using decorator syntax.

When DecoratedCounter is hot reloaded, it loses its internal state. This is not the case for the Counter component.

Let me know if there's anything else I can do to help!

[nfcampos]

I think this comes down to our babel plugin needing to be aware of decorators, otherwise it only wraps the decorated hoc, not the original component that was decorated

[gaearon]

I wonder if a simple way to fix this would be to insert our own “registering” decorator as the innermost one if we see a decorated class.

[nfcampos]

Yeah that sounds like it'd work

[liady]

@gaearon @nfcampos Any news on that? Currently all of our classes are decorated, which prevents them from being properly hot-reloaded (the Root component just refreshes and loses all internal state).

Is there a temporary fix we can do in our code to get it working?

[reem]

@liady we had the same problem over at @terminalcloud but I figured out a way to get things to work in the meantime.

By my understanding of the code, react-hot-loader/babel goes through your modules, finding any variable or class declarations, checking if they are components, and registering them for reloading later. In order for stateful reloading to work, react-hot-loader must know about the full path of components from the root to your stateful component. If even one link in this chain is missing that subtree will be completely remounted.

In our case we use two decorators, connect from react-redux and view from redux-elm. To get this to work with react-hot-loader, ensure that each component class created by any decorator is exposed as an intermediate identifier, for instance:

// WILL NOT WORK
export default connect()(view(() => <div/>))

// WILL NOT WORK
const View = view(() => <div>)
export default connect()(View)

// WORKS
const View = () => <div/>
const ElmView = view(View)
export default connect()(ElmView)

in the first example, react-hot-loader doesn't know about the actual component or the intermediate one created by view. In the second example it knows about both intermediate components, but is still missing the base component. In the final example react-hot-loader can detect all three components, and hot reloading should work as normal.

This workaround is pretty cumbersome imo, but it does work for now...

[calesce]

@reem to be clear, those don't look like decorated components, but are higher-order components, so this is more likely related to #378, although a similar issue.

Unfortunately it's a really tricky one, since right now the best that the Babel plugin can do is to look for top-level references. One idea would be to "expand" the code, to recursively turn your first example into the last example (split out currying?), but that sounds like it'd be riddled with edge cases.

[reem]

@calesce I'm not exactly sure what you mean, what's an example of a decorator that would cause a problem and is not a higher order component?

This sort of thing is where a true multi-phase compiler with a middle representation between AST and output representation is really helpful - the AST can be transformed into a radically simplified middle layer where all expressions are already split into their smallest possible pieces and you have a true CFG to work with (where all conditionals, loops, etc. have all been turned into jumps). Then implementing a "decorator" for all values of some type is much much easier, because you can detect them directly instead of relying on parsing, which is going to be inherently fragile.

[calesce]

I'm not exactly sure what you mean, what's an example of a decorator that would cause a problem and is not a higher order component?

Decorators are a stage-2 JavaScript proposal, denoted with @. The most common ones from libraries are probably @connect and @autobind, which are class-level decorators.

What's different is that you can have something like:

@connect(mapStateToProps)
export default class App extends React.Component {...}

that doesn't get reloaded, but this does:

export default connect(mapStateToProps)
(class App extends React.Component {...})

I'm not familiar with the exact decorator semantics to tell you why, I haven't dug enough into this specific issue.

This sort of thing is where a true multi-phase compiler with a middle representation between AST and output representation is really helpful

Ha, yeah that sure would be nice, I'm guessing you have Rust in mind. 😉

[reem]

Ha, yeah that sure would be nice, I'm guessing you have Rust in mind. 😉

You got me :P

Your examples are a little odd to me because in my testing it appears that:

export default connect(mapStateToProps)
(class App extends React.Component {...})

actually does not get reloaded, only:

class App extends React.Component { ... }
export default connect()(App)

does. This would explain why decorators don't work, because the @ form is treated like the former, not the latter.

[calesce]

Yeah you're right, my mistake. I typically write it out like in your latter example.

It looks like with decorators, you can't have a reference to the original class, which is why @gaearon suggested having the plugin insert a decorator that registers it with the RHL runtime.

Another approach might be running the transform-decorators plugin first, but if you have multiple decorators then you'd still have the issue of multiple intermediate components.

[reem]

I think it's all actually the same problem: if you don't have a top level variable or class with your component, rhl can't discover it. Decorators, HOC, etc. are all just instances of this problem because they encourage you to not have all your components in a place where rhl can find them.

It really seems like the only way to conclusively solve this is to do something like override React.Component (though this doesn't catch stateless components!!) and/or unpack compound expressions into individual pieces that can be inspected.

[theKashey]

This is a most important thing to be solved in future. And the hardest puzzle to solve.

Currently, there are two ways:

1. Integrate deeply into the webpack, and ignore changes in components outside the changed file.
2. Try to create a way to handle unexported components. Use stack traces as fingerprints.

First one is a bit tricky - currently, on module update, you will require new sources, and render the new application from the scatch. RHL in this case preventing React to unmount the old application.

It is also possible just to require the new sources and re-render the old application. And RHL will replace all the React components by the new ones. Magic.
And what if React Hot Loader will replace not all the new ones, but only the CHANGED one - this will throw away all the issues, as long all decorators and compositions before the change point will be the same as before.
There is only one small problem - if you will edit React component, and we will replace it - that is ok. But if you will edit some common code, and the change will affect the wider amount of React components... How to detect the borders of the change?
Still easy, but one has to understand which files contain React components, and which is not (Easy), and track how one exports its imports (possible) and extending the change borders by doing it.

PS: You can do it manually by placing module.hot.accept.

[lgra]

I've just shown the warning about the use of Decorators in RHL readme.
I'm pretty confused. I'm used to decorate my React classes and for instance, I don't face this issue. States of my decorated components seems to be preserved when hot reloaded.

Am I wrong ? Or is my way to use decorator that prevents this issue ?
I can't say.

Here is a repo https://github.com/lgra/lg-webpack with a kind of React app template with WebPack and RHL. It can be run with vscode debug tool, or npm start. When modifying render method of decorated component, state seems to be preserved, even if the method itself is enhanced by the decorator (in the example, the render method of web_modules/app/group, enhanced by @renderDuration).

[theKashey]

Look like it working just because you dont "decorate" classes (like HOC), but you modify their prototypes. Thus it is the same class and you had it as a top level variable.

[lgra]

Understand! Thanks a lot.
I'm using class decorator without returning a new constructor method. In fact, I don't return anything, and let the mechanism default to the initial constructor - the one and only used by RHL.
I'm using class decorators to enhance the class prototype, as a replacement to mixing mechanism. I will continue to use it this way ;-)

[theKashey]

Ok, the "decorators" branch in zdavis/react-hot-boilerplate shows the problem pretty clearly.
@zdavis - thanks for this case
V4 is also unable to solve this puzzle. The problem is this this code

<div>
      <h1>Hello, world!!!!</h1>
      {children}
</div>

In does create {children} prop for a div with a length 2, and second element is also an array.
In the same time after react-render, ie while one traverse react-tree it will be children prop with a length 3. Look like something unflatten the array.

[gregberge]

Sounds fixed in v4.0.0-beta.5.

[mqklin]

@neoziro I have this warning now after each update (I use connect decorator from react-redux):

Not sure what is it though

[gregberge]

@mqklin you can ignore it, we will switch these warnings to debug mode.

[dehypnosis]

logger.js:30 React-stand-in:, Non-controlled class Form(branch(Apollo(inject-
branch(withState(withHandlers(withState(withHandlers(BankAccountForm)))))))) 
contains a new native or bound function  validateFields ƒ validateFields(ns, opt, cb) {
...

Yes I can ignore warnings, but still cannot hot reload composed codes.
I can develop with hot reload component's jsx parts but nothing else.
I recomposed all the handlers, states, props,.. So i am still sad with manual reloading.
But surely better than nothing.

Is there any workaround or progress about this issue?

[theKashey]

Just yesterday we desided not to do anything. #821.
The key reason - you are not going to hotreload something from node_modules. Ie change the function code.
Warning is just a warning. Everything should work.
If not - could you provide some example, to let us understand the problem?

[zdavis]

Really awesome to see this fixed! We're excited to try out the new version of RHL. Thank you all for your work on it!

[mqklin]

I would like to support the project on opencollective, but can't find it there
@neoziro are you planning to register there?

[gregberge]

@mqklin yes, I just submitted it, I keep you in touch!

[gregberge]

@mqklin done https://opencollective.com/react-hot-loader

[mqklin]

Cool! Glad I can support the project as much as I can. Hope I will be able to give back more! Thank you for you work!