Add cache to `isGitIgnored` and `isGitIgnoredSync`

[fisker]

I was running xo in eslint-plugin-unicorn project, and found it's very slow. So I add some debug info

eslint.isPathIgnored: 0.097ms
micromatch.isMatch: 0.247ms
isGitIgnoredSync: 2.774s
eslint.isPathIgnored: 0.138ms
micromatch.isMatch: 0.537ms
isGitIgnoredSync: 2.396s
eslint.isPathIgnored: 0.091ms
micromatch.isMatch: 0.245ms
isGitIgnoredSync: 2.709s
eslint.isPathIgnored: 0.107ms
micromatch.isMatch: 0.331ms
isGitIgnoredSync: 2.806s
eslint.isPathIgnored: 0.176ms
micromatch.isMatch: 0.543ms
isGitIgnoredSync: 2.408s
eslint.isPathIgnored: 0.14ms
micromatch.isMatch: 0.579ms

And found isGitIgnoredSync is very slow. It keeps glob and read .gitignore file again and again.

After this

isGitIgnoredSync: 0.1ms
eslint.isPathIgnored: 0.193ms
micromatch.isMatch: 0.197ms
isGitIgnoredSync: 0.051ms
eslint.isPathIgnored: 0.074ms
micromatch.isMatch: 0.248ms
isGitIgnoredSync: 0.151ms
eslint.isPathIgnored: 0.118ms
micromatch.isMatch: 0.454ms
isGitIgnoredSync: 0.109ms
eslint.isPathIgnored: 0.077ms
micromatch.isMatch: 0.223ms
isGitIgnoredSync: 0.037ms
eslint.isPathIgnored: 0.074ms
micromatch.isMatch: 0.244ms
isGitIgnoredSync: 0.091ms

Problem is how should we process this? Add an option? Add a new createIsGitIgnored function? Or just cache the result?

[sindresorhus]

The cache key needs to take into account the gitignore file mtime, otherwise, it will cause incorrect stale results if the gitginore file changes.

[sindresorhus]

Hmm, this caching seem to assume it's being used in a short-running process.

[fisker]

For xo, it should created an isGitIgnored on each xo.lintFiles() call, and only read .gitignore once. So let's add a createIsGitIgnored function?

[fisker]

One second, it seems xo already filter the ignored file https://github.com/xojs/xo/blob/b15cbfed6e7db7100158fc9d614b69621449604e/index.js#LL31C34-L31C34, maybe just remove this line https://github.com/xojs/xo/blob/b15cbfed6e7db7100158fc9d614b69621449604e/index.js#L93? This line also use ignore: ignorePatterns, but isGitIgnoredSync doesn't accept it at all https://github.com/sindresorhus/globby/blob/cb892abe709a6359e80e1895b8a6d4a0890155f4/ignore.js#L59.

[fisker]

Finally understand the problem.

xo isn't use isGitIgnoredSync correctly.

+   const isIgnore = isGitIgnoredSync({cwd});
    const reports = await Promise.all(
        Object.values(groups)
            .map(async filesWithOptions => {
                for (const options of filesWithOptions) {
                    /* ... */
-                   isGitIgnoredSync({cwd})(filePath)
+                   isIgnore(filePath)
                    /* ... */
                }
            }));

We have to admire this isGitIgnoredSync is a bad design.