Feature: Disable Extracting Specified CSS Module from Chunks

[lsycxyj]

This PR contains a:

• bugfix
• new feature
• code refactor
• test update
• typo fix
• metadata update

Motivation / Use-Case

You may disable extracting css modules programmatically by passing a function.

const miniCssExtractPlugin = new MiniCssExtractPlugin({
  disableExtract({ module, isAsync }) {
    // Do whatever you want. Disable by content size for example:
    // return module.content.length < 10 * 1024;
    // Or disable extracting from all async chunks
    return isAsync;
  },
});

Breaking Changes

None

Additional Info

I found #348 doesn't work at all by now and it's not flexible enough, so I made another pull request based on recent master branch.

[lsycxyj]

Well... the lint problems may be a bit difficult to be fixed...

[alexander-akait]

What is use case? Why you need disable async css?, also i think we should solve this in other case, provide developers function shouldBeExtracted(options) { if (something) { return false } return true; }, it is allows solve many cases, not only async, fox example you can keep small css inside js and many other cases

[lsycxyj]

@evilebottnawi See this comment. I don't need to make extra an request to fetch the css file of its own async js chunk which it has been loaded.

[alexander-akait]

@lsycxyj please read my comment above, we can implement general solution instead only for one case

[lsycxyj]

@evilebottnawi For general solution, I think a "shouldBeExtracted" function for all modules would be much better. I can decide whether a css module depending on its information: whether it is an async module, content, file size, and more.

[alexander-akait]

@lsycxyj yes, feel free to update PR and implement this option 👍

[lsycxyj]

@evilebottnawi I managed it. Now disableExtract option has been implemented.

[alexander-akait]

Here a lot of breaking changes, regressions and very bad code, idea is good, but implementation very bad

[alexander-akait]

Look here https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/src/loader.js#L101, just skip module for keeping module in js

[lsycxyj]

@evilebottnawi Do you mean like my latest commit? (37ce619). I made less code difference.
I'm afraid I don't quite understand how to skip module in https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/src/loader.js#L101

[lsycxyj]

@evilebottnawi Friendly ping

[lsycxyj]

Hi, @evilebottnawi, Is there anything else need to be improved? I suppose there can't be less code changes to implement this feature.

[alexander-akait]

In todo, a lot of issues, you can use own branch if need this asap

[lsycxyj]

Merge from latest master, fix bug and reopen

[lsycxyj]

Known issue: some variables will become "void 0" in some modules after enabling disableExtract in complicated cases, and I don't have any good idea that I can rebuild the modules correctly. Could you do me a favor, please? @evilebottnawi

[codecov]

Codecov Report

Merging #432 (40af212) into master (b146549) will increase coverage by 1.13%.
The diff coverage is 97.91%.

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
+ Coverage   88.55%   89.68%   +1.13%     
==========================================
  Files           5        5              
  Lines         428      475      +47     
  Branches       96      104       +8     
==========================================
+ Hits          379      426      +47     
  Misses         47       47              
  Partials        2        2              

Impacted Files	Coverage Δ
src/index.js	89.49% <97.67%> (+1.64%)	⬆️
src/loader.js	90.90% <100.00%> (+1.43%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b146549...40af212. Read the comment docs.

[sokra]

The functionally seem to be based on rebuilding modules. That's really problematic as it leads to inconsistencies and performance problems. Rebuilding modules should be avoided.

There is already the option to apply mini-css only to some modules via the rule system. I don't think this PR is needed anymore when the isAsync feature is removed.

[sokra]

Name it in positive way e.g. should extract

[lsycxyj]

The functionally seem to be based on rebuilding modules. That's really problematic as it leads to inconsistencies and performance problems. Rebuilding modules should be avoided.

There is already the option to apply mini-css only to some modules via the rule system. I don't think this PR is needed anymore when the isAsync feature is removed.

@sokra Thank you for your review.

Actually, this feature is originally named "disableAsync". The main purpose of the feature is to find out async modules and disable the extraction programactically.

Then, @evilebottnawi gave me advice that to do it with a function like shouldBeExtracted, and it became "disableExtract".

I do understand it leads to some problems. However, I can't figure out any better way to implement the feature by now. I'm still looking for an alternative way.

[alexander-akait]

There is already the option to apply mini-css only to some modules via the rule system. I don't think this PR is needed anymore when the isAsync feature is removed.

Why this is unacceptable?