shellscape / webpack-manifest-plugin

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

Nameless chunks don't throw errors #17

Closed gmac closed 8 years ago

gmac commented 8 years ago

Just updated to the latest version of Webpack Manifest Plugin and noticed what looks to be a widely reported bug. Nameless chunks cause errors... see: https://github.com/danethurber/webpack-manifest-plugin/issues/16.

Nameless chunks are actually fairly common in Webpack, because they're created by the native code splitting feature require.ensure(...). Code splits will produce new chunks formatted as:

{
  name: null,
  files: [ '4.4.9cfa2b27bb561f8c2bbd.js' ]
}

Now, these nameless code split chunks are automatically catalogued within the Webpack runtime so that it knows how to fetch and reconstitute the chunk composition. These nameless units are not entry chunks nor initial chunks, so there's not much value in being able to load them independently via a manifest. Therefore, this update simply omits nameless chunks from the manifest and leaves them as wards of the Webpack runtime.

jordaaash commented 8 years ago

While I like the philosophical approach of treating nameless chunks as a hidden implementation detail, there are reasons it would be valuable to list them as keys in the manifest still. For example, I use the manifest on the server side to look up files for serving in memory (avoiding a check for the file's existence, which is relevant when using Android's virtualized filesystem for bundled assets). I'm of the belief that the manifest should report all files output by Webpack, even if they aren't entries or initial chunks. What about listing them by chunk ID, e.g.

"4.4": "4.4.9cfa2b27bb561f8c2bbd.js"

or just filename, if we don't want to infer or care about the chunk ID:

"4.4.9cfa2b27bb561f8c2bbd": "4.4.9cfa2b27bb561f8c2bbd.js"

I realize this is duplicative and not very useful if one is just looking for entry chunks, but I think it's preferable for the manifest to be complete than opinionated in that way. What do you think?

danethurber commented 8 years ago

sorry i've been busy and just getting time to address this.

@gmac thanks for a quick solution

@jordansexton are you suggesting something along these lines to get the manifest key if there is no chunk.name

var chunkName = chunk.name
  ? chunk.name.replace(this.opts.stripSrc, '')
  : chunk.id.toString();

proposal -> https://github.com/danethurber/webpack-manifest-plugin/commit/70c2b47739cd04e2efc8b021aef2cfabc3646765

gmac commented 8 years ago

For something like that, it would probably be more reliable just to look through the chunk files. Nameless chunks should still have all generated file information there, at which time a literal mapping of the webpack file name to itself could be established.

On Saturday, April 16, 2016, Dane Thurber notifications@github.com wrote:

sorry i've been busy and just getting time to address this.

@gmac https://github.com/gmac thanks for a quick solution

@jordansexton https://github.com/jordansexton are you suggesting something along these lines to get the manifest key if there is no chunk.name

var chunkName = chunk.name ? chunk.name.replace(this.opts.stripSrc, '') : chunk.id.toString();

proposal -> cfb3fd8 https://github.com/danethurber/webpack-manifest-plugin/commit/cfb3fd80ab88162927b8aebdcfb53fa307536518

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/danethurber/webpack-manifest-plugin/pull/17#issuecomment-210845462

gmac commented 8 years ago

Okay @jordansexton, makes sense to me. Having the manifest document all files –known and unknown– is a good idea. For known files, manifest gives us a mapping of what we called a file relative to what Webpack called it. For unknown files, manifest gives us a record of all files we never knew about that Webpack created and named for us. I've updated the PR to make nameless chunks output a literal mapping of their child files, such as:

{ "1.1.js": "1.1.js" }

Also adds a new test and normalizes some syntax inconsistencies.

jordaaash commented 8 years ago

Nice! Thanks for the thoughtful fix to the issue I (rather carelessly) introduced. 😅