webpack-contrib / uglifyjs-webpack-plugin

[deprecated] UglifyJS Plugin
MIT License
1.38k stars 179 forks source link

feat(options): add chunkFilter option #369

Closed WearyMonkey closed 5 years ago

WearyMonkey commented 6 years ago

Adds a chunkTest option to filter chunks.

This PR contains a:

Motivation / Use-Case

The existing test/include/exclude options are not useful when the filenames are generated at buildtime, e.g.

// in your webpack.config.js
entry: {
   myEntry: 'src/foo.js',
},
output: {
   filename: '[hash].[ext]',
},
minimizer: [
  new UglifyJsPlugin({
    include: // ?? no way to match hash here.
    chunkTest: chunk => chunk.name === 'myEntry',
  }),
]

Breaking Changes

Additional Info

Related to: https://github.com/webpack-contrib/uglifyjs-webpack-plugin/issues/21

jsf-clabot commented 6 years ago

CLA assistant check
All committers have signed the CLA.

codecov[bot] commented 6 years ago

Codecov Report

Merging #369 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #369      +/-   ##
=========================================
+ Coverage   97.69%   97.7%   +0.01%     
=========================================
  Files           5       5              
  Lines         260     262       +2     
  Branches      102     103       +1     
=========================================
+ Hits          254     256       +2     
  Misses          6       6
Impacted Files Coverage Δ
src/index.js 96.45% <100%> (+0.05%) :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 bf36e21...013ffd4. Read the comment docs.

alexander-akait commented 6 years ago

/cc @michael-ciniawsky what do you think?

WearyMonkey commented 6 years ago

@evilebottnawi renamed to chunkFilter

also it should be before .filter(ModuleFilenameHelpers.matchObject.bind(null, this.options)), not first

Didn't understand this sorry. Can you provide a code snippet? I thought the chunk filter should go before the chunk file accumulator.

WearyMonkey commented 6 years ago

@evilebottnawi @michael-ciniawsky any thoughts?

alexander-akait commented 6 years ago

@WearyMonkey Please read my review above and fix problem

WearyMonkey commented 6 years ago

@evilebottnawi I'm only seeing:

Better name for option/function chunkFilter

I renamed chunkTest to chunkFilter as you suggested.

also it should be before .filter(ModuleFilenameHelpers.matchObject.bind(null, this.options)), not first

Didn't understand this sorry. Can you provide a code snippet? I thought the chunk filter should go before the chunk file accumulator.

Am I missing something else? I think you commented during the github downtime so maybe something was lost.

WearyMonkey commented 6 years ago

@evilebottnawi as far as I can tell I've responded to all your feedback. Am I missing something?

alexander-akait commented 6 years ago

Need fix test on windows

alexander-akait commented 6 years ago

/cc @WearyMonkey can you update snapshots, looks something wrong with cache on appveyor side

WearyMonkey commented 6 years ago

@evilebottnawi I've updated the snapshots on my mac, and tests are passing. Do I need to update them on Windows as well?

alexander-akait commented 6 years ago

@WearyMonkey will be great :+1:

WearyMonkey commented 6 years ago

@evilebottnawi I ran npm test -- -u on windows, and seems to of touched all the snapshots, and the CI is still failing.

Not sure where to go from here?

alexander-akait commented 5 years ago

Close in favor https://github.com/webpack-contrib/uglifyjs-webpack-plugin/pull/382, anyway thanks for the PR and helping to use do webpack better