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

Modified ignored filter for node_watcher to avoid common exception #91

Merged
merged 1 commit into from
Feb 21, 2017

Conversation

ro-savage
Copy link
Contributor

@ro-savage ro-savage commented Feb 15, 2017

@amasad
This PR updates node_watcher to pass the ignore filters to nodejs_walker so that they can be ignored immediately by nodejs_walker rather than later on in the process.

This was done because on macOS Sierra something (probably fs.lstat) appears to crash when it is searching too many files/directories.

This change will allow a fix in Jest to solve issue jestjs/jest#1767, which should also fix Create-React-App issue facebook/create-react-app#871 and possibly React Native issue facebook/react-native#10028.

I have added a new test, to make sure subdirs are being ignored.

Edit: Updated original commit to improve tests and removed old comments

This may also solve issue #72

@ro-savage ro-savage changed the title Modified ignored filter for node_watcher to avoid common error Modified ignored filter for node_watcher to avoid common exception Feb 15, 2017
@ro-savage ro-savage force-pushed the add-dir-ignore-for-node-watcher branch from 75fe6e5 to 1988b5d Compare February 15, 2017 20:36
@ro-savage
Copy link
Contributor Author

ro-savage commented Feb 15, 2017

I never fully read your readme.

I see under mode you talk about the same issue

If you're running OS X and you're watching a lot of directories and you're running into nodejs/node-v0.x-archive#5463, use watchman

This should allow users to avoid this issue by ignoring folders before

    .on('dir', normalizeProxy(dirCallback))
    .on('file', normalizeProxy(fileCallback))

are ever called.

@ro-savage
Copy link
Contributor Author

@amasad - Any feedback? Would love to get this merged so we can make fixes to Jest.

@amasad amasad merged commit 338f25e into amasad:master Feb 21, 2017
@amasad
Copy link
Owner

amasad commented Feb 21, 2017

Sorry been swamped. Looks good, thanks

amasad added a commit that referenced this pull request Feb 21, 2017
@amasad
Copy link
Owner

amasad commented Feb 21, 2017

Published 1.6

@ro-savage
Copy link
Contributor Author

Awesome. Thanks Amasad!

@ro-savage ro-savage deleted the add-dir-ignore-for-node-watcher branch February 21, 2017 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants