shellscape / webpack-manifest-plugin

webpack plugin for generating asset manifests
MIT License
1.44k stars 184 forks source link

options.generate breaks multi-compiler output #185

Closed ZebraFlesh closed 4 years ago

ZebraFlesh commented 5 years ago

Using a multi-compiler setup with Webpack 4 to build both ES5 and ES2015+ output, I can do the following:

const seed = { };
const configureManifest =() => {
    return {
        seed,
        generate: (manifestSeed, files) => {
            return files.reduce((manifest, {name, path, isInitial}) => {
                if (isInitial) {
                    console.log('----> initial', name, path)
                }
                return {...manifest, [name]: path};
            }, manifestSeed);
        }
    };
};

module.exports = [{
  // other config omitted
  plugins: [
    // other plugins omitted
    new ManifestPlugin(configureManfiest()
  ]
}, {
  // other config omitted
  plugins: [
    // other plugins omitted
    new ManifestPlugin(configureManfiest()
  ]
}];

This outputs the console.log for 2 initial chunks, however, the manifest only contains the entries from the second compiler. If I comment out the generate option, I get a full manifest that spans both compiler runs. It's almost as if the generate option is not honoring the seed option?

ZebraFlesh commented 5 years ago

Just dug in a little more and changing the generate function from

(manifestSeed, files) => {
    return files.reduce((manifest, {name, path, isInitial}) => {
        if (isInitial) {
            console.log('----> initial', name, path)
        }
        return {...manifest, [name]: path};
    }, manifestSeed);
}

to

(manifestSeed, files) => {
    return files.reduce((manifest, {name, path, isInitial}) => {
        if (isInitial) {
            console.log('----> initial', name, path)
        }
        manifest[name] = path;
        return manifest;
    }, manifestSeed);
}

fixes the problem. However, I'm not sure why removing the spread operator would have this effect. (Worth mentioning: platform is node 10.15.1 on macOS Mojave.)

So maybe the issue is with the description of the default generate value in the readme? I was inspired to try the object[attribute] form after looking at this plugin's source code.

mastilver commented 5 years ago

Yep, looks like we have a bug

Here: https://github.com/danethurber/webpack-manifest-plugin/blob/6e736d40cdcc4dc9e0098ce6615b23407081b105/lib/plugin.js#L178-L181 manifest and seed end up as the same object, which is not correct

Using the spread operator, you create a new object which expose the issue

mastilver commented 5 years ago

Ignore previous comment

It's not really a bug, but we should warn in case the seed is edited, I think webpack now support warning as part of it stats object

It would be ideal to add a special code to that warning go users can easily look it up and find example on what is the problem and the solution as the warning message might not me enough


if emitCount > 1 and seed !== this.opts.seed then warn

shellscape commented 4 years ago

I took a stab at creating a reproduction for this but was unable to match the conditions necessary to duplicate the problem. I'm up for helping to resolve this one for you, but we'll need a reproduction. Please provide a reproduction by choosing one of the options below:

  1. Using the REPL.it plugin reproduction template at https://repl.it/@shellscape/manifest-plugin-repro
  2. Provide a minimal repository link (Read https://git.io/fNzHA for instructions). Please use NPM for installing dependencies! These may take more time to triage than the other options.

    ⚠️ ZIP Files are unsafe and maintainers will NOT download them.

Until then, we'll close this for housekeeping. If you have the time to add a reproduction to this issue, we'll reopen it post-haste.