Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to Exclude Folders From Content Globs #620

Closed
bdkjones opened this issue Feb 28, 2021 · 10 comments
Closed

Unable to Exclude Folders From Content Globs #620

bdkjones opened this issue Feb 28, 2021 · 10 comments
Labels

Comments

@bdkjones
Copy link
Contributor

bdkjones commented Feb 28, 2021

Describe the bug
I'm filing this as a bug because the top results on Google for "purgeCSS skip folder" link to outdated advice from the PurgeCSS team. Specifically: #158 (comment)

You advise folks to use this syntax to skip a certain folder:

content: [
    '!(skippedFolder)/**/*.html',
    ...
]

The trouble is that your glob dependency removed support for ! as a negation operator in version 6.0. You currently use version 7 of glob. The ! operator is supported only in glob 5.x and below.

The way to skip folders in glob 6.x+ is to use their ignore option, which PurgeCSS does not currently expose.

To Reproduce
Simply try to use the ! operator to negate a content glob; it will fail and PurgeCSS will remove rules that it SHOULD keep because it's not scanning files appropriately.

Expected behavior
PurgeCSS should allow users to skip certain folders, such as node_modules—especially because these folders can contain thousands of subfolders and having PurgeCSS scan all of them needlessly makes you look slow and bad.

Screenshots
N/A

Environment (please complete the following information):

  • OS: macOS 11.2.2
  • Package purgeCSS
  • Version 4.0.0

Additional Context
There are other people asking for ways to exclude folders:
#551

@tordans
Copy link

tordans commented Mar 1, 2021

Thanks for giving this more exposure.

Some additional references:

https://github.com/isaacs/node-glob/blob/master/README.md#comments-and-negation says

Comments and Negation
Previously, this module let you mark a pattern as a "comment" if it started with a # character, or a "negated" pattern if it started with a ! character.
These options were deprecated in version 5, and removed in version 6.
To specify things that should not match, use the ignore option.

The new ignore option is described at https://github.com/isaacs/node-glob/blob/master/README.md#options as

ignore Add a pattern or an array of glob patterns to exclude matches. Note: ignore patterns are always in dot:true mode, regardless of any other settings.

At first glance, it looks like the ignore option would have go to into
https://github.com/FullHuman/purgecss/blob/master/packages/purgecss/src/index.ts#L372-L374 where ATM only one option is given:

      } catch (err) {
        filesNames = glob.sync(globfile, { nodir: true });
      }

@bdkjones
Copy link
Contributor Author

bdkjones commented Mar 1, 2021

Yea, I forked the repo and implemented the option (I called it “skiplist” because I felt that was more descriptive).

I think(?) I got it right, but I had some trouble building everything. Probably because I’m doing something dumb with the mono repo.

I’ll push my changes to my fork and then post it here so you can take a look. I implemented the option only for purgeCSS core and postcss-purgecss because I don’t use any of the other bindings. Should be pretty easy to extend all of them though.

@Ffloriel
Copy link
Member

Ffloriel commented Mar 2, 2021

Thanks for taking a look at this @bdkjones.
I will take care of the other bindings.
To build the monorepo, you would need to run:

npm i
npm run bootstrap

and you can then run the test, and build:

npm run test
npm run build

@tordans
Copy link

tordans commented Mar 2, 2021

@bdkjones Reading the "Note" at master...bdkjones:master#diff-ec064e2ecde8584821d98c416e636e9a3889037faa41387bc725986b877a403eR99

If you provide globs for the content parameter, you can use this option to exclude certain files/folders that would otherwise be scanned. Pass an array of regex patterns that should be excluded. Note: this option has no effect if content is not globs.

… I wonder if the name skiplist could already community those constraints (the list applies to "content" and only if content is "globs").

Something like contentGlobsSkiplist for example …

@bdkjones
Copy link
Contributor Author

bdkjones commented Mar 3, 2021

I've successfully got the ignore parameter working....but only when I pass an exact path to the file that should not be scanned. I've added a test case that currently passes under those conditions, but fails when using any part of a path, such as skippedFolder in my test example.

I suspect this is because the ignore parameter wants an array of patterns and I'm passing an array of strings because that's what the TypeScript compiler told me it expected (obviously I've never written TS).

Here's what I have: master...bdkjones:master

@bdkjones
Copy link
Contributor Author

bdkjones commented Mar 3, 2021

Actually, I think it might be that I’m just really bad at Regex. I can’t seem to make glob’s ignore option work with anything less specific than:

/full/path/to/skippedFolder/**

Unanchored patterns such as /skippedFolder/ don’t work. Nor do anchored patterns such as skipped.html$

I looked at the test cases glob uses for their ignore option, but haven’t successfully figured out what’s going on yet.

@bdkjones
Copy link
Contributor Author

Turns out I'm only 50% stupid. It was an issue with cwd(); I didn't realize the directory the test script was running from. The test now passes and I think the skippedContentGlobs parameter should now work with an entry such as node_modules/** when PurgeCSS is run in production with the cwd set to the parent folder that contains the node_modules folder. I'll submit my work as a PR, but I highly suggest someone look it over carefully—I make no promises it won't format your Mac and kill your grandmother.

@bdkjones
Copy link
Contributor Author

PR submitted: #638

@tordans
Copy link

tordans commented Mar 19, 2021

FYI for those looking for a workaround until #638 (thanks bdkjones) is usable in your codebase, you could do something like this:

This will remove all paths that include the /styleguide/ folder.

const contentGlobWithoutStyleguide = () => {
  // (1) Recreate the file list the same way PostCSS does it
  const glob = require("glob")
  return [
    './app/**/*.html.erb',
    './app/**/*.html.slim',
    './app/helpers/**/*.rb',
    './app/javascript/**/*.js',
    './app/javascript/**/*.jsx',
    './config/locales/**/*.yml',
    './config/locales/**/*.json',
  ].flatMap((pattern) => glob.sync(pattern, { nodir: true }))
    .filter((path) => !path.includes('/styleguides/')) // (2) Ignore Styleguide in PurgeCSS
}

environment.plugins.push(
  require('@fullhuman/postcss-purgecss')({
    content: contentGlobWithoutStyleguide(),
    safelist: ,
    defaultExtractor: 
  })
)

To test this, you can console.log the outpout of the defaultExtractor. For example like this:

postCssDebugOutputForDevelopmentEnv = true // default false

      defaultExtractor: content => {
        let match = content.match(//gi) || []
        if (postCssDebugOutputForDevelopmentEnv === true) {
          console.log(match)
        }
        return match
      }

@bdkjones
Copy link
Contributor Author

Closing because this feature has landed on master. Thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants