webpack-contrib / purifycss-webpack

UNMAINTAINED, use https://github.com/FullHuman/purgecss-webpack-plugin
MIT License
772 stars 37 forks source link

problem with ExtractTextPlugin filename #85

Closed t47io closed 7 years ago

t47io commented 7 years ago

Hi, I have seen a problem with PurifyCSSPlugin under production mode. Here is my code:

    new ExtractTextPlugin({
      filename: `${DEBUG ? '[name]-[hash:8]' : '[chunkhash:8].min'}.css`,
      allChunks: true,
    }),
    new PurifyCSSPlugin({
      paths: glob.sync(`${rootPath}/app/**/*.{jsx,json,scss}`),
      styleExtensions: ['.css', '.scss'],
      moduleExtensions: ['.html'],
      purifyOptions: {
        minify: !DEBUG,
        info: !DEBUG,
        rejected: true,
      },
      verbose: true,
    }),

This works perfectly fine when DEBUG=true, and I can see the verbose in console like:

Assets to purify: main-843f0e2c.min.css
Files to search for used rules: file1, file2, file3, ...
##################################
PurifyCSS has reduced the file size by ~10.0%
##################################
This function took:  411 ms
##################################
Rejected selectors:
css1
css2
css3
...
##################################

However, when DEBUG=false, it failed to match any assets:

Assets to purify: 

That's all! So I have narrowed down the problem is actually in my ExtractTextPlugin up there. So basically I learned this:

    new ExtractTextPlugin({
      filename: '[name].css', // this HAS TO START WITH '[name]'
      allChunks: true,
    }),

Is this a desired behavior?

bebraw commented 7 years ago

Hi,

There's something going on. It has been designed to work after ExtractTextPlugin.

Could you set up a tiny project to study?

t47io commented 7 years ago

@bebraw here, src/index.js line 35:

              const assetsToPurify = search.assets(
                compilation.assets, options.styleExtensions
              ).filter(
                asset => asset.name.indexOf(chunkName) >= 0
              );

Basically it demands chunkName to be present in filename. I wonder if my request for being able to use '[chunkhash].min.css' in ExtractTextPlugin is a reasonable practice to you.

bebraw commented 7 years ago

Ok, if you could PR a fix, that would be great.

t47io commented 7 years ago

Well I'm afraid not. This piece of code has been in there for a long time (written by you initially, see https://github.com/webpack-contrib/purifycss-webpack/commit/1f678940f76c4ddb97057632b65129eb88d58b27), and I'm not entirely sure about how important it is to keep the plugin working.

bebraw commented 7 years ago

Looks like it comes from compilation.chunks. It's some corner case the logic is missing.

I might have some proper time for this in a few weeks.

t47io commented 7 years ago

@bebraw How about this? Instead of filtering against name, use files:

Line 33 of src/index.js

          compilation.chunks.forEach(
            ({ name: chunkName, files, modules }) => {
              const assetsToPurify = search.assets(
                compilation.assets, options.styleExtensions
              ).filter(
                asset => files.indexOf(asset.name) >= 0
              );
              ...      

Please advice for testing.

bebraw commented 7 years ago

@t47io Great start. I added a couple of comments to the PR.

bebraw commented 7 years ago

I think we solved this. Thanks for the PR.