.eslintignore patterns are not excluded from globbing, so performance can be very slow

[rocketmonkeys]

What version of ESLint are you using?
v3.1.1

What parser (default, Babel-ESLint, etc.) are you using?
Default (espree?)

Please show your full configuration:

In .eslintignore:

/some/folder/with/lots/of/files/**

What did you do? Please include the actual source code causing the issue.
run eslint .

What did you expect to happen?
eslint should be quick, since I've excluded folders via .eslintignore.

What actually happened? Please include the actual, raw output from ESLint.
eslint takes a long time, since it still globs all folders even if they're in .eslintignore.

This is a follow-on from #5679. The fix for that one excluded "DEFAULT_IGNORE_DIRS" from globbing, which was a good start.

However, if your project has large folders outside of the default ones, globbing will still take a long time even if you specify them in .eslintignore.

Including all patterns from .eslintignore speeds things up a lot. The goal is to exclude the patterns in .eslintignore from globbing, just like we do with DEFAULT_IGNORE_DIRS already.

[platinumazure]

I think the dust has mostly settled from our last attempt to fix globbing of ignored directories (in the default ignore directories), so I'd love to see us take another crack at this. But we'll see what the team thinks.

[alberto]

Thanks for opening the issue. I think I have an idea of how to do this, so I'll take a stab at this.

[nzakas]

We did say we would do this, so marking as accepted.

[rocketmonkeys]

My PR (rocketmonkeys#1) has a really naive attempt at fixing this. It's probably all sorts of wrong, but it does work for at least my case, and speeds things up a ton.

[ilyavolodin]

@rocketmonkeys Unfortunately, it wouldn't work for all cases. .eslintignore is using gitignore style, but traversal is done using globs. While those two patterns intersect and have common syntax, they are not the same, and in a lot of cases, glob will not understand gitignore patterns. That's the reason we couldn't fix it correctly the first time around.

[nzakas]

@rocketmonkeys you should read through the whole thread on #6215 where we discuss some of the gotchas.

[alberto]

My intention was to do glob ignoring only when we have an ignored folder in a glob compatible syntax (no wildcards or any special syntax? absolute path?) an we can ensure it's not unignored later (no unignore at all?).

[kaelzhang]

Maybe we could inherit glob.GlobSync(glob.sync.GlobSync) class, override prototype._process method, and use our own childrenIgnored method?

Our custom childrenIgnored could support both minimatch patterns and gitignore patterns.

But there is still maintainence issue, that we will have to upgrade our sub-class if GlobSync changes.

node-glob is not that easy to be monkeypatched.

[alberto]

@eslint/eslint-team any opinions on how do we want to approach this? @kaelzhang also submitted #6730 that addresses this by forking glob

[mysticatea]

Though I don't understand this issue.
If we use monkeypatching, Glob.prototype._readdir might be proper.

My image is:

const IGNORE = Symbol("ignore");

class OurGlob extends glob.GlobSync {
    constructor(pattern, options, ignore) {
        super(pattern, options);
        this[IGNORE] = ignore; // use symbol to avoid to conflict with base class's property.
    }

    _readdir(abs, inGlobStar) {
        if (this[IGNORE].match(abs)) {
            return null;
        }
        return super._readdir(abs, inGlobStar);
    }
}

[nzakas]

I don't think we want to fork glob to solve this problem. Inheritance seems like a better option.

[kaelzhang]

@nzakas Ok，I'm working on this.