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

Don't iterate over files and folders that should be ignored #1023

Open
watson opened this issue Dec 6, 2017 · 16 comments

Comments

@watson
Copy link
Member

commented Dec 6, 2017

In a project of mine, I have a build directory that quickly gets really large. It's ignored via .gitignore, so standard doesn't fail on it, but it still runs through all files in the directory. It results in a running time of 13 seconds as opposed to 1 second without the build directory.

I know this have been discussed before (#680) and it seems to be a "feature" inherited from eslint. But I'm opening this issue to hopefully start a discussion on how to work around this or how to potentially fix it (either in standard or in eslint).

@Flet

This comment has been minimized.

Copy link
Member

commented Dec 6, 2017

Maybe this fixes it in eslint 4?
eslint/eslint#8706

@watson

This comment has been minimized.

Copy link
Member Author

commented Dec 6, 2017

Hmm not sure. I'm not sure why caching should fix it, but I know almost nothing about eslint internals, so I could very well be wrong.

@ematipico

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2017

AFAIK standard doesn't support eslint 4. I am right?

@watson

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2017

@ematipico Correct. I think what @Flet was hinting at was if the solution to this might come automatically when at some point standard is upgraded to 4.x

@Flet

This comment has been minimized.

Copy link
Member

commented Dec 15, 2017

@watson could you try with standard@beta? It uses the latest version of eslint. At least we could determine if its fixed? :)

@watson

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2017

@Flet doesn't seem to fix it unfortunately:

image

@watson

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2017

hmmm hang on... might have spoke too soon. Let me just check something

@watson

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2017

Nah, I wanted to try to also ignore the folder inside package.json, but no dice. Here you can also see the time it takes if I delete the folder:

image

@Flet

This comment has been minimized.

Copy link
Member

commented Dec 15, 2017

Aww, was worth a shot :(

@GantMan

This comment has been minimized.

Copy link

commented Dec 20, 2017

I'm having this same issue. Fix is traversing ignored paths and complaining with a exit code 1

@searls

This comment has been minimized.

Copy link

commented Dec 21, 2017

Can confirm, I have the same issue with a very-very-very large (almost cyclic) examples/node_modules ignored directory in testdouble.js (still reproducible at this commit). All major versions of Node.js literally run out of heapspace attempting to scan it.

@feross

This comment has been minimized.

Copy link
Member

commented Feb 27, 2018

The issue seems related to deglob. Posting more info from @searls:

screen shot 2018-02-27 at 3 41 09 pm

screen shot 2018-02-27 at 3 41 14 pm

@searls

This comment has been minimized.

Copy link

commented Feb 27, 2018

LOL amazing issue report

@Flet

This comment has been minimized.

Copy link
Member

commented Feb 27, 2018

Nice thank you for digging @serls :)

@watson

This comment has been minimized.

Copy link
Member Author

commented Mar 12, 2018

I took at stab at this and found that this is a combination of a few issues.

As @searls pointed out, the issue is inside deglob.

  1. Diving further, the time is spend inside the glob module: https://github.com/standard/deglob/blob/bce559a275faf286395c52912a82c601a6740f2d/index.js#L23-L27
  2. The ignore option passed to the glob module does not contain patterns found in the .gitignore file, so files only ignored here will still be processed by glob
  3. Even if the lines from the .gitignore file were passed to the ignore option, these would have to end with /** to ignore whole directories. This is inconsistent with how .gitignore works and with what's documented in the README

Suggested solution:

  1. Document that you need to append /** to directories you wish to ignore in the package.json standard.ignore array
  2. Convert the patterns from .gitignore to patterns compatible with the glob module and add them to the list of ignored glob patterns

@feross what do you think of this?

@searls

This comment has been minimized.

Copy link

commented Mar 20, 2018

My preferred solution would be to update deglob to behave like the glob module or .gitignore when it comes to foo/**/* style patterns.

I realize it could be seen a breaking change, but one person's breaking change is another's patch fix! Based on the popularity of using **/* to mean "all files, recursively" I can't imagine anyone is intentionally passing deglob ignore rules of that type with an expectation they will not be ignored recursively.

Is that on the table?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.