webpack-contrib / purifycss-webpack

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

Enable/disable per entry #106

Open crucialfelix opened 7 years ago

crucialfelix commented 7 years ago

purifycss-webpack@0.6.2

I see that there is a way to provide different paths per entry, but I need to completely disable purifycss for some entries.

I have an angular-js app that cannot be read by purifycss. I only want to run purifycss on one entry.

Is there a way to do this that I've missed ?

Thanks.

bebraw commented 7 years ago

It's all or nothing right now. I could accept a PR changing this if you can propose good semantics on how to handle it.

crucialfelix commented 7 years ago

Maybe:

new PurifyCSSPlugin({
  // Default settings for all processed css files
  styleExtensions: ['.css'],
  moduleExtensions: ['.html'],
  minimize: false,
  paths: glob.sync(path.join(__dirname, 'app/*.html')),
  purifyOptions: {
    whitelist: ['debug', 'blink']
  },
  verbose: true,

  // Add global disable switch
  disable: true, 

  // For each entry you may supply custom settings
  // that are merged over the global defaults.
  entry: {
    [entryName]: {
      disable: false,  // active here
      moduleExtensions: ['.js'],
      paths: glob.sync(path.join(__dirname, 'react/*')),
      purifyOptions: {
        whitelist: ['wf-active']
      }
    },
    // etc for further entries...
  }
});

I think that reads clearly. I would know fairly quickly how to use it. What do you think ?

I would have to go learn that json schema, but it doesn't look too complicated.

bebraw commented 7 years ago

I would drop the global disable: true since you can handle that on configuration level.

I wonder if we could do some kind of a include/exclude array instead.

I'll try to get more input on this issue so we get the design right.

michael-ciniawsky commented 7 years ago

Global include /exclude instead of disable is better imho 👍 disable globally ===

- new PurifyCSS() 

The per entry config is basically 👍 , only concerns of mine are maintenance > used ? and performance

bebraw commented 7 years ago

The per entry config is basically 👍 , only concerns of mine are maintenance > used ? and performance

It's actually done already. What we would do here is to make the behavior more granular.

The more I think about it, the more a good include/exclude setup makes sense to me. That could follow webpack semantics exactly and I agree it's the way to go. Essentially that's a filtering pass before anything gets passed to Uglify.

crucialfelix commented 7 years ago

Do you mean include [entryName,...] ?

I find that when there are multiple entries that they are all substantially different.

bebraw commented 7 years ago

@crucialfelix Yeah, that would be a start.

crucialfelix commented 7 years ago

I still think a disable switch is very useful. It's easy to disable for all and then enable for the few or the one. Or enable for all and just disable one.

In development I would disable it; maybe I'm using tachyons and I want all those classes available in my browser to experiment with. In production the unused ones get stripped.

ExtractTextPlugin uses disable. UglifyJSPlugin uses compress.

I'm not familiar with any that use include / exclude lists. Which ones are you thinking of ?

The implementation for how I described it above would actually be really easy. Only a few lines of code.

disable is just an if statement.

options per entry is essentially just:

chunkOptions = Object.assign({}, options, options.entry[chunkName] || {})

(rough sketch. options should be merged without .entry etc.)

As I mentioned yesterday, when I have multiple entries then for purifycss they would each have completely different parameters. Different templates/js to scan, different white lists.

eg.

I'm happy to put together a PR with whatever the consensus is.

bebraw commented 7 years ago

I still think a disable switch is very useful. It's easy to disable for all and then enable for the few or the one. Or enable for all and just disable one.

The point was that it's possible to achieve the same result globally without touching the plugin. That's how everything else works in webpack ecosystem. I think we should refactor possible disable flags out of the existing plugins for this reason as it's a bad abstraction (negative value from plugin implementation pov + adds inconsistency since not every plugin implements it).

Please set up a prototype based on include/exclude as discussed above. It doesn't have to be fancy, just enough to show the exact usage you have in mind.

crucialfelix commented 7 years ago

Sorry, you've totally lost me.

Where is this include / exclude api ?

This relates to rules, but from reading this I have no idea what it is actually for:

https://webpack.js.org/configuration/module/#rule-include

I can find this:

An object {test, include, exclude} similar to module.loaders enables it for specific files.

https://webpack.js.org/concepts/output/#output-devtoollinetoline

But I don't see how this relates to configuring a plugin.

If you say that "the same result can be achieved globally without touching the plugin", then I'm not sure what you are proposing to do to this plugin.

Sorry, I don't understand the proposal.

bebraw commented 7 years ago

There's a piece like search.assets in the code. I imagine you would apply additional .filter where to check each asset against an include rule. This works if you can figure out into which entries each asset belongs to (asset might contain the info). exclude would be the same except inverse.

The interface could look like include: (asset) => { ... } to get started. If an asset contains reference to an entry where it belongs, this would be enough to achieve what you want. Check that first, though.

mradambeck commented 7 years ago

Any update on this? It would be great if there was any way at all to match entry/split points to paths. A lot of treeshaking can be done with CSS modules, but if you're not generating your HTML via JS, that's not an option, and something like this would be great. For my use case, even if we could just refer to the chunk name in the path declaration or something it could work.