search

johnagan / clean-webpack-plugin

A webpack plugin to remove your build folder(s) before building
MIT License
1.96k stars 136 forks source link

Update webpack to version 5 #195

Closed strootje closed 3 years ago

strootje commented 3 years ago

Updated to webpack version 5. I had to drop support for webpack version 3 and node < 10.

codecov-io commented 3 years ago

Codecov Report

Merging #195 (6d8a232) into master (021d2b6) will increase coverage by 1.13%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #195      +/-   ##
===========================================
+ Coverage   98.86%   100.00%   +1.13%     
===========================================
  Files           1         1              
  Lines          88        76      -12     
  Branches       30        27       -3     
===========================================
- Hits           87        76      -11     
+ Misses          1         0       -1
Impacted Files Coverage Ξ”
src/clean-webpack-plugin.ts 100.00% <100.00%> (+1.13%) :arrow_up:

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 64c4241...6d8a232. Read the comment docs.

johnagan commented 3 years ago

wow! great work here and thank you for the contribution πŸ‘ I'm not currently using webpack to validate we're all good here, but the code changes look good. Would be great to get some other eyes on this before bringing it in. cc @chrisblossom

strootje commented 3 years ago

I'm not really sure how to fix the coverage report. I've added some code to make typescript happy (for selecting the related assets) but I'm not really sure how to trigger that which webpack. Tips are appreciated

djcsdy commented 3 years ago 
import { Compiler, Stats, compilation as compilationType } from 'webpack';
declare type Compilation = compilationType.Compilation;

should also be changed to

import { Compiler, Stats, Compilation} from 'webpack';

for compatibility with type declarations provided by webpack 5, see #193.

strootje commented 3 years ago

@djcsdy I think I did that already. Could you specify in which file I've missed it?

djcsdy commented 3 years ago

@strootje My mistake, you're right, it's already done. I must have been looking at the wrong commit.

alpadev commented 3 years ago

The README.md still mentions

NOTE: Node v8+ and webpack v3+ are supported and tested.

Just to mention it, the zero configuration doesn't work anymore with this plugin and v5 (probably the reason why you had to limit that test case to webpack v4). Mentioned the reason for this here: https://github.com/johnagan/clean-webpack-plugin/pull/181#issuecomment-683350264.

alpadev commented 3 years ago

I'm not really sure how to fix the coverage report. I've added some code to make typescript happy (for selecting the related assets) but I'm not really sure how to trigger that which webpack. Tips are appreciated

I think the reason for this is that you removed some code which changed the ratio between tested and untested code. Looks like there is only one line/case that isn't covered by tests see https://codecov.io/gh/johnagan/clean-webpack-plugin/src/b2dccbbcd626a151772332ec1ba567473458a8c2/src/clean-webpack-plugin.ts#L340

You can try changing it to

/* istanbul ignore next */
throw error;

I checked del/rimraf and looks like they handle errors quite well so I'd say it's not that easy to build a test for this.

strootje commented 3 years ago

@chrisblossom and @johnagan any chance for a rereview?

johnagan commented 3 years ago

@strootje nice work! I'll merge it in πŸŽ‰