Deleted chunk are kept in manifest.json in watch mode

[egeriis]

Hey. This is a peculiar little issue I've found, that I'd like to help resolve.

What I am experiencing is that if I remove a chunk from my webpack build, this chunk is still references in manifest.json. It is even more visible if renaming said chunk, which will cause both chunks to show up in manifest.json.

E.g. here I am using webpackChunkName and have a chunk called "browse":

const Browse = Loadable({
  loading: StaticTabbedLoadingShell,
  loader: () => import(/* webpackChunkName: "browse" */ '../Browse')
});

This will correctly show up in manifest.json:

"browse.js": "js/bundle_mobile_browse.no_css.js"

While webpack is running in watch mode, I change the chunk name in the comment annotation to be "browse-wat". This will lead to two entries in manifest.json:

"browse.js": "js/bundle_mobile_browse.no_css.js"
"browse-wat.js": "js/bundle_mobile_browse-wat.no_css.js"

Where the latter will be in the bottom of the manifest.json file.

I have attempted to debug this, some findings are that both reduce and filter callbacks are only called with the correct chunk, i.e. the deleted or renamed chunk does not appear here.

[mastilver]

I hate to say that but this is a feature not a bug...

You will need to use your own generate https://github.com/danethurber/webpack-manifest-plugin#optionsgenerate, that use an empty object rather than the seed

[mastilver]

Hum, actually there is a solution, move the seed variable in the emit function.

Do you want to send a PR?

[egeriis]

When you say it's a feature, and not a bug, is there something I need to keep in mind when submitting a PR?

[mastilver]

just make sure that the seed is still shareable between different instances. It's already covered by tests, so you should be good :)

[egeriis]

Had a look now. Honestly not sure I understand :/ How does seed relate to this "caching"? As far as I can tell it's only used here in emit, but simply used as default carry for reduce. But if seed is an empty object by default?

https://github.com/danethurber/webpack-manifest-plugin/blob/master/lib/plugin.js#L148-L156

Hope you don't mind elaborating.

[mastilver]

seed is an empty object by default

Yes, but only on first run. On watch mode, seed will be populated with previous builds references Whereas, if you move the seed assignment inside the emit function, it will be a new one on each emit

[mastilver]

Wrong button :)

[egeriis]

Ah, of course. Because that object is changed on L153. Gotcha.

[egeriis]

It's been a while, but I had a chance to return to this today. The changes I've attempted have had some unexpected tests failing.

My theory here is that some features worked by accident. This comes from a perspective that the change I've made is very minimal and only affect that the options.seed object is not mutated, which I don't think it should.

But hopefully you can shed some light on this, if you don't mind, @mastilver?

Diff:

diff --git a/lib/plugin.js b/lib/plugin.js
index 3c77e73..42eebec 100644
--- a/lib/plugin.js
+++ b/lib/plugin.js
@@ -35,3 +35,2 @@ ManifestPlugin.prototype.getFileType = function(str) {
 ManifestPlugin.prototype.apply = function(compiler) {
-  var seed = this.opts.seed || {};
   var moduleAssets = {};
@@ -46,2 +45,3 @@ ManifestPlugin.prototype.apply = function(compiler) {
   var emit = function(compilation, compileCallback) {
+    var seed = this.opts.seed ? _.clone(this.opts.seed) : {};
     var publicPath = compilation.options.output.publicPath;
@@ -187,3 +187,3 @@ ManifestPlugin.prototype.apply = function(compiler) {
         });
-  
+
         compilation.applyPluginsAsync('webpack-manifest-plugin-after-emit', manifest, compileCallback);
@@ -206,2 +206,2 @@ ManifestPlugin.prototype.apply = function(compiler) {

-module.exports = ManifestPlugin;
\ No newline at end of file
+module.exports = ManifestPlugin;

Test errors after applying this patch

1) ManifestPlugin using real fs multiple compilation should not produce mangle output
  Message:
    Expected object to have properties
        main-0.js: 'main-0.js'
        main-1.js: 'main-1.js'
        main-2.js: 'main-2.js'
        main-3.js: 'main-3.js'
        main-4.js: 'main-4.js'
[...snip]
        main-998.js: 'main-998.js'

2) ManifestPlugin basic behavior combines manifests of multiple compilations
  Message:
    Expected object to have properties
        one.js: 'one.js'

[mastilver]

Can you send a PR

And the reason it's failing, is because you are cloning seed instead of letting it being mutated (it's a feature we want to keep)

[egeriis]

Sure can. Will get to it within the next few days.

Can you elaborate why you would want to mutate an outside object? I may be wrong, but I feel like it's not the right approach. I also think this is what is causing this bug?

[mastilver]

I'm working on it

We want to be able to mutate it so it's usable for with multiple compilation (when export an array of config)