HMR code block should not be output for distribution, should accept an option

[drawyan]

Do you want to request a feature or report a bug?

Bug

What is the current behavior?
The output always has the following template (source code in index.js):

// Hot Module Replacement
if(false) {
	// When the styles change, update the <style> tags
	if(!content.locals) {
		module.hot.accept("!!../node_modules/some-npm-registry/postcss-loader/2.0.6/node_modules/postcss-loader/lib/index.js!./myComponent.pcss", function() {
			var newContent = require("!!../node_modules/some-npm-registry/postcss-loader/2.0.6/node_modules/postcss-loader/lib/index.js!./myComponent.pcss");
			if(typeof newContent === 'string') newContent = [[module.id, newContent, '']];
			update(newContent);
		});
	}
	// When the module is disposed, remove the <style> tags
	module.hot.dispose(function() { update(); });
}

And this block is useless in prod because it will never be run with if (false) while HMR is off. But the problem is that the "!!../node_modules/some-npm-registry/postcss-loader/2.0.6/node_modules/postcss-loader/lib/index.js!./myComponent.pcss" is always being output in the dist file and breaks other bundling tools such as JSPM/SystemJS doing a static verification on the file path, the file path is only for local development and never been exported to dist.

If the current behavior is a bug, please provide the steps to reproduce.

The above HMR code block is always being generated in the final output.

What is the expected behavior?
The style-loader should introduce an option to not output this block if this flag is false, so that we could pass in the environmental flags. I would like to send a pull request for this and just wanted to make sure there's no side effect to just avoid outputting this block for production distribution when HMR is not needed.

If this is a feature request, what is motivation or use case for changing the behavior?

Please mention other relevant information such as your webpack version, Node.js version and Operating System.

[alexander-akait]

@drawyan good idea 👍 PR welcome

[drawyan]

PR is here: #264

[onlywei]

@evilebottnawi @drawyan @michael-ciniawsky sorry for resurrecting this issue, but I think that the pull request solution to this is slightly different from the ideal. I am highly confused by this "option" to enable/disable HMR. Having an option is fine, but I think that's a totally different issue from the one this issue is originally reporting.

I look at this as a dev vs dist situation. If I am doing development, HMR is probably pretty useful. If I am a library author and I am using webpack to generate distribution files, HMR is totally useless. Because HMR is totally useless in the distribution scenario, why should any option to enable/disable HMR matter? In the dist file, the entire code block that begins with if (false) { ... } should not exist no matter what options I pass in...

[michael-ciniawsky]

options.hmr = true // {Default} === Add HMR Code + Use HMR
options.hmr = false === Don't add HMR Code + Don't use HMR

@onlywei Could you describe what isn't ideal atm ? This is a simple opt-out for scenarios where the HMR Code/HMR isn't desired/needed. The main style-loader use case nevertheless is for development expecting HMR to 'just' work. Any other use case can be fairly simply achieved by e.g options.hmr = process.env.NODE_ENV === 'developement' ? true : false. webpack itself shouldn't try to be 'smart' here and guess the usecase(s) based on various other config parameters etc

[onlywei]

I agree that in development, HMR should 'just' work.

However, on the flip side, for distribution files the block of dead code that represents HMR should also 'just' not exist. Not only does it increase file size, but it also messes with any other tools that rely on static analysis (such as JSPM/SystemJS). Library authors should not have to specify an option for this code to not exist in their dist file.

In summary, the HMR could should 'just' exist during development, but 'just' not exist for dist files.

[drawyan]

OK, I've created a new PR, which is I think a better solution than the previous. Depending on the invoking npm script lifecycle event:

• npm prepublish/prepublishOnly for prod distribution, default hmr to false
• Others for non-prod, default hmr to true

So that most of the packages will not have this dead code block in their dist file when publishing to npm, and we can safely fix those packages without them changing a line but bumping the dependency lib version of style-loader.

Let me know if you got any questions, thanks.

#270

@onlywei @evilebottnawi @michael-ciniawsky