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

resolve webpack-hot-middleware conflicts #49

Closed
wants to merge 4 commits into from
Closed

Conversation

xughv
Copy link

@xughv xughv commented Mar 19, 2020

When I use webpack-hot-middleware with a custom path:

server.js

...
app.use(
  require('webpack-hot-middleware')(compiler, {
    log: false,
    path: '/__my_hmr',
  })
);
...

webpack.config.js

...
entry: {
  main: ['webpack-hot-middleware/client?path=/__my_hmr', './src/index.js'],
},
...

The client will send two HMR requests:

image


The require('webpack-hot-middleware/client') in WHMEventSource.js will create a new EventSourceWrapper, and the path (from query) is the key of EventSourceWrapper in webpack-hot-middleware:

https://github.com/webpack-contrib/webpack-hot-middleware/blob/cb29abb9dde435a1ac8e9b19f82d7d36b1093198/client.js#L119

So inject user's webpack-hot-middleware/client entry before ErrorOverlayEntry, and try to customize overlay on the existing client.

https://github.com/webpack-contrib/webpack-hot-middleware/blob/cb29abb9dde435a1ac8e9b19f82d7d36b1093198/client.js#L302
https://github.com/webpack-contrib/webpack-hot-middleware/blob/cb29abb9dde435a1ac8e9b19f82d7d36b1093198/client.js#L152

@pmmmwh
Copy link
Owner

pmmmwh commented Mar 19, 2020

CC @blainekasten would be great if you can provide some insights on this.

I will take a look at the WHM docs tomorrow.

@xughv
Copy link
Author

xughv commented Mar 20, 2020

In short, via

  • webpack-hot-middleware/client in the entry array
  • require('webpack-hot-middleware/client') in WHMEventSource.js

the WHM client connect() will be executed twice.

If they have different path(from resource query), the client will create connection for each path.

@KevinGruber
Copy link

@pmmmwh we face the same issue in our cli. we use a seperate server with webpack-hot-middlware and we also see that behaviour. Would be cool if you could merge that #soon.

@pmmmwh
Copy link
Owner

pmmmwh commented Mar 29, 2020

Hi! I have taken a look at this and I think a better way to handle this would be to:

  1. Make the ErrorOverlayEntry the last entry in the chain, and
  2. Allow the plugin to accept custom sockPath (see Add custom sockHost/sockPort/sockPath #52)

(I'm assuming that requiring the client with the same path will not execute connect twice, but please correct me if I am wrong)

Update:
Please check #64 if you have time, it should solve most of the issues :)

@xughv
Copy link
Author

xughv commented Apr 3, 2020

😮 I guess sockPath cannot solve this problem.


Only use the WHM singletonKey in WHMEventSource.js to custom overlay can solve it now.
https://github.com/webpack-contrib/webpack-hot-middleware/blob/cb29abb9dde435a1ac8e9b19f82d7d36b1093198/client.js#L152

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 3, 2020

Only use the WHM singletonKey in WHMEventSource.js to custom overlay can solve it now.

There is handling for the singletonKey - sockPath is only a fallback.

@xughv
Copy link
Author

xughv commented Apr 3, 2020

Only use the WHM singletonKey in WHMEventSource.js to custom overlay can solve it now.

There is handling for the singletonKey - sockPath is only a fallback.

😨 But it seems that webpack not support require() dynamically by variable.

Maybe sockPath cannot be used as a fallback.

image

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 4, 2020

😨 But it seems that webpack not support require() dynamically by variable.
Maybe sockPath cannot be used as a fallback.

Yes, I am experimenting with dynamic import or just scrap the idea of injecting the sockPath into the fallback. Still debating whether doing something like that makes any sense ...

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 7, 2020

Since #64 has been merged I'll close this for now.

@pmmmwh pmmmwh closed this Apr 7, 2020
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.

None yet

3 participants