Add playground for React Hot Loader 2

gaearon commented 9 years ago

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 with hot() and @hot calls written by hand. You'll see that this enables many things that didn't work before:

Hot reloading HOC code without losing state
Hot reloading components wrapped in HOCs without losing state
Defining several components in a single file
Using autobind decorator

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.

gaearon commented 9 years ago

If you want to try:

git clone https://github.com/gaearon/react-hot-boilerplate.git
cd react-hot-boilerplate
git checkout rhl-2
npm install
npm start
open http://localhost:3000

and edit the stuff I marked in the comments.

gaearon commented 9 years ago

I'd be happy if @jlongster, @caspervonb, @kureev, @milankinen took five minutes to play with this. ;-)

kompot commented 9 years ago

@gaearon Tried the project you mentioned above and removing <Something1 /> makes <Something2 /> loose its state.

gaearon commented 9 years ago

Interesting observation: what if it's not hot(uniqueId), but reactClass(uniqueId).

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 tweakReactClass is

// 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.

gaearon commented 9 years ago

@kompot I think this is expected—it's just how React's diff algorithm works. If the children given to createElement as spread arguments change order, it will not reconcile, as it treats them as having implicit keys based on their order. Current version of React Hot Loader should have the same problem. I'm not sure it's really a problem though. In extreme cases when this is too inconvenient you can assign explicit keys to elements and it will work. I agree we could do better but I wouldn't want to invent our own diffing algorithm on top of what React already does.

kompot commented 9 years ago

@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)

sderosiaux commented 9 years ago

Hey, I tried a bit. It's working properly, just one question : not sure it's a bug, but if you remove the @autobind then the method tick() is not hot anymore (i guess it's part of the "function identity" RHL is using so it does not match anymore) ; the page is still running properly but the method with the @autobind is now a ghost function you can't access/modify (which is used on the running page).

gaearon commented 9 years ago

@chtefi Not sure what you are referring to? If I remove @autobind, its setTimeout will not see this.tick and will schedule undefined in a timeout (which will do nothing). If I then bind in the constructor (this.tick = this.tick.bind(this)) it works again.

Kureev commented 9 years ago

I think we can avoid specifying name of the module like @hot('RendererA') there if class has a name. Also, I have a question: why if I change RendererB, the whole app rebuilds? If I have a side-effects somewhere in the code, it may cause some unexpected issues.

gaearon commented 9 years ago

I think we can avoid specifying name of the module like @hot('RendererA') there if class has a name.

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.

gaearon commented 9 years ago

Also, I have a question: why if I change RendererB, the whole app rebuilds?

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:

sderosiaux commented 9 years ago

@gaearon yes indeed. Maybe I'm missing something but when i remove @autobind, HR happens, the next setTimeout(this.tick, speed) will call tick() which is not bound to this anymore, so, the js should crash no? It does not right now because it still have a reference to the @autobind tick() lost in space ? If I manually refresh my page it crashes properly (cannot read counter of undefined). Basically, my page after the HR behaves differently than after a manual reload.

gaearon commented 9 years ago

@chtefi Yeah, handling that is too complicated I think, and I'm not sure it's something worth pursuing. Filed as https://github.com/gaearon/react-proxy/issues/16.

Kureev commented 9 years ago

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)

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?

gaearon commented 9 years ago

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).

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.

I don't like that much a complexity you want to introduce by using hotify function

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 this.constructor and the class name will point to the same proxied class, many components in one file will work without caveats like needing to export them, and you can even edit components inside functions as long as consuming side has meaningful IDs attached to them. This just seems a less hacky and more stable approach, even at the cost of more work.

Kureev commented 9 years ago

What complexity are you referring to?

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 @hotify for every class).

milankinen commented 9 years ago

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. :+1:

Layer 1 - proxy

react-proxy seems quite "ready" to me. It does the same thing than react-hot-api and has a much cleaner API. I'd focus on the "second layer" now.

Layer 2 - decoration

The "decoration layer" and its heuristics will be absolutely the most challenging task (as @gaearon said) - especially the HOC case...

Detecting the right place of the hot(..) function
Defining the right identity for each hot(...) call

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 hot(..) functions, it is 100% replaceable without the need of replacing proxy/HMR. CoffeeScript etc.. developers may write their own transformations/language plugins for the task.

Layer 3 - hot module replacement

The 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 theme.js from you browserify-react-live 2nd example).

The Webpack hot-reloading tries to solve this with hot.accept(): it propagates the hot reloading to the dependent modules until all of the parents accept the reloading. Then (hopefully) all closures bind the new implementation. And this is the reason why livereactload reloads the whole bundle. Of course in livereactload's case the whole bundle reloading causes other (non-wanted) side effects because e.g. react and react-router modules have their internal state that is swept away during the reload...

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 ./) but not the global ones. Of course this causes side-effects (e.g. starting web sockets) of the user code to occur multiple times but I think the community has moved towards functional programming and "purity" (thanks to e.g. redux) and we should encourage people to write side-effect free code.

Of course the "accept" level detection is in the consideration of HMR implementer and more aggressive accepting may require some module.exports inspection (if all module's exports are React classes, then it's ok to accept module reloading).

tl;dr;

I like this idea :+1:
3 layers
- proxy layer has no dependencies
- other layers depend on only the proxy layer
- improvements to the proxy & decoration layers benefit all HMR layer developers
module.exports inspecting can't be avoided when implementing HMR

Kureev commented 9 years ago

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 theme.js etc - I'm currently considering the best way to implement hot replacement for that kind of modules. But it seems to be a more "general" approach, than React. Maybe we can find a way to write a universal module replacement + use react-proxy to prevent losing state for React components.

gaearon commented 9 years ago

@milankinen Nicely put, this is exactly how I think about it. :+1:

gaearon commented 9 years ago

Just published: https://github.com/gaearon/babel-plugin-wrap-react-components

jlongster commented 9 years ago

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: instance._reactInternalInstance._rootNodeID. It basically indicates where it is in the DOM. We really need to solve https://github.com/facebook/react/issues/4595 to get an official API for managing local state outside the tree though.

gaearon commented 9 years ago

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 react-hot-loader to later include a similar default set of useful wrappers (e.g. error handling + hotifying), for now, you can play with them here.

There are two new features in the updated demo:

The source is no longer annotated with @hot: Babel plugin applies transformations specified in .babelrc automatically.
Errors thrown inside render() are catched, as another example of wrapping transformation.

gaearon commented 9 years ago

You Will Be Shocked To Learn What Came Out Of This

https://github.com/gaearon/react-transform-boilerplate

gaearon / react-hot-boilerplate