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

Updates require multiple file saves #36

Closed
tomatau opened this issue Mar 24, 2017 · 12 comments
Closed

Updates require multiple file saves #36

tomatau opened this issue Mar 24, 2017 · 12 comments

Comments

@tomatau
Copy link

tomatau commented Mar 24, 2017

  • Node Version: v6.10.0
  • NPM Version: 3.10.10
  • koa Version: 2.0.1
  • koa-wepback Version: 0.3.1
// webpack.config.js
export default {
  entry: {
    head: [
      'webpack-hot-middleware/client',
      'babel-polyfill',
      `${APP}/entry.js`,
      `${STYLES}/main.scss`,
    ],
  },

Server integration:
https://github.com/tomatau/breko-hub/blob/koa2/src/helpers/hotReload.js#L24-L38

Actual Behavior

After making any file change, my server is logging

COMPILER DONE
webpack built a6f0726c3057b262a440 in 330ms
publish everyClient built

But sometimes the client logs nothing. However, if I repeatedly save the file (without changes), eventually my client will log

[HMR] bundle rebuilding
client.js?3ac5:207 [HMR] bundle rebuilt in 883ms
process-update.js?e13e:27 [HMR] Checking for updates on the server...

How can we reproduce the behavior?

Clone this branch on this boilerplate repository: https://github.com/tomatau/breko-hub/tree/koa2
Ensure you have a .env file with NODE_ENV=development. Enable the debugger (sensible defaults in example.env). Run npm start and make multiple file changes to any react component.

hot-reload Webpack compile started... +39s
  hot-reload Webpack compiling... +0ms
  hot-reload Clearing /app/ module cache from server +648ms
  hot-reload hot reloading server change /Users/tomatao/Projects/tomatao/js/breko-hub/src/server/webpack-assets.json +235ms
  hot-reload Webpack compile started... +2s
  hot-reload Webpack compiling... +1ms
  hot-reload Clearing /app/ module cache from server +285ms
  hot-reload hot reloading server change /Users/tomatao/Projects/tomatao/js/breko-hub/src/server/webpack-assets.json +133ms
  hot-reload Webpack compile started... +2s
  hot-reload Webpack compiling... +1ms
  hot-reload Clearing /app/ module cache from server +344ms
  <-- GET /1691eb07362da1beccb5.hot-update.json
  --> GET /1691eb07362da1beccb5.hot-update.json 200 1ms 43b
  <-- GET /0.1691eb07362da1beccb5.hot-update.js
  --> GET /0.1691eb07362da1beccb5.hot-update.js 200 2ms 8.03kb
  hot-reload hot reloading server change /Users/tomatao/Projects/tomatao/js/breko-hub/src/server/webpack-assets.json +126ms

You can see a "Webpack compile started..." for each file save, but only eventually does the request come through for the hot-update chunks.

Notes

Thanks for making this library. I originally tried koa-webpack-middleware but it was breaking mime types as the hot middleware set octetstream on every response.

My boilerplate (linked) was working fine in every way for hot reloading (even server side live updates and hot updates of css-modules). I have made the minimal changes to migrate to koa2 and debugging this issue has left me stumped. I have thrown logs all over webpack-hot-middlewares client middleware and server middleware and can see the server sending messages but the client only receiving a subset of them. Eventually if I CMD+S on my file enough times after making a change the client will update.

This may be a more appropriate issue for webpack-hot-middleware but that repository seems less responsive to issues.

@shellscape
Copy link
Owner

shellscape commented Mar 24, 2017

Have you tried running this with the default dev and hot config?

It may be a pain, but to determine which module is breaking you could deconstruct koa-webpack and manually wrap and add the middleware in your app. That'd root out the possibility that it's an issue with composing the two.

On the surface this looks like it might be a problem with webpack-hot-middleware. I'll have a look at the boilerplate later on this evening, thanks for linking to that.

@tomatau
Copy link
Author

tomatau commented Mar 24, 2017

Have you tried running this with the default dev and hot config?

Yeah, I started without the custom path and added it for debugging. It makes no difference.

It may be a pain, but to determine which module is breaking you could deconstruct koa-webpack and manually wrap and add the middleware in your app. That'd root out the possibility that it's an issue with composing the two.

I'd have no idea how to wire up koa2 to the express middleware without the passthrough stream! I've rebuilt the middleware internally to my app so I can debug and remove options that aren't appropriate for my use case but still the same issue. I can debug all the way down to webpack-hot-middleware and see that it's publishing the "built" messages to clients but the clients aren't receiving them until the messages are fired twice.

On the surface this looks like it might be a problem with webpack-hot-middleware. I'll have a look at the boilerplate later on this evening, thanks for linking to that.

Thanks that would be really appreciated. I've pushed a new commit on the koa2 branch with your middleware baked in for convenience of debugging.

@shellscape
Copy link
Owner

Well the good news is that you're not 🍌 s. I was able to replicate the problem last night. The bad news is that I couldn't come up with a solid reason as to why. An idea a friend proposed was to use a 3rd party file listener module as middleware and publish the update event to hot-middleware manually. That really shouldn't be necessary, but I can't figure out the why part of it not working correctly.

@tomatau
Copy link
Author

tomatau commented Mar 27, 2017

I did more digging and think it's related to koa-compress - it seems to be trying to gzip the event-stream... I have no idea why though! And also when passing a filter function to explicitly not compress the event-stream, it still does?... Something's definitely off

@shellscape
Copy link
Owner

@tomatau fascinating. when removing that middleware, was everything working as it should? And if so, I wonder if that begs the question of if we can do something to recognize that and compensate for it, outside of the webpack-* middleware.

@tomatau
Copy link
Author

tomatau commented Mar 27, 2017

koa-compress under the hood is using a library called compressible. Compressible see's that the responses content type contains text as it's "text/event-stream" and returns true.

The simple fix I've done here is:

app.use(compress({
  filter: type => !(/event\-stream/i.test(type)) && compressible(type),
}))

There is a way to disable compression using ctx.compress = false but baking this into koa-webpack feels a bit specific to koa-compress' problems?

@shellscape
Copy link
Owner

yeah, agreed that it's very specific to koa-compress. I can add a note to the README point to this issue and your last comment for users of that middleware. sound good?

@tomatau
Copy link
Author

tomatau commented Mar 27, 2017

Yep sounds good

@tomatau tomatau closed this as completed Mar 27, 2017
@shellscape
Copy link
Owner

Thanks for working through this one, I've no doubt it'll help some folks out down the road with a similar setup.

@Nicholaiii
Copy link

Just a quick note about the performance of the work around; you can increase performance a bit by storing the regexp literal into a const, so it doesn't have to be compiled more than once during runtime.
Under low load, this is of course negligible, but we are talking 3-4 times better performance
Judge for yourself on jsPerf

JounQin added a commit to JounQin/vue-ssr that referenced this issue Sep 5, 2017
… inside and I believe there is nothing wrong with koa-compress or compressible

shellscape/koa-webpack#36 (comment)
@talha-asad
Copy link

talha-asad commented Nov 9, 2017

Reading up here and here

This behavior can be fixed by changing the headers on the event and then manually flushing it similar to how it is done on the express example. Do we really need to overwrite the default filter function? as it is correct in compressing text/event-stream.

@mrchief
Copy link

mrchief commented Jan 19, 2018

@talha-asad Were you able to make your approach work with koa? Would you mind sharing a gist or code sample?

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

No branches or pull requests

5 participants