-
Notifications
You must be signed in to change notification settings - Fork 30
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
Idea: ERB helper to easy enabling HMR #17
Comments
Again, why this javascript can't be part of the JS library? |
Yes, you can find more context here: renchap/webpacker-react-example#2 (comment) We havent been able to find documentation on the internals for HMR in Webpack 2. I would have preferred a 100% JS solution where you only have to register your component, but it does not seem possible due to the way HMR works. |
@renchap i like the idea, alternative would be a babel plugin, but the .erb solution is just much simpler. 👍 |
@sevos do you think we can have a simpler pure-JS solution? I tried again to look at how to make this work with Webpack & hot-reloading, without any results. I will work on implementing the ERB otherwise. |
First, what do you think if we define the requirements explicitly? Like a real ticket at a software product dev. What is the exact challenge? |
Sure. I have several goals:
|
I have been thinking on how to improve our HMR support and have a way to toggle it globally while reducing the boilerplate.
The best idea I have is to provide a Ruby helper and have people use
packs/application.js.erb
.The interface would look like:
Which will generate the following JS code:
We can then have a Rails config flag to enable / disable HMR, as well as a
register_react_components
param to override it. This will also allow to only enable HMR (and lot relevant code) depending on Rails environment.Webpacker enables ERB processing in Webpack config by default, so this should work out of the box. I dont fully like the need to have the users move their packs to ERB, but it looks like much more flexible than any other solutions I thought about.
@sevos @mfazekas @daninfpj any feedback?
The text was updated successfully, but these errors were encountered: