Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Using globs for directory is slow #12

Closed
freshollie opened this issue Sep 16, 2019 · 9 comments
Closed

Using globs for directory is slow #12

freshollie opened this issue Sep 16, 2019 · 9 comments

Comments

@freshollie
Copy link

The recommended config for multiple directories is very slow (./packages/**/tsconfig.json)

This is probably due to the library walking the node_modules directories? Would it be possible to ignore the node_modules folders?

For now, I recommend explicit levels in the directories glob, which is much faster.

"settings": {
    "import/resolver": {
    "ts": {
      "directory": "./*/*/tsconfig.json" // Any package, but don't go deeper than the package root
    }
  }
@JounQin
Copy link
Member

JounQin commented Sep 21, 2019

@freshollie Sorry I didn't notice this issue previous, I'll take a look ASAP.

@JounQin
Copy link
Member

JounQin commented Sep 21, 2019

@teoxoy Would you like to help with this?

@teoxoy
Copy link

teoxoy commented Sep 21, 2019

./packages/**/tsconfig.json shouldn't look in the node_modules folder.

Having said that, the example glob can be optimized like this: ./packages/*/tsconfig.json - this way the star (*) will only match one layer deep instead of the globstar (**) matching n layers

@freshollie can you test the proposed glob and see if it's faster?

If it is, we should update the readme.

@JounQin
Copy link
Member

JounQin commented Sep 22, 2019

@teoxoy Should directory matched in node_modules be ignored always? What means we can use glob.sync(pattern, { ignore: 'node_modules' }).

@teoxoy
Copy link

teoxoy commented Sep 22, 2019

ignore should only be used when the pattern would include a path that then ignore will remove.

Using it in our case, will make things slower and with no real benefits since ./packages/*/tsconfig.json will not match anything inside node_modules.

As I said above, the issue was that I used a globstar instead of a star for the example.

@JounQin
Copy link
Member

JounQin commented Sep 22, 2019

@teoxoy Thanks for clarifying. Maybe another stupid question, should we replace glob with tiny-glob according to its benchmark?

@JounQin
Copy link
Member

JounQin commented Sep 22, 2019

@freshollie v0.4.0 has been released.

@freshollie
Copy link
Author

Hey @JounQin, thanks for this, updating now.

@teoxoy, yes, using the single star glob was always faster. I had already discovered this before posting the issue, so I was more trying to inform that using ** would make eslint very slow and that having it in the readme was a bad idea.

@JounQin
Copy link
Member

JounQin commented Sep 24, 2019

@freshollie Yep, we've changed ** to * in README, thanks for reporting any way.

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

No branches or pull requests

3 participants