Skip to content

Support mini-css-extract-plugin #17

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

Closed

Conversation

GoodForOneFare
Copy link
Contributor

What?

This rehashes CssModule objects generated by mini-css-extract-plugin.

Why?

extract-text-webpack-plugin will not support webpack@4, and mini-css-extract-plugin is now the officially supported path for CSS.

@@ -56,26 +56,85 @@ function replaceStringInAsset(asset, source, target) {
*/
function reHashChunk(chunk, assets, hashFn) {
const oldHash = chunk.renderedHash;
const oldChunkName = chunk.files[0];
const fileIndex = chunk.files.findIndex(file => file.endsWith('.js'));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With mini-css active, .js files tend to end up in files[1].

* This function updates the *name* of the main file (i.e. source code), and the *content* of the
* secondary files (i.e source maps)
*/
function reHashCssAsset(chunk, assets, hashFn) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of similar code to reHashChunk in here. Ideally, I'd be able to refactor this in a follow-up PR.

const oldHash = chunk.contentHash[miniCssKey];
const fileIndex = chunk.files.findIndex(asset => asset.match(oldHash));
if (fileIndex === -1) {
throw new Error('Asset file not found for CssModule hash. Please use [contenthash] in MiniCssExtractPlugin placeholders.');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If [contenthash] isn't used, runtime chunks end up with the same chunkhash in CSS / JS asset maps. This makes is impossible to reliably know which hash you're replacing. This could just replace the first instance it finds, and hope for the best ¯\(ツ)

Having problems testing this 😕 Will update this PR if I figure it out.

@scinos
Copy link
Owner

scinos commented May 3, 2018

Update: I've been looking at this PR and I can reproduce the problem. It is caused by my assumption that each chunk has one main file that need to be renamed + hashed, and many secondary files that only need to be rehashed.

I'm working on a fix using @GoodForOneFare solution (basically merge the duplicated code), but before releasing it I would like to test it more with other plugins that might create extra "main" files. Hopefully it will happen in a couple of days.

@tomxhu
Copy link

tomxhu commented May 8, 2018

any updates on this PR? would love to implement this in our repo.

@scinos
Copy link
Owner

scinos commented May 8, 2018

@tomxhu still working on it. I have it working locally, I just need to clean it up a big and push it. Hopefully will happen today.

@scinos
Copy link
Owner

scinos commented May 9, 2018

Merged manually on master.
Thanks @GoodForOneFare !

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