webpack-hot-client icon indicating copy to clipboard operation
webpack-hot-client copied to clipboard

revert(#77): incorrect check for existing socket clients

Open alexander-akait opened this issue 7 years ago • 17 comments

Currently version doesn't work with https://github.com/webpack-contrib/webpack-serve/blob/master/docs/addons/watch-content.config.js, here description why https://github.com/webpack-contrib/webpack-hot-client/issues/93 . Also if you have other additional websocket clients it is also break hot reloading.

We should reopen https://github.com/webpack-contrib/webpack-hot-client/issues/61 for searching better fix

Type

  • [x] Revert

Issues

  • Fixes #95

Semver

  • [x] Patch

alexander-akait avatar Jul 27 '18 14:07 alexander-akait

@shellscape And? right now hot broken when you try to use https://github.com/webpack-contrib/webpack-serve/blob/master/docs/addons/watch-content.config.js example (just try run and looks how hot broken), also additional websocket clients break hot, the previous PR implemented the incorrect fix.

https://github.com/webpack-contrib/webpack-hot-client/issues/61 is rare use case (i.e. rare use case vs documented feature what is broken). Also PR was merged without tests, what mean nothing can say it is right fix or not.

alexander-akait avatar Jul 27 '18 15:07 alexander-akait

And what? You're asking to revert a PR that fixed a bug, in order to fix a bug with your config. That will break the previous fix. You're asking to introduce a regression to fix a regression. That is not even remotely OK.

shellscape avatar Jul 27 '18 15:07 shellscape

@shellscape my config? Try to use your example from https://github.com/webpack-contrib/webpack-serve/blob/master/docs/addons/watch-content.config.js. You merge incorrect fix and don't wan't fix it?

alexander-akait avatar Jul 27 '18 15:07 alexander-akait

You're asking to introduce a regression to fix a regression. That is not even remotely OK.

This PR must also contain a fix for #61 to be merged. You may not introduce a regression to fix a regression.

shellscape avatar Jul 27 '18 15:07 shellscape

@shellscape This is not possible to fix because developer have two wesockerts in one window (i.e. two hot clients). It can be solve only using multi compile mode (he should prebuild iframe entry and include already built script(s)).

alexander-akait avatar Jul 27 '18 15:07 alexander-akait

/cc @shellscape I understand correctly that you are ignoring fixing incorrect fix (no tests, no docs, no workarounds) and confirm that the current version does not work with officially documented feature as watch-content?

alexander-akait avatar Jul 27 '18 15:07 alexander-akait

@shellscape Not wanting to get into the middle of some long going issues you and @evilebottnawi has, but I can +1 on this issue.

HMR does not work for anything but the first listed module in cases like this: https://github.com/webpack-contrib/webpack-serve/issues/119#issuecomment-412071796

To me this seems just as serious as #61 since there's not even a fallback with refresh in this case, it just silently decides there's not any change.

hedefalk avatar Aug 10 '18 13:08 hedefalk

@hedefalk Due to some misunderstanding between developers now solving the question about webpack-serve, it is possible that this project will no longer be supported, shellscape not professionally behaved as a developer in some aspects. We apologize for the current situation, in the near future it will be resolved.

alexander-akait avatar Aug 10 '18 13:08 alexander-akait

I am running into the same issue as this. Since webpack-dev-server just doesn't work correctly with HTTPS, this project seemed like good and more modern alternative.

Hopefully the misunderstandings @evilebottnawi mentioned, can be resolved. It would be a shame that such a project would be abandoned.

blackshadev avatar Aug 17 '18 09:08 blackshadev

@blackshadev better use webpack-dev-server and create issue about https or mention me if issue already exists. The project will merge with webpack-dev-server in near future.

alexander-akait avatar Aug 17 '18 09:08 alexander-akait

I mentioned you in both issues on the webpack-dev-server. see issue 1449 and issue 851

blackshadev avatar Aug 17 '18 10:08 blackshadev

@blackshadev thanks!

alexander-akait avatar Aug 17 '18 10:08 alexander-akait

I hate it when you guys fight.

j-walker23 avatar Dec 06 '18 12:12 j-walker23

@j-walker23 this package abadoned, don't use it

alexander-akait avatar Dec 06 '18 12:12 alexander-akait

@j-walker23 I have moved on from this org a long while ago. it's unfortunate that the webpack-contrib org is unable or unwilling to work on it. if you have any issues feel free to reach out to me personally and I'll be happy to help you work on a fork, or fork and help with any issues much like I did for several other webpack-contrib packages.

(I don't get notifications from this org any longer, so I likely won't see replies. Someone was kind enough to let me know about yours.)

shellscape avatar Dec 06 '18 12:12 shellscape

@shellscape Thanks man, very nice offer, I really appreciate that! I just moved from jspm to webpack yesterday, and it's been quite the rollercoaster. Webpack seems awesome. But man, when something doesn't work and no help can be found from google, it's a high learning curve.

Thanks for the heads up too. I am going to fork now, the code in these alternative webpack and HMR packages that you authored are so much easier to read so I really didn't want to go back to default WDS and friends if i didn't have too.

I'm trying to figure out a HMR setup to a existing non js server app (python) and it's proving difficult. I'm not to the point where i could feel smart enough to ask decent questions just wasting your time but maybe if I'm having trouble glueing everything together at the end i might bother you. Thanks again.

j-walker23 avatar Dec 06 '18 13:12 j-walker23

Issue addressed in https://github.com/hedgepigdaniel/webpack-hmr-client

hedgepigdaniel avatar Oct 03 '19 07:10 hedgepigdaniel