webpack-contrib / purifycss-webpack

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

Purifycss with Tachyons #73

Closed joaovpmamede closed 7 years ago

joaovpmamede commented 7 years ago

I was trying this plugin (version 0.4.2) with Tachyons but it seems that it adds a couple more classes than what I was expecting.

I have an example here. Just run npm run build and check the content of dist/tachyons.css.

On my index.html I'm only using the .pa4 class from tachyons (spacing module) but it seems that the final css file contains a lot more:

screenshot 2017-01-29 21 00 13

Also tried to use a class inside a media-query (.pa4-l) and I got .pa0 to .pa7 and .pa0-l to .pa7-l.

bebraw commented 7 years ago

Looks like

new PurifyCSSPlugin({
  // Give paths to parse for rules. These should be absolute!
  paths: {
    app: glob.sync(path.join(__dirname, 'index.html'))
  },
}),

fixes your use case.

The problem is that your current definition makes it to traverse each entry of your setup so you have to constrain it to a specific entry (this is by design).

After this change it picks up only global styles as you might expect.

joaovpmamede commented 7 years ago

@bebraw tried with app and tachyons but it still doesn't work. Did you try running the example?

Thanks

bebraw commented 7 years ago

@joaovpmamede Yeah. It worked for me. The end result has comments so you'll need to minify to get rid of those too.

joaovpmamede commented 7 years ago

Sorry to insist but did the generated css file have the .pa4 class in it?

I've updated the repo with the generated dist folder. Webpack config also updated by your comment.

bebraw commented 7 years ago

Sorry to insist but did the generated css file have the .pa4 class in it?

Nope. Here's the output.

joaovpmamede commented 7 years ago

I see, so it's still not working as expected. I have that class in my index.html so shouldn't be adding it to the generated css file?

bebraw commented 7 years ago

The entry paths are expected to be JavaScript. See here.

I don't mind accepting a PR that would extend that definition.

joaovpmamede commented 7 years ago

I might not be getting how this works in the end. I just want to get the classes defined in index.html on my generated css file.

bebraw commented 7 years ago

That app: glob.sync(path.join(__dirname, 'index.html')) could become just app: path.join(__dirname, 'index.html').

I'll re-open this issue in case someone else wants to debug.

joaovpmamede commented 7 years ago

I'll try to work a bit on this at a later time but yes this should be open because it's not working as it should. From my understanding and per documentation:

Your scripts and view files will be scanned for classes, and those that are unused will be stripped off your CSS - aka. "purified".

But it's failing to strip the other classes that I'm not using on my "view" file

Thanks

bebraw commented 7 years ago

It's some subtle thing. I added verbose flag although you probably want to add more logging to the code to understand better what it's picking or not picking.

An interesting alternative would be to use Purify without webpack to rule out a Purify related issue. If that fails, there's not much to do.

joaovpmamede commented 7 years ago

An interesting alternative would be to use Purify without webpack to rule out a Purify related issue. If that fails, there's not much to do.

I'll try to do that. Thank you so much for spending time and looking at this 👍

maxsalven commented 7 years ago

I'm not sure if this is the same issue, but I'm trying to use Tachyons with React. Inside my React app, I have a class of "green". However, the resulting CSS does not have the class "green". The resulting CSS has an assortment of seemingly random classes from Tachyons instead, none of which are used in the app.

The class is inside a javascript file, not a 'view' file, so not sure if the problem is the same as this issue. If you'd like me to open a new issue, let me know, thanks.

Here is a sample repo, built on an ejected "Create React App" template. npm run build will produce the css file for you. Github Repo

johakr commented 7 years ago

@maxsalven Don't know if you just mixed it up in the test repo. But your issue there is that you pass paths.appBuild instead of paths.appSrc. If you pass the latter path, the green class is included in the build.

Kikobeats commented 7 years ago

Using the old plugins this worked for me:

new PurifyCSSWebpackPlugin({
      basePath: path.resolve('src/www'),
      resolveExtensions: ['.js'],
      purifyOptions: {
        minify: true,
        rejected: true
      }
    }),

what is the equivalent code for that?

bebraw commented 7 years ago

@Kikobeats moduleExtensions is the new resolveExtensions. You would have to glob basePath content and pass it through paths. purifyOptions is the same.

Kikobeats commented 7 years ago

so, something like:

new PurifyCSSPlugin({
      paths: glob.sync(path.resolve('src/www/**/*')),
      moduleExtensions: ['.js'],
      purifyOptions: {
        info: true,
        minify: true,
        rejected: true
      }
    }),

but it doesn't work:

WARNING: Could not read /Users/josefranciscoverdugambin/Projects/windtoday/windtoday-marketplace/src/www/assets.

WARNING: Could not read /Users/josefranciscoverdugambin/Projects/windtoday/windtoday-marketplace/src/www/assets/css.

WARNING: Could not read /Users/josefranciscoverdugambin/Projects/windtoday/windtoday-marketplace/src/www/assets/img.

WARNING: Could not read /Users/josefranciscoverdugambin/Projects/windtoday/windtoday-marketplace/src/www/assets/img/provider.

WARNING: Could not read /Users/josefranciscoverdugambin/Projects/windtoday/windtoday-marketplace/src/www/assets/js.
##################################
PurifyCSS has reduced the file size by ~0.0%
##################################
This function took:  56212 ms
##################################
Rejected selectors:

##################################

meanwhile in the old plugin this works fine, see here: https://travis-ci.org/windtoday/windtoday-marketplace/builds/208534731#L1332

bebraw commented 7 years ago

@Kikobeats It's a good idea to pass { nodir: true } to glob to avoid matching directories as there's nothing to read (that case could be fixed). Did you try the verbose mode to see what it matches? It's not picking up something. If you have time, study the code in detail. It should be easy to follow.

Kikobeats commented 7 years ago

Logs here:

https://gist.github.com/Kikobeats/634745c9623933a4a4f6adb74a675064

I'm trying to see the differences, but using the same input as the old plugin, it si not producing the same output :(

bebraw commented 7 years ago

@Kikobeats Could you extract a tiny project that works in the old and fails in the new plugin? The simpler, the better. That would help a ton.

Kikobeats commented 7 years ago

yes, https://github.com/Kikobeats/react-boilerplatinum

this use the old plugin; try to update with the new 😄

bebraw commented 7 years ago

@Kikobeats Can you try?

diff --git a/webpack.config.js b/webpack.config.js
index 1ff9f5f..c2d9cff 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -1,12 +1,13 @@
 'use strict'

 const HtmlWebpackHarddiskPlugin = require('html-webpack-harddisk-plugin')
-const PurifyCSSWebpackPlugin = require('purifycss-webpack-plugin')
+const PurifyCSSWebpackPlugin = require('purifycss-webpack')
 const ExtractTextPlugin = require('extract-text-webpack-plugin')
 const HtmlWebpackPlugin = require('html-webpack-plugin')
 const OfflinePlugin = require('offline-plugin')
 const webpack = require('webpack')
 const path = require('path')
+const glob = require('glob');

 const config = require('./config.json')
 const pkg = require('./package.json')
@@ -71,8 +72,9 @@ module.exports = {
       filename: 'assets/css/bundle.css'
     }),
     new PurifyCSSWebpackPlugin({
-      basePath: path.resolve('src/www'),
-      resolveExtensions: ['.js'],
+      paths: glob.sync('src/app/**/*', { nodir: true }),
+      styleExtensions: ['.css'],
+      moduleExtensions: ['.js'],
       purifyOptions: {
         minify: true,
         rejected: true

I think the main thing missing was moduleExtensions. Tachyons relies on JS by the looks of it. This gives 24.2 kB CSS instead of 35.4 kB so the config might still be missing something.

styleExtensions has to be eliminated by the looks of it. Only css makes sense there.

Kikobeats commented 7 years ago

Thanks for your time @bebraw, it works for me 😄

joaovpmamede commented 7 years ago

An interesting alternative would be to use Purify without webpack to rule out a Purify related issue. If that fails, there's not much to do.

@bebraw just tested this and it happens without webpack. So it's not related at all with this project.

bebraw commented 7 years ago

@joaovpmamede Thanks! Please report upstream. 👍