search

gaearon / react-hot-boilerplate

Minimal live-editing example for React
MIT License
3.91k stars 879 forks source link

Bring @next in line with webpack 2 official guide #111

Closed petertrotman closed 7 years ago

petertrotman commented 7 years ago

With the stable release of webpack 2, it may be worth switching the dependency to use it. This branch aims to match the guide provided at https://webpack.js.org/guides/hmr-react/ and works well with webpack 2.

It's no longer compatible with webpack 1, so I understand if you don't want to merge it. However, this repo is what I used as a guide for implementing RHL before webpack 2 - and I imagine there are others who do the same - so it would be nice if the webpack-supplied guide was at least referenced here.

calesce commented 7 years ago

Hey this looks great! Now that Webpack 2 is in full release this should be the default. I've cut a next-webpack1 branch for people who can't upgrade.

petertrotman commented 7 years ago

Regarding using express rather than webpack-dev-server:

The overlay that you reference can be activated in the webpack.config.js by inserting inline: false in the devServer configuration object. However, (for reasons I'm not entirely clear on) the new webpack docs recommend using the inline mode with HMR and it is on by default https://webpack.js.org/configuration/dev-server/#devserver-inline-cli-only . When the dev server is not in inline mode, it is in iframe mode, which applies the overlay by displaying its own main page and embedding the app in an iframe which has its own issues in development.

Additionally, speaking as someone who was a beginner at this not too long ago, the use of the express server rather than webpack-dev-server (like most of the other boilerplates implementing RHL) isn't explained anywhere and for me it was a source of confusion. If there are other reasons for using it - such as known issues with webpack-dev-server - then I am open to the idea of using the express server, but I think it will be important to clearly document the reasons why.

My feeling on this matter is that the guide over at https://webpack.js.org/guides/hmr-react/ is very solid and, to avoid confusing people new to the scene, this repo should aim to mirror that as much as possible unless there are really compelling reasons otherwise (which should be clearly documented). Ideally, in my opinion, this repository would either be a simple reference implementation of that guide, or it should take that guide one step further with best-practice configuration for production.

Looking forward to your thoughts.

calesce commented 7 years ago

The overlay that you reference can be activated in the webpack.config.js by inserting inline: false in the devServer configuration object

Yeah I've messed around with all of the options, and haven't successfully disabled inline mode to make iframe notifications appear, whether by CLI options or devServer config. I'm wondering if the docs are out of date. I also haven't found a way to quiet the logging in the DevTools console. 😛

My feeling on this matter is that the guide over at https://webpack.js.org/guides/hmr-react/ is very solid and, to avoid confusing people new to the scene, this repo should aim to mirror that as much as possible unless there are really compelling reasons otherwise (which should be clearly documented)

Totally agreed. WDS is used more in the community documentation/blog posts. It also makes the code/configuration much simpler for this project (no server.js). I'll merge this, and hopefully people can make the WDS experience as nice as possible.

Thanks again!

petertrotman commented 7 years ago

Nice - with the overlay, if you switch inline: false then by default WDS serves the app with overlay at http://localhost/webpack-dev-server/bundle . I had a quick look at the docs but I didn't see an easy method of changing the path. Perhaps by using the proxy settings but I haven't messed with it.

Also I found a glaring error in this PR - fixed in my new PR! Sorry about that...

calesce commented 7 years ago

Ah ok, yeah that's much better. It's kind of annoying to have to go to /webpack-dev-server but that can probably be fixed.

I also don't like the persistent bar, I'd rather just have the overlay appear for build errors. There's already a feature request for it: https://github.com/webpack/webpack-dev-server/issues/735