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

Replace pre-emptive compiler with additional pass #28

Merged
merged 2 commits into from
Sep 9, 2016
Merged

Conversation

mzgoddard
Copy link
Owner

Fixes #27

webpack 2 can run as many compile passes as needed by responding to a
new plugin point called need-additional-pass. The way this PR supports
accurate used modules is through using additional passes as necessary.
This has the benefit of only using one pass when only one is needed and
working with other plugins like HMR that the pre-emptive step was
breaking in cases.

webpack 2 can run as many compile passes as needed by responding to a
new plugin point called need-additional-pass. The way this commit
supports accurate used modules is through using additional passes as
necessary. This has the benefit of only using one pass when only one is
needed and working with other plugins like HMR that the pre-emptive
step was breaking in cases.
@mzgoddard mzgoddard merged commit fb5f7ce into master Sep 9, 2016
@mzgoddard mzgoddard deleted the webpack-2-hmr branch September 9, 2016 04:55
@MoOx
Copy link

MoOx commented Sep 9, 2016

Thank you so much for you work!
I am still having issue:

It works with your plugin enabled when there is no cache yet, but does not work for the 2nd build (when cache is used).

The first time HMR push updated as expected, the second time the dev server send this messages (same as before)

[HMR] Checking for updates on the server...
[HMR] Cannot find update (Full reload needed)
[HMR] (Probably because of restarting the server)

@mzgoddard
Copy link
Owner Author

@MoOx I want to guess that you need to clear your cache. You changed the webpack config without clearing the cache (like turning on HMR), leaving modules from a previous render built without the HMR plugin in a build that has HMR on. Those modules that accept hot module changes will have the code that would execute to do that disabled from a build that had HMR disabled.

This raises a really interesting edge case for #17. Its not just being in development or production but it can also be running with the webpack cli tool versus the webpack-dev-server cli tool since the dev-server modifies the webpack options. That is something I wasn't thinking about for #17, so that'll help while I figure out the solution for that. I wish that had a simple answer.

@thangngoc89
Copy link

@mzgoddard The dev server always modify the cache in the same way, not sure what's causing this. With webpack 1, everything works perfectly.

@MoOx
Copy link

MoOx commented Sep 9, 2016

Like we said in #17, we use a hash to create/reuse a cache folder. So if HRM worked without cache, and then right after a restart stopped working, isn't it a hard-source plugin issue?
In both run, HMR was enabled so config didn't changed. (I deleted the cache by hand in the first place, just in case).

@mzgoddard
Copy link
Owner Author

mzgoddard commented Sep 9, 2016

@MoOx it could still be hard-source. My hesitance in thinking its that and not clearing the cache or some config thing is from trying to reproduce your stated error. I tried with HMR with the dev-server and hot-middleware. I hit the error with the hot-middleware (though I understand how to repro with dev-server too now). When I looked at the output script I saw that my module.hot block for a stylesheet hadn't been evaluated. It wasn't evaluated because the module was from a cache that had been built without HMR enabled.

Can you look in your built script for the module.hot block that should be working but isn't? Whether its hard-source or not, you should have a module.hot block to accept your changes.

For example style-loader creates a block like

if (module.hot) {
  module.hot.accept('./path/to/content.css', function() {
    var newContent = require('./path/to/content.css');
    /* do stuff with it */
  });
}

With HMR disabled the output of that would look like

if (false) {
  module.hot.accept('./path/to/content.css', function() {
    var newContent = require('./path/to/content.css');
    /* do stuff with it */
  });
}

With HMR enabled it'll look like:

if (true) {
  module.hot.accept(13, function() {
    var newContent = require(13);
    /* do stuff with it */
  });
}

@MoOx
Copy link

MoOx commented Sep 10, 2016

OK, I have if (true)... Not sure what I changed, but seems to be working now...
Will be more careful next time, sorry for the false alarm...

@mzgoddard
Copy link
Owner Author

@MoOx no worries. This at least gives me more to document for others and more to think about for stuff like #17 to reduce the possibility you or others using HardSource run into that problem.

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