shellscape / webpack-manifest-plugin

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

Allow the basePath and publicPath to both be used #37

Closed weaverryan closed 7 years ago

weaverryan commented 7 years ago

Hi there!

We're building a library where we need to control the manifest key prefix and value prefix independently. The use-case is when publicPath is a CDN (i.e. in production), but you still want to keep the same keys, which include some other prefix (e.g. /build):

{
    "build/one.js": "https://coolcdn.com/one.js",
    "build/two.js": "https://coolcdn.com/two.js"
}

This PR allows you to use basePath and publicPath at the same time. There is no BC break or behavior change for existing users. Without this, we need to maintain a copy of this library, which is lame :).

Thanks!

weaverryan commented 7 years ago

Ping! Is this lib still maintained! It's pretty small, but I noticed not a lot of activity recently.

Thanks!

mastilver commented 7 years ago

Hi @weaverryan

I'm not sure I understand your use case, can't you use a dynamic publicPath I don't think I'm going to merge that in

weaverryan commented 7 years ago

Hey @mastilver!

Yes, I can explain further!

1) I'd like to be able to prefix the manifest key. So, I want to use the basePath option - 2) I also want to prefix the manifest value, but with a different value, as I'm using a CDN. So, I want to use the publicPath option.

The problem is simply that when basePath is set, the publicPath option is ignored: you can't prefix the manifest key and value at the same time. I'd like the manifest.json to look like this:

{
    "build/one.js": "https://coolcdn.com/one.js",
    "build/two.js": "https://coolcdn.com/two.js"
}

After using the plugin in this way:

new ManifestPlugin({
  // ...
  basePath: 'build/',
  publicPath: 'https://coolcdn.com',

  // this is the new option, because currently, publicPath is ignored when basePath is set
  useBasePathAndPublicPath: true
})

Does that make sense? Is there another way to achieve this I'm missing? We actually maintain a fork of this plugin due to this for the Webpack Encore library: https://github.com/symfony/webpack-encore/blob/master/lib/webpack/webpack-manifest-plugin.js

Cheers!

mastilver commented 7 years ago

Can't you do: publicPath: `https://coolcdn.com/${true ? 'build/' : ''}`

To be honest I plan to deprecate publicPath and basePath to be more inline with the webpack way of doing it, so I would rather not add new functionality

weaverryan commented 7 years ago

Can't you do: publicPath: https://coolcdn.com/${true ? 'build/' : ''}

Hmm, check out the code as it is now: if you pass publicPath at all to the plugin, then you can't control the manifest key prefix. There's currently not possible way to get the following JSON:

{
    "build/one.js": "https://coolcdn.com/one.js",
    "build/two.js": "https://coolcdn.com/two.js"
}

where build/ is a prefix added to the key and https://coolcdn.com/ is a different prefix added to the value.

To be honest I plan to deprecate publicPath and basePath to be more inline with the webpack way of doing it, so I would rather not add new functionality

Would there be no way to prefix the key or the value (so the manifest would always be a pure "main.js' => "main.abc123.js" mapping)? Does prefixing violate the webpack way of doing things in some way (or do you mean that the plugin would automatically use publicPath from webpack itself, instead of having your re-specify it manually)?

I wonder if this this could all be fixed by adding an event hook where the user could modify the finished mapping object right before it's emitted.

Thanks!

mastilver commented 7 years ago

I wonder if this this could all be fixed by adding an event hook where the user could modify the finished mapping object right before it's emitted.

This is the plan for #55 (NOTE: it's only the first draw, I will add more details later)

weaverryan commented 7 years ago

If #55 was done (specifically map()), that would make the publicPath part completely flexible :).

What about basePath - i.e. the ability to control/change the key (e.g. prefix it)? I don't see a way to control that in #55 (assuming the basePath option were fully removed).

mastilver commented 7 years ago

What about basePath - i.e. the ability to control/change the key (e.g. prefix it)? I don't see a way to control that in #55 (assuming the basePath option were fully removed).

reduce() :)

It's not done yet, I still don't know if it's a good idea, I I would like to have some thought on that (especially if there could be some drawback)

weaverryan commented 7 years ago

Thanks @mastilver :). I'm going to wait to see what develops on #55 then. This PR accomplishes this while protecting BC... but honestly, if we're making a bunch of changes for a 2.0 release, we should just wait and do it properly there.

gmac commented 7 years ago

Closing this in favor of https://github.com/danethurber/webpack-manifest-plugin/pull/61