Add hot reloading #2095
Add hot reloading #2095
Conversation
When a tag gets re-defined (eg `riot.tag2`), the hot reloader looks through DOM, finds every instance of that tag, and replaces it.
Replacing tags goes through the full lifecycle: the old instance is un-mounted, and the new instance calls the constructor and mount callbacks. Hot reloading will preserve the tag's original attributes, yields, and state.
If you need to do anything custom before or after the reload, you can use the callbacks:
```
riot.util.hotReloader.on('before-unmount', () => /* some cleanup */)
riot.util.hotReloader.on('after-mount', () => /* some setup */)
```
This is useful when you have 3rd party libraries that do DOM manipulation.
Here's a gist on using it with Gulp:
https://gist.github.com/rogueg/4d48d32bd678aba784455ef8685c4494
|
@rogueg thanks for your pull request but I would like to create another repo called |
|
I considered that, but the hot-reload code is tightly coupled to riot. Putting them in the same repo makes it easy to ensure that you always have the right version. Setting up a new repo is a hassle, making a build, test setup, npm package, etc. Could you elaborate on the advantages of a separate repo? |
|
I consider hmr an additional feature to riot but not part of the core. I have never seen other libs adding new bundles just to handle this feature see also: https://github.com/vuejs/vue-hot-reload-api I am just afraid to have too many riot bundles to maintain in the same repo, now we have: I would rather prefer to give the access to internal riot APIs to handle this feature in an external script in another repo that our users could eventually install via npm. I would like to hear also the opinion of the other @riot/core-maintainers on this |
|
React native is an example of a lib that includes hot reloading. Most projects you reference are Webpack-specific, but thats not what this PR is. I feel like the goal is to make hot reloading just work for our users, and this does that. I still don't see any advantage to a separate repo. |
|
I'm waiting the team of Rollup supports hmr... rollup/rollup#50 BTW, I have no preference to include it or not. Could it be a option to include it in the loader module together? |
|
Brunch handles HMR with if (module.hot) {
module.hot.accept(module, callback);
} |
|
@rogueg I would like to move your code here https://github.com/riot/hmr providing a simple api that anyone could use independently form the bundler they are using. |
|
All this code does is expose a small, optional API that allows for replacing tags at runtime. It doesn't do HMR (there aren't even modules!), and it isn't specific to any particular build tool. Most users will never interact with this API directly. @GianlucaGuarini you mention that users could "install it via npm", but that shouldn't ever happen. Instead, the API will be used by the various build tools: gulp-riot, riotjs-loader, riotify, etc. When the user sets up one of those, they get hot-reloading for free, without any additional configuration. Once this PR gets merged, we need to make PRs against all those build tools to use this API. Only after that happens will our users actually be able to benefit from hot reloading. Hot reloading is tightly coupled with the internals of riot. This hot-reload code (for riot@3.0) won't work with riot@3.1. Putting them in the same repo ensures that you'll always have matching versions. |
|
@rogueg I have tried your patch with a simple example here and I've got some issues. That's exactly the reason I would like to have it into its own separate repo to create a solid and specific unit test only for this feature and to avoid to overlap its issues with the ones riot specific. @ptb seems to have made already a pull request on riot-hmr that could be also renamed Side note: I really like the @rogueg approach because it magically works, but I guess that a more explicit api could be better suitable to the current increasing complexity of the frontend stacks allowing our users deciding by themselves when a tag an which one should be reloaded |
|
Seems like your example code itself is causing the issue. If you change FWIW, this commit does add tests for hot reloading. |
|
@rogueg ok sorry I made a mistake here you can check the updated example http://plnkr.co/edit/5jx5T1iK0LeS2EiQbPBI?p=preview and as you can see the |
|
It's working as intended. My understanding is that React and Vue do the same thing. |
|
Ah ok nevermind I guess the |
|
Sounds fine to me. |
|
@rogueg thank you so much for this patch I really like it |
|
the @rogueg patch was published as separate riot module and it's already being used in our official webpack loader I am closing this pull request |
When a tag gets re-defined (eg
riot.tag2), the hot reloader looks through DOM, finds every instance of that tag, and replaces it.Replacing tags goes through the full lifecycle: the old instance is un-mounted, and the new instance calls the constructor and mount callbacks. Hot reloading will preserve the tag's original attributes, yields, and state.
If you need to do anything custom before or after the reload, you can use the callbacks:
This is useful when you have 3rd party libraries that do DOM manipulation.
Here's a gist on using it with Gulp:
https://gist.github.com/rogueg/4d48d32bd678aba784455ef8685c4494