Skip to content
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

Using webpack-dev-middleware instead of webpack-dev-server directly #2034

Merged
merged 3 commits into from
Dec 5, 2016

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Nov 28, 2016

Also switched from react-hot-loader to webpack-hot-middleware

@fbarl
Copy link
Contributor Author

fbarl commented Nov 29, 2016

@davkal I think it should be ready now. I removed OccurrenceOrderPlugin as it didn't seem necessary and on my machine it was slowing down recompilation time by 5-7%.

@davkal
Copy link
Contributor

davkal commented Dec 1, 2016

Bundling and hot-reload works. Although I noticed the following:

  1. set breakpoint in a JS file and see it break
  2. change JS file and see it hot reload
  3. notice how the breakpoint is no longer working

Does this happen in master too? Is there something that can be done about it?

@davkal
Copy link
Contributor

davkal commented Dec 1, 2016

Also, node keeps crashing for me after some time.

$ npm start

> weave-scope@1.2.0 start /Users/david/weave/src/github.com/weaveworks/scope/client
> node server.js

Scope UI listening at http://:::4042
Scope Proxy Path UI listening at http://:::4043/scoped/
webpack built 3f997545c9c66947efb7 in 37629ms
webpack building...
webpack built e9a8fddd0b61b6fb2cd1 in 8170ms
webpack building...
webpack built 697ca15b25ee7de884a8 in 8999ms
webpack building...
webpack built 6eb71654772bc6ccb061 in 8215ms
webpack building...
webpack built a908aa8498f177a69648 in 10110ms
Abort trap: 6

$ node -v
v6.9.0

@davkal
Copy link
Contributor

davkal commented Dec 1, 2016

This PR could be helpful for node crashing: Graylog2/graylog2-server#2433

@fbarl
Copy link
Contributor Author

fbarl commented Dec 1, 2016

@davkal I'm afraid I didn't manage to fix any of the problems you had :/

  1. set breakpoint in a JS file and see it break
  2. change JS file and see it hot reload
  3. notice how the breakpoint is no longer working

Does this happen in master too?

Yes it does (see this comment) - for me it was the case even before merging #2017, although that might have been due to a Chrome DevTools bug.

Is there something that can be done about it?

I gave it a couple of hours of research, but haven't come up with any good solutions. By all chance it is a problem with source maps setup which gets fixed if we switch back to using devtool: 'eval' in webpack.local.config.js, but that's supposedly not what we would want to do, since in #2011 we decided to go with devtool: 'source-map'. The root problem for me on Chrome (and Firefox) seems to be source code in browser dev tools not getting refreshed after a successful hot reload. webpack/webpack#2478 suggests that getting source maps work with React & webpack-hot-middleware might be a tricky thing. The Chrome browser workaround to refresh the dev tools unfortunately doesn't rerun any code that would hit the breakpoints again.

I wonder how much this source maps issue is platform dependent and whether we'd get exactly the same issues on Safari. @jpellizzari, maybe you could take a quick look at this and see if you could find a fix, since you already added source maps to this project successfully once :)


Also, node keeps crashing for me after some time.

I really couldn't reproduce the crash you had even after multiple file updates:

webpack building...
webpack built baa902bc1839453ffbec in 14052ms
webpack building...
webpack built bc80ba7cbdfabfa808d0 in 12749ms
webpack building...
webpack built f5aed7fb9ce2f105ecf4 in 11552ms
webpack building...
webpack built 6c1759dabdca29d7023e in 11193ms
webpack building...
webpack built 5b064d7a26266ce26955 in 12285ms
webpack building...
webpack built 00717b2180bb69b8ca8b in 11273ms
webpack building...
webpack built 55028de6862cef7e14da in 11365ms
webpack building...
webpack built 4c969f9733c98ddef08a in 10726ms
webpack building...
webpack built 01583b476d29c090c780 in 11705ms
webpack building...
webpack built dcf01c118124bddfd759 in 11179ms
webpack building...
webpack built 7085f2874001065e5ccf in 11407ms
webpack building...
webpack built cc70d602aa623a736565 in 11306ms
webpack building...
webpack built 35c76d0d878049c325e4 in 11162ms
webpack building...
webpack built 9684d1a648f03dbcab03 in 11326ms
webpack building...
webpack built 40d2813cae444b0f66b6 in 12540ms
webpack building...
webpack built b7c764d4dbb4e3c326cb in 12193ms
webpack building...
webpack built a3357312ca2fa563391a in 12346ms
webpack building...
webpack built ba1dcae3202bc9cc87fe in 11961ms
webpack building...
webpack built 63e10b675dccdc15105a in 11485ms
webpack building...
webpack built 3d90575ab386d5a07d19 in 13392ms

However, if it's related to the PR you referenced, it is most likely coming from webpack-dev-server, as the list on http://localhost:4042/webpack-dev-server on master seems to be growing with every new hot update. @davkal, could you maybe try reproducing the error and then see if it still appears after raising the node memory limit as suggested in this comment?

@jpellizzari
Copy link
Contributor

jpellizzari commented Dec 1, 2016

@fbarl I am able to reproduce the issue that @davkal reported, even with the transpiled code. Unfortunately, I don't have any bright ideas on how to fix it; the issue link you provided is pretty convincing that this is a bug with chrome devtools.

One workaround (in addition to alt-r, like the chromium issue suggests) is using a debugger statement in the source, which seems to work after a hot reload:
screen shot 2016-12-01 at 9 31 58 am

When I changed the devtool, I thought it would be useful to have the original code for debugging. I forgot that most of the variable names would be changed by babel, making it almost impossible to log things to console while paused at a breakpoint. Is everyone else having the same issue?

If so, my vote is that we go to devtool: 'cheap-source-map' to get the transpiled code for debugging.

@fbarl
Copy link
Contributor Author

fbarl commented Dec 2, 2016

Thanks for looking into it @jpellizzari! Here's my feedback:

...the issue link you provided is pretty convincing that this is a bug with chrome devtools

I would have also concluded so if I didn't have the same problem on Firefox :/ That's why I was curious to see whether the same source maps reloading problem happens on Safari with our current configuration.

One workaround [...] is using a debugger statement in the source, which seems to work after a hot reload

If we decide to abandon using breakpoints in combination with hot reloading, I agree using debugger statement would be the best way to go. In that case, I think we could optimize hot loading recompiling time 2-3 times without downgrading source maps, by using devtool: 'eval-source-map'.

@davkal what are your thoughts on this?

p.s. @jpellizzari, I'm not able to reproduce your issue with variables being renamed. So the source code gets mapped back to the original for you, but you're not able to access the original variables from the console, when paused on a debugger statement? Maybe that's another browser issue :/ My understanding is that the options eval-source-map and source-map should really map back to the original code, i.e. the state before babel does any renaming.

@davkal davkal merged commit 5ae3a5a into master Dec 5, 2016
@davkal davkal deleted the use-webpack-dev-middleware branch December 5, 2016 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants