-
Notifications
You must be signed in to change notification settings - Fork 879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add playground for React Hot Loader 2 #33
Conversation
If you want to try:
and edit the stuff I marked in the comments. |
I'd be happy if @jlongster, @caspervonb, @Kureev, @milankinen took five minutes to play with this. ;-) |
@gaearon Tried the project you mentioned above and removing |
Interesting observation: what if it's not Meaning, our hypothetical Babel plugin would be unaware of hot reloading. All it would do is locate (to the best of its knowledge, with adjustable heuristics) React classes (in the best case, classes returned by HOCs too), and wrap them in an “identity” decorator with no runtime behavior. // Our Babel plugin would just wrap every React class it finds with this:
// babel-plugin-react-class-finder/decorator.js
export function function identity(cls) {
return cls;
} Then, in our Webpack config (or its Browserify analog), we can do this: plugins: [
NormalModuleReplacementPlugin(
'babel-plugin-react-class-finder/decorator.js',
'./tweakReactClass.js'
)
] where // hypothetical packages exporting decorators
import catchReactErrors from 'react-error-catcher-decorator';
import enableHotReloading from 'react-proxy-decorator';
export default function tweak(ReactClass) {
return catchRenderErrors(enableHotReloading(ReactClass));
} In production, you'd just keep it a no-op. |
@kompot I think this is expected—it's just how React's diff algorithm works. If the children given to |
@gaearon Yep, that's definitely not a problem. Was a bit surprised that I did not see such behaviour having used 1.x RHL extensively for a long time. But probably it's there as you've mentioned. Thanks for doing great job on DX tools, cheers) |
Hey, I tried a bit. It's working properly, just one question : not sure it's a bug, but if you remove the |
@chtefi Not sure what you are referring to? If I remove |
I think we can avoid specifying name of the module like |
I don't mean for users to specify it, this is just a playground. In the future, I plan to write a Babel plugin that would look for React classes and annotate them with unique IDs as described here. Here's a proof of concept (outdated) of such plugin. |
What are you calling a rebuild? Webpack always rebuilds the whole bundle on hot update (it needs to be ready to serve something in case user refreshes the page). However, if you look at response hot update JS, it only includes the changed module: |
@gaearon yes indeed. Maybe I'm missing something but when i remove |
@chtefi Yeah, handling that is too complicated I think, and I'm not sure it's something worth pursuing. Filed as gaearon/react-proxy#16. |
Wow, I though that HMR allow you to update only specific module, weird. In my react-live-patch implementation I update only changed module (other modules don't even know that update happens). I don't like that much a complexity you want to introduce by using hotify function: I understand that it'll solve issue with hot-reloading classes wrapped by high-order-components, but we can find different workaround like exposing vanilla class: class Foo extends Component { ... }
export { decorate(Foo) as default, Foo as ReactClass } or something like this. It automatically remove babel plugin for component finding. Does it make any sense? |
That's exactly what happens here, too. Maybe I'm missing your point. Only the changed module is updated, that's all. I put everything in one file to keep the example simple, but you can split it into modules, and they would get hot-reloaded independently.
What complexity are you referring to? This is exactly what React Hot Loader has always been doing: wrapping your classes. This time, I just want change the wrapping to occur right after class definition instead of before export. I see this as more consistent, and less surprising: both |
I'm talking about additional babel plugin to extract React class from module code and about the need of wrapping every class by custom function to make it hot-reloadable. In both cases (using decorator or additional export) you can achieve the same goal, but if you consider to use export you may avoid writing extra plugin for babel and boilerplate code (I'm talking about specifying |
First, sorry about the delay. Been busy at the work lately... Here are my observations and thoughts: First of all, I really like this idea having one project for React class proxying, one for proxy applying (class decoration) and one for HMR (per build tool). The benefits are obvious: when proxying/decoration receive improvements and/or bug fixes, they become available for all users (webpack, browserify, etc. etc....). This would also reduce the overlapping work we are doing at the moment. 👍 Layer 1 - proxy
Layer 2 - decorationThe "decoration layer" and its heuristics will be absolutely the most challenging task (as @gaearon said) - especially the HOC case...
In my opinion, Babel plugin is the right choice for this decoration layer - it is de facto at the moment and the majority of the React projects use it. And because the responsibility of this "layer" is just to detect the right place and identity for Layer 3 - hot module replacementThe responsibility of the "HMR layer" is to detect which parts of the code must be replaced and just reload them. And @Kureev: just reloading the changed module is not enough. JavaScript has closures so if you reload just the modified file, closures from the other files have already bound the code from previous module, thus "nothing happens" (you can demonstrate this by trying to change the The Webpack hot-reloading tries to solve this with And here comes the interesting part that should be taken into account before starting the implementation of the bottom layers: How to detect if the module can "accept" hot-reloading (stop propagation) if the proxying and decoration is done in the other layers? Too aggressive accepting will cause "nothing to happen" during the reload and too passive accepting may cause unwanted side effect. In my opinion, the safest and the easiest way is to reload all local modules (starting with Of course the "accept" level detection is in the consideration of HMR implementer and more aggressive accepting may require some tl;dr;
|
Hi @milankinen ! Thanks for your feedback, it was really interesting to read. I agree with you in the most of points you have about live reloading. As for my opinion, it's better to avoid side-effects by not reloading non-react modules (all in all it's react hot reloading) than to reload everything and break working flow - that's a deviation point in our implementations. But as for |
@milankinen Nicely put, this is exactly how I think about it. 👍 |
Just played with it, looks cool and I love the idea to just mark up react components at build time. I don't have time right now to read this whole thread (sorry... I know that's lame), but are you trying to figure out how to track state across React components? Because there's this dirty little secret that Sebastian will probably kill me for mentioning: each instance has an ID that should be relatively stable across instances: |
I simplified the example by removing HOCs. We'll make sure they work before the release, but it's not essential right now. I wrote babel-plugin-wrap-react-components to locate React components in code and apply arbitrary wrapping to their classes. Now, the playground includes two such transforms. I intend There are two new features in the updated demo:
|
You Will Be Shocked To Learn What Came Out Of This https://github.com/gaearon/react-transform-boilerplate |
It's not clear yet what React Hot Loader 2 is going to be, but The Death of React Hot Loader could give you some idea. In a nutshell, we're going to make it very slim and rethink crucial Webpack-independent parts.
This PR shows a playground of what React Hot Loader 2 should support. We don't actually even use RHL here: it's just React Proxy (new engine), a custom
hot()
decorator and a source code withhot()
and@hot
calls written by hand. You'll see that this enables many things that didn't work before:The tricky part is generating these
hot()
calls. We'll probably evolve proof-of-concept babel-plugin-react-hotify to support these scenarios, along with other scenarios like babel-plugin-react-error-catcher.This may not be the best news for compile-to-JS languages like CoffeeScript, but I feel this is a step forward nevertheless because we're able to ditch the ugly exports rewriting, and enable a bunch of scenarios that didn't work before.
Feel free to play with it! More to come.