shellscape / webpack-manifest-plugin

webpack plugin for generating asset manifests

MIT License

1.44k stars 184 forks source link

Deprecate cache parameter in favour of seed #57

Closed nathanbirrell closed 7 years ago

nathanbirrell commented 7 years ago

Functionality for cache stays the same, the more generic name is better suited for when your app needs to set custom keys.

requested in #55

closes #25

codecov-io commented 7 years ago

Codecov Report

Merging #57 into master will increase coverage by 3.69%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
+ Coverage   89.28%   92.98%   +3.69%     
==========================================
  Files           2        2              
  Lines          56       57       +1     
==========================================
+ Hits           50       53       +3     
+ Misses          6        4       -2

Impacted Files	Coverage Δ
lib/plugin.js	`92.85% <100%> (+3.76%)`	: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 a985131...357f3d7. Read the comment docs.

mastilver commented 7 years ago

@a-x- @zuzusik @gmac Does initValue make sense to you (I had no idea how to name it so I just looked at the doc for Array.reduce :D )

nathanbirrell commented 7 years ago

Hey @gmac, I agree.

I've updated the PR, unit tests and readme to reflect the move to seed (mentioned here).

It will still accept cache for now.

Cheers

mastilver commented 7 years ago

@gmac are you ok with those changes? Or shall we output a deprecation warning using util.deprecate?

zuzusik commented 7 years ago

Thinking about further improvements here: is there a way to detect that we are in multi-compiler mode inside of a plugin and then switch to seeding mode?

This would be really great and convenient - no need to specify seed as {} - it will just work out of the box.

gmac commented 7 years ago

Sure, it's possible to detect multi-compiler mode... you just look at the number of compilers configured. Off the top of my head, I think that's managed by an array called compilers on the root Webpack compiler object.

This sounds like a potentially useful feature, although beyond the scope of this PR. There's definitely some nuances there that would need to be solved for. The way I'd see it working would be to pass an option like universalManifest: true. When enabled, a plugin instance would configure around an internal singleton and assign any provided seed params onto that. Otherwise, it would configure around the provided seed object itself. The nuance would be in tracking the right object identity.

nathanbirrell commented 7 years ago

Thanks all. Updated the readme with @gmac's suggestion 👍🏼

mastilver commented 7 years ago

👍 I will release that tonight