shellscape / webpack-manifest-plugin

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

Breaks when chunk has multiple files with the same extension #30

Closed kostasmanionis closed 3 years ago

kostasmanionis commented 8 years ago

Hello!

Since this package builds the memo object keys only using the chunk name and the file extension(see here), it breaks when the chunk has multiple files with the same extension. For e.g.

index.css
index.above.css

It outputs something like this

"/fancy/project/dist/index.css": "/fancy/project/dist/index.99b9974b.above.css"

I could send in a PR if we could agree on how to handle these cases.

mastilver commented 7 years ago

hi @kostasmanionis

Can you be clearer on this? I don't get how to reproduce your issue What's your webpack config?

mastilver commented 7 years ago

^ ping @nitruxa @yuya373 @sashalifestar @993318936 @bobaaaaa @harryurban

bobaaaaa commented 7 years ago

Hi. This is our project setup.

Setup

// main.ts
import '../scss/foo.critical.scss';
import '../scss/foo.main.scss';
// webpack.conf.js (simple)
{
  module: {
    rules: [
      {
        test: /\.main\.scss$/,
        use: ExtractTextPluginMainCss
      },
      {
        test: /\.critical\.scss$/,
        use: ExtractTextPluginCriticalCss
      }
    ]
  },
  plugins: [
    new ManifestPlugin()
  ]
}

Expected manifest.json output

{
  "foo.main.css": "foo.main.$hash.css",
  "foo.critical.css": "foo.critical.$hash.css"
}

Actual manifest.json output

{
  "foo.main.css": "foo.main.$hash.css"
}
mastilver commented 7 years ago

I see, I didn't think about the case when you have multiples ExtractTextPlugin

kostasmanionis commented 7 years ago

The config that @bobaaaaa shared illustrates the issue perfectly 👍 I've ended up with using webpack-assets-manifest with the fileExtRegex option. Maybe a similar solution would work here.

bobaaaaa commented 7 years ago

@kostasmanionis thx for the tip. I have a workaround with a different CSS loader chain + webpack-manifest-plugin

mastilver commented 7 years ago

I think fixing #23 will also fix that issue

mastilver commented 7 years ago

At the moment it's possible to use the map option to resolve it: https://github.com/danethurber/webpack-manifest-plugin/blob/43b48f1eab84d718439383fa52a683c79e237fab/spec/plugin.spec.js#L524-L545

I'm keeping it open, but I'm not sure it's that easy to fix...

kostasmanionis commented 7 years ago

@mastilver agree that it's not an easy fix out of the box. Should be good enough that an escape hatch is provided for people who want more control over their manifest.

thanks! :+1:

loheander commented 6 years ago

@mastilver The suggested map function doesn't seem to affect the problem, it just adds the full directory to the output manifest but the extra css files are still not added to the manifest. Just a single line with the first css file.

mastilver commented 6 years ago

@jheander I can't help you without more information, send us your webpack config

loheander commented 6 years ago

Sure! So this is our setup:

// entry.js
import 'style/tzcontrol.less';
import 'style/tzcontrol.scss';
import 'style/tz-appshell.less';
// webpack.conf
const ExtractTextPlugin = require('extract-text-webpack-plugin');
const ManifestPlugin = require('webpack-manifest-plugin');

const extractSass = new ExtractTextPlugin(`[name]-[contenthash]-new.css`);
const extractLess = new ExtractTextPlugin(`[name]-[contenthash].css`);
const extractShell = new ExtractTextPlugin(`[name]-[contenthash]-shell.css`);

config.module.rules = [
    {
        test: /\.scss$/,
        include: [
            path.resolve(__dirname, 'src/style'),
        ],
        loader: extractSass.extract({
            use: ['css-loader', 'sass-loader'],
            fallback: 'style-loader',
            publicPath: './',
        }),
    },
    {
        test: /\.less$/,
        include: [
            path.resolve(__dirname, 'src/style'),
        ],
        exclude: [
            path.resolve(__dirname, 'src/style/tz-appshell.less'),
        ],
        loader: extractLess.extract({
            use: ['css-loader', 'less-loader'],
            fallback: 'style-loader',
            publicPath: './',
        }),
    },
    {
        test: /\.less$/,
        include: [
            path.resolve(__dirname, 'src/style/tz-appshell.less'),
        ],
        loader: extractShell.extract({
            use: ['css-loader', 'less-loader'],
            fallback: 'style-loader',
            publicPath: './',
        }),
    },
];

config.plugins = [
    extractLess,
    extractSass,
    extractShell,
    new ManifestPlugin({
      map: function(file) { 
         file.name = path.join(path.dirname(file.path), file.name); 
         return file; 
       } 
    }),
];

Expected manifest.json output

{
  ...
  "app.css": "app-c5d773788e95ba4c03ea1c1e306fef61.css",
  "app-new.css": "app-c06bf711eecbd8785ddf0737986d8343daad8681-new.css",
  "app-shell.css": "app-54e823c0625da14e976b6d1cef1a36a971527a1a-shell.css",
  ...
}

Actual manifest.json output

{
  ...
  "/assets/app.css": "/assets/app-c5d773788e95ba4c03ea1c1e306fef61.css",
  ...
}
seaburrows commented 6 years ago

I encountered this problem today and worked around it by simply removing the hash in the file name using map. My case was slightly different to this but I've modified my solution to this use case and hopefully the below will help.

Hopefully for you this will work:

new ManifestPlugin({
    map: function(file) { 
        file.name = file.path.replace(`-${file.chunk.hash}`, '');
        return file; 
    },
}),

However the hash we are using may be the same for all files with the same entry (a problem I had). If so you could do a messier version:

new ManifestPlugin({
    map: function(file) { 
        file.name = file.path.replace(/-[a-z0-9].+\.(css|js)/i, '.$1');
        return file; 
    },
}),

Neither of these are tested, but maybe they will help you get a workaround in place 😄

squidfunk commented 6 years ago

+1

shellscape commented 3 years ago

Hey all! I've taken over maintenance of the plugin and am doing some housecleaning. For Issues over a year old without a reproduction, we're going to go the route of closing them first. However, they're not dead! If the issue is still pending and still a problem, please reply with a reproduction and we'll reopen post-haste.

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.