shellscape / webpack-manifest-plugin

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

Add subfolder support #18

Closed orbiteleven closed 7 years ago

orbiteleven commented 8 years ago

I export my files into subfolders:

...
output: {
    filename: 'javascripts/main-[hash].js',
  },
...

however this (great!) plugin was not picking up on that for main.js (and main.css), though it would for assets:

...
"images/foo.png": "images/foo-79a318e1a4e1bc0aac94b6ba2bbb3b2a.png",
"main.js": "javascripts/main-2ce5bbf6dc7dbe06a0e9.js",
"main.css": "stylesheets/main-2ce5bbf6dc7dbe06a0e9.css"
...

This diff will prepend any subfolders to main.js/main.css:

...
"images/foo.png": "images/foo-79a318e1a4e1bc0aac94b6ba2bbb3b2a.png",
"javascripts/main.js": "javascripts/main-2ce5bbf6dc7dbe06a0e9.js",
"javascripts/main.css": "stylesheets/main-2ce5bbf6dc7dbe06a0e9.css"
...

Not as familiar with the webpack plugin system, so if there's a "smarter" way to do this, by all means let me know :)

pgilad commented 8 years ago

Great addition, but this should be added with a flag to be backwards compat.

pgilad commented 8 years ago

@danethurber is it possible to advance this somehow?

mblarsen commented 7 years ago

@danethurber cool, could you add the flag/option so it can me merged?

lukeed commented 7 years ago

Doesn't look like there's much activity here, but this should 💯 be included.

mastilver commented 7 years ago

I agree this PR make sense, though I think it's a breaking change

Can you rebase that on top of master, please?

mastilver commented 7 years ago

This is already possible by doing:

{
        entry: {
          'javascripts/main': path.join(__dirname, './fixtures/file.js')
        },
        output: {
          filename: '[name].js'
        }
      }

Let me know if I missed something here! :)

lukeed commented 7 years ago

You've missed something 😄 The entry filename is supposed to be a file name only. It affects the rest of the build cycle & will likely break other plugins.

mastilver commented 7 years ago

@lukeed Cool 👍

Now is #66 enough for that or should we add another options like https://github.com/danethurber/webpack-manifest-plugin/issues/23#issuecomment-275038728 suggested ?

I'm not a big fan on adding yet another option, while #66 will allow:

map: (file) => ({
    ...file,
    name: path.dirname(file.path) + file.name
})
lukeed commented 7 years ago

Not sure tbh. I've been tuned out of this for quite some time. All this time I've been using my fork and it's done exactly what I need -- and what's described in this PR.

orbiteleven commented 7 years ago

Yeah... This is an over-year-old PR. I've moved on to other solutions and not really following the work on this project. Is this still needed?

lukeed commented 7 years ago

Last I checked was 3 to 4 months ago. Was certainly still needed then. I'm in same boat -- my own workaround has been great.

orbiteleven commented 7 years ago

Original diff was 14 April 2016; renewed interest in March, but I'm not sure if #66 or #23 solve this in different ways. Again, not using the project anymore. I'm happy to update the diff, but it'd take some "dusting off" on my part

mastilver commented 7 years ago

@orbiteleven I added your test on #66: https://github.com/danethurber/webpack-manifest-plugin/pull/66/files#diff-4170f24227285478db88bb820df9a581R446

mastilver commented 7 years ago

My issue with adding this PR in is that everybody got their own needs and we can't take care of all of them. This is why the hooks provided by #66 are to me the solution for that

mastilver commented 7 years ago

Thank you @orbiteleven but I'm closing this as it's now possible using the map option: https://github.com/danethurber/webpack-manifest-plugin/blob/43b48f1eab84d718439383fa52a683c79e237fab/spec/plugin.spec.js#L524-L545

orbiteleven commented 7 years ago

Works for me!