Upgrade to css-minimize-webpack-plugin 2.0

[stof]

This upgrades cssnano to version 5, which fixes a bunch of bugs and uses postcss 8 (avoiding to install multiple versions of postcss).
The BC breaks in the plugin are dropping support for webpack 4 (not impacted Encore as we already dropped it), removing some options which were only for webpack 4 and the fact that cssnano 5 requires node 10.13+ (not impacting Encore as we already require 10.19+)

Closes #965

[weaverryan]

Thanks for this Stof!

I think we need to also remove our sourceMap option here -

webpack-encore/lib/plugins/optimize-css-assets.js

Line 22 in 8c2752a

sourceMap: webpackConfig.useSourceMaps

- as it looks like this option is was removed.

This would be the first time (since our 1.0) that we would upgrade dependencies over a major version. That's a gray area in our BC policy. The changes, in this case, are not for any options, etc that are used (or advertised to be used, e.g. in index.js documentation) by Encore. Therefore, my opinion is that it does not require a major a bump (and I think you agree).

[stof]

For fixing the tests, I went the lazy way for finding the version of webpack that changed the naming of chunks, and I made 5.35 (the current version) the min version. I know that 5.25 is still using the old naming of the split chunk.

Due to the way yarn and npm work, supporting a wide range of versions for dependencies is not useful, as yarn and npm will use the latest version anyway (happily installing multiple versions of the package in different parts of the tree if needed). The place where supporting wider ranges is useful is for peerDependencies.

[stof]

ok, apparently, it is not webpack itself changing the naming of chunks, but probably a plugin...

[stof]

@weaverryan how do you generally fix this issue about the generated chunk ids being different for the lowest deps job ?

[weaverryan]

@weaverryan how do you generally fix this issue about the generated chunk ids being different for the lowest deps job ?

I have been, on a case-by-case basis, making our test suite able to be "smarter" by matching in a fuzzy way. For example:

webpack-encore/test/functional.js

Line 444 in d074101

'main.[hash:8].js',

Outside of this PR, we just need to find the new problems and make them fuzzy like this :)

[weaverryan]

Was this related to the babel-loader bump? V 8.2.2 causes an extra module to be in here?

[stof]

webpack itself. They refactored their runtime in the 5.25 release

[stof]

@weaverryan the issue is the test about splitting entry points in production mode. there, the file names are integers generated by webpack, and they depend on a bunch of things

[weaverryan]

@weaverryan the issue is the test about splitting entry points in production mode. there, the file names are integers generated by webpack, and they depend on a bunch of things

👍 So, for reference, this line causes problems:

js: ['/build/runtime.js', '/build/961.js', '/build/38.js', '/build/main.js'],

Matching the 2 integers in a "fuzzy" way would be a little odd... just because they could be anything. The test would be reduced to:

• Check that there are 4 .js files
• And that 2 of them are runtime.js and main.js... but the other 2 could be anything

It makes me think it would be easier just to rewrite the test. Instead of asserting the entire entrypoints.json contents, we would check specifically for the two items above (check for 4 files, and that two of them are runtime.js and main.js).

For the webpackAssert.assertManifestKeyExists('build/843.js'); check, probably we could loop through all of the main keys in the entrypoints.json file, and assert that they exist (without the opening slash) in manifest.json.

[stof]

@weaverryan any chance that you do the refactoring of this test, as you know better than me the architecture of the testsuite ?

[weaverryan]

No problem - I was thinking already about doing that - I pushed a commit that should hopefully do the trick :)

[stof]

Looks like the enableIntegrityHashes() now breaks in the highest deps job (which was not the case last week).

[weaverryan]

I noticed that too :/ - I'll take a look at it in a separate PR - the cause is not immediately obvious

[weaverryan]

Thanks Stof!

[Kocal]

Thanks! :)