Macil / react-hot-transform

Tweak React components in real time with Browserify.
MIT License
34 stars 0 forks source link

Consider migrating to React Transform #3

Closed gaearon closed 6 years ago

gaearon commented 9 years ago

I tried my best to split React Hot Loader into several projects that can be used for other build systems or frameworks. React Hot Loader is in the process of getting deprecated. Please check out:

I imagine you should be able to create react-transform-browserify-hmr while staying in the React Transform ecosystem. This will allow us to collaborate instead of having two separate ecosystems. What do you think?

Macil commented 9 years ago

react-transform-webpack-hmr is a babel transform and not a webpack loader, right? It looks to me like the only Webpack-specific thing is the use of the HMR API, which Browserify-HMR compatibly implements. Using browserify-hmr + babelify + babel-plugin-react-transform + react-transform-webpack-hmr should work fine if I'm understanding it right, but I haven't tested it yet. This react-hot-transform project will become unnecessary once that's stable then. I'll gladly put a note in this project's readme recommending people to go that route instead when you discontinue react-hot-loader.

gaearon commented 9 years ago

react-transform-webpack-hmr is a babel transform and not a webpack loader, right?

It's actually neither. Any react-transform-* thing is just a function of options => ReactClass => ReactClass signature. In this case, it's a function that works together with a Babel transform. Another example of a such transform (but unrelated to hot reloading) is react-transform-catch-errors.

It looks to me like the only Webpack-specific thing is the use of the HMR API, which Browserify-HMR compatibly implements.

I think so.

Using browserify-hmr + babelify + babel-plugin-react-transform + react-transform-webpack-hmr should work fine if I'm understanding it right, but I haven't tested it yet.

This would be ridiculously awesome. If you find time to experiment please let me know the results. I really want everyone in React hot reloading land to consolidate their efforts and use the same tech, so that would go a long way towards making that happen!

Macil commented 9 years ago

I tried it out and the babel react transform stuff works! I've made the "reactTransform" branch of browserify-hmr have its example use it instead of react-hot-transform. The only issue was that redbox-react doesn't work with Browserify (https://github.com/KeywordBrain/redbox-react/issues/17), so I left it and react-transform-catch-errors out of the example for now.

Are you very attached to the name react-transform-webpack-hmr? If it were renamed to react-transform-hmr, then that would make its compatibility with Browserify-HMR less surprising to people in the future. No one in the livecoding space has used the term HMR to refer to anything with an incompatible API as far as I've seen. livereactload, browserify-patch-server, and amokify all avoid the term, so there'd be no added confusion about whether react-transform-hmr would be compatible with them.

gaearon commented 9 years ago

I agree about renaming to react-transform-hmr, will do that.

gaearon commented 9 years ago

cc @milankinen — have you been also implementing HMR for Browserify in 2.x.x branch? Do you want to join efforts?

milankinen commented 9 years ago

Yeah sure! @gaearon what was in your mind?

As you can see, I've implemented my own babel transformer (https://github.com/milankinen/livereactload/blob/2.x.x/src/babel-transform/main.js).

And I could also start using react-transform-webpack-hrm if the module.hot dependency could be changed to optional: https://github.com/gaearon/react-transform-webpack-hmr/blob/master/src/development.js#L24-L35

gaearon commented 9 years ago

@milankinen I was just curious if your efforts intersect with @AgentME's in https://github.com/AgentME/browserify-hmr and whether you can join your forces in this.

milankinen commented 9 years ago

Yes, they intersect indeed. However, the implementations differ so much that I don't know how to join the efforts reasonably. :-/

Macil commented 9 years ago

module.hot is the hot module replacement API, so making the react-transform-hmr transform still do things without it makes the name seem a little misleading. People could expect that other things that depend on the hot module replacement API (like my ud library) would work with livereactload too.

But other than the name, it doesn't look like there's any technical issues. As @milankinen points out, Livereactload's transform is pretty much equivalent to react-transform-hmr if it were made to not call module.hot at all if it's not present. If it's likely that livereactload will will not change to require its own transform to do anything extra, and react-transform-hmr isn't strictly depending on anything only found in the HMR API, then it seems like a good idea to merge them. (In that case, up for another rename? react-transform-live supporting Webpack, Browserify-HMR, and LiveReactload doesn't seem half bad. We could check if browserify-patch-server and Amokify would be trivial to support in the same transform too.)

gaearon commented 9 years ago

For now I decided to stick with HMR so it behaves consistently across the supported platforms. I renamed the package to react-transform-hmr.

gaearon commented 9 years ago

The only issue was that redbox-react doesn't work with Browserify (KeywordBrain/redbox-react#17), so I left it and react-transform-catch-errors out of the example for now.

This should be fixed in redbox-react@1.0.6. Would you mind deprecating react-hot-transform in favor of babel-plugin-react-transform and friends?

If you're not satisfied with forcing users to use Babel, we can implement react-transform-loader and react-transformify which would act like babel-plugin-react-transform (of course, with some limitations—they would only “see” exported components).

Macil commented 6 years ago

Closing out old issues. The readme has been updated with a deprecation notice pointing to react-hot-loader, which appears to be long-since recommended and no longer be tied to webpack as it's a Babel transform nowadays.