Replace clean-webpack-plugin with the core output cleaning feature#1313
Replace clean-webpack-plugin with the core output cleaning feature#1313stof merged 1 commit intosymfony:mainfrom
Conversation
|
The ESLint job requires #1311 to pass, because the deprecated |
package.json
Outdated
| "css-minimizer-webpack-plugin": "^5.0.0", | ||
| "fastest-levenshtein": "^1.0.16", | ||
| "mini-css-extract-plugin": "^2.6.0", | ||
| "multimatch": "^5", |
There was a problem hiding this comment.
Can we use picomatch instead?
It has 0 dependencies (where multimatch has 7 dependencies), and weight much less, see graphs:
- picomatch: https://npmgraph.js.org/?q=picomatch#select=exact%3Apicomatch%404.0.2
- mulitmatch: https://npmgraph.js.org/?q=multimatch%406.0.0
picomatch is also compatible with CJS modules.
Thanks!
There was a problem hiding this comment.
no we cannot (easily). The feature I'm using is precisely the ability to pass multiple patterns including negative patterns (due to the way our cleanupOutputBeforeBuild API looks like), which is the main point of multimatch.
If we decide to break compatibility a lot more by removing support for the first argument of our current cleanupOutputBeforeBuild, I could do it without any dependency.
There was a problem hiding this comment.
and picomatch might switch to ESM in the future as well (there is a PR opened about that). So just using picomatch won't let us use CJS forever (my expectation is that more and more packages will migrate to ESM)
There was a problem hiding this comment.
note that this still reduces our dependencies compared to clean-webpack-plugin
There was a problem hiding this comment.
What about breaking the cleaning feature and let the user decide what it wants to keep (through output.clean.keep if I remember)?
We could directly pass its configuration to Webpack, it will be easier for us, no need for extra work nor dependencies.
WDYT ?
There was a problem hiding this comment.
We still need the config to keep the manifest.json for the dev-server (unless the issue mentioned in the code comment is not relevant anymore).
If we can remove that part, I'm fine with doing a bigger BC break dropping the first argument (and letting users configure keep through the callback if needed).
There was a problem hiding this comment.
However, as I'm not using the dev-server myself (I'm using the watch mode), I don't have a setup to test this.
There was a problem hiding this comment.
Good news, the option output.clean.keep() seems totally ignored in dev-server:
- Just to be sure, I've removed by
public/buildfolder, - Commented
cleanedPaths.push('!manifest.json') - Ran the dev-server
And file public/build/manifest.json is present, with good URLs, HMR works too, etc...
So we can remove cleanedPaths.push('!manifest.json') and go for a bigger BC :)
There was a problem hiding this comment.
I updated the PR with the simpler implementation.
2fed871 to
34f8e0b
Compare
|
Thanks for your modifications! I've tried your branch locally and everything worked fine, as expected I had to update We can merge after a rebase IMO PS: in the next week, I will try to add E2E tests for the dev-server, to check if it correctly boots, if page is updated after a modification, etc... that will ease this kind of changes. |
Closes #1289
The options configurable by the configurator callback passed as second argument of
cleanupOutputBeforeBuildare not the same anymore (thedryoption used in the example still exists)