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

Publish correct action for built modules in multi-compiler (built/sync) #312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kronuz
Copy link

@Kronuz Kronuz commented May 25, 2018

In the client side, when receiving the "built" action, it tries to load a manifest for the updated chunks, but such manifest is only available when the bundle actually has changed (is not available the first time either, for example), so the result is an error message in the client saying it's getting a 404 for such manifest. Changes where made so it only tries to load such manifest when "built" action is received, but not "sync".

In the server side, this pull request makes it so in multi-compiler each bundle is sent with proper action. Not all bundles change when one does, so not all should send "built" action, which would try to load a manifest that might not be there.

Fixes issue #270

@jsf-clabot
Copy link

jsf-clabot commented May 25, 2018

CLA assistant check
All committers have signed the CLA.

@glenjamin glenjamin self-assigned this May 26, 2018
@glenjamin
Copy link
Collaborator

I'm struggling the follow this scenario.

Part of this will be because the existing events I've implemented are not well documented.

I'm pretty sure the sync event was introduced to handle clients connecting to a new server and making sure they're in sync.

I don't think there should need to be special-cased logic to skip a sync. If the client bundle has the same name and doesn't have the same hash, it should try and update against the server.

It does sound from your description that the bug is to do with when the server sends events to clients.

Another thing i've just noticed is that the example/ in this repo doesn't follow the advice given in the README: https://github.com/webpack-contrib/webpack-hot-middleware#multi-compiler-mode

The multi-compiler support is a bit patchy, it's been added ad-hoc by multiple people over the years.

This pull-request seemed like it would improve support in general, but the branch went stale without anyone to champion it #155

@Kronuz
Copy link
Author

Kronuz commented May 27, 2018

The thing is for multi-compiler support to be fully functional and easier to setup, there are also a couple changes needed in the main template plugins

@Kronuz
Copy link
Author

Kronuz commented May 27, 2018

I created a feature request in webpack for that: webpack/webpack#7406

@glenjamin
Copy link
Collaborator

Thanks for looking into this. It’s probably also worth comparing notes with webpack-hot-client to see what it does

@Kronuz
Copy link
Author

Kronuz commented May 27, 2018

Apparently, webpack-hot-client is also using module.hot and __webpack_hash__, which would work only for the bundle where the client is running on.

Kronuz added a commit to Kronuz/UniversalWebpackTarget that referenced this pull request May 28, 2018
multi compiler mode doesn't work.
[webpack-contrib/webpack-hot-middleware#270]

Publish correct action for built modules in multi-compiler
[webpack-contrib/webpack-hot-middleware#312]

First class hot reload support in Multi-Compiler mode
[webpack/webpack#7406]
@IIIristraM
Copy link

Are any updates on this issue ?

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.

4 participants