fix: allow consumers to access CssModule

[barak007]

This PR contains a:

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

Motivation / Use-Case

Hey,
This PR fixes a subtle breaking change happened in the last release.
I had an integration that was injecting CssModule instances to the compilation, and it assumed this plugin will take care of them.
After the latest release it was impossible for me to get the CssModule class since it cannot be registered twice for serialization.
This PR allows others to get access to the CssModule class within same webpack running version.

Breaking Changes

none

Additional Info

[codecov]

Codecov Report

Merging #703 (47b836f) into master (1be21d2) will increase coverage by 0.17%.
The diff coverage is 97.56%.

@@            Coverage Diff             @@
##           master     #703      +/-   ##
==========================================
+ Coverage   89.58%   89.75%   +0.17%     
==========================================
  Files           6        6              
  Lines         768      771       +3     
  Branches      232      235       +3     
==========================================
+ Hits          688      692       +4     
+ Misses         77       76       -1     
  Partials        3        3              

Impacted Files	Coverage Δ
src/loader.js	92.59% <85.71%> (+0.17%)	⬆️
src/index.js	87.87% <100.00%> (+0.29%)	⬆️
src/utils.js	91.89% <100.00%> (+0.46%)	⬆️

---

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 1be21d2...a6d9608. Read the comment docs.

[alexander-akait]

We need small test for this to avoid this problem in future + comments why we need to do it

[alexander-akait]

I think we need do the same for CssDependency, so we need to remove shared and use the same approach

[barak007]

I'm not sure what's going on with the lint. maybe related to new lines?

[alexander-akait]

Yep, need to run prettier on these files

[alexander-akait]

I think we don't need shared util anymore, just use plugin.getCssDependency(webpack) in loader

[barak007]

true. I'm not sure about this error now it cannot happen.

if (!CssDependency) {
  throw new Error(
    "You forgot to add 'mini-css-extract-plugin' plugin (i.e. `{ plugins: [new MiniCssExtractPlugin()] }`), please read https://github.com/webpack-contrib/mini-css-extract-plugin#getting-started"
  );
}

[alexander-akait]

true. I'm not sure about this error now it cannot happen. this happens only when you forget to add plugin

[barak007]

also something with prettier is not working for me.

[barak007]

But now it will always generate the CSSDependency class. I can add validation that it already requested once if needed

[alexander-akait]

Run npm run lint:prettier -- --fix

[alexander-akait]

Yep, I think it make sense

[barak007]

first prettier is failed to execute for some unknown reason. (I can share the log if needed)
second I'm thinking that this error is an unwanted detail because now an integration can possibly create the class unrelated to the fact that someone forgot to config the plugin. maybe this error should be based on loader context provided from the plugin.

[alexander-akait]

second I'm thinking that this error is an unwanted detail because now an integration can possibly create the class unrelated to the fact that someone forgot to config the plugin. maybe this error should be based on loader context provided from the plugin.

Agree, we can use Symbol in apply for this

[barak007]

Sine I'm kind of working on blind. I will need to double check this. I will try to clone it later with correct NL config and try to run all the lints and tests locally.

[barak007]

The lint error is just for the commit messages it can be solved with squash. The other tests I'm not sure about. It seems that it's also broken on master.

[barak007]

the failing test looks to me like pure flakiness. from my perspective this PR is ready.

[alexander-akait]

Thanks

[pieh]

Is this correct change?

Shouldn't here compiler be used as cache key and not webpack package/module? In case of multiple compilers created from same webpack package, now one compiler has impact on another 🤔

I did hit issue of getting those recent release:

You forgot to add 'mini-css-extract-plugin' plugin (i.e. { plugins: [new MiniCssExtractPlugin()] }), please read https://github.com/webpack-contrib/mini-css-extract-plugin#getting-started

Tho this might be wrong configuration for SSR bundle compilation where I don't want to extract css anymore (because that happen in browser bundle compilation), but I do still have mini-css-extract-plugin loader in front of css-loader for css-modules (tho css-loader has exportOnlyLocals option, so it doesn't do actual css processsing other than extracting class names).

[alexander-akait]

This loader doesn't have sense without the plugin

[alexander-akait]

Using extra loader on top reduce your bundling perf, so I recommend to remove it