react-scripts test is watching files in node_modules

[thisconnect]

Can you reproduce the problem with latest npm?

yes

Can you still reproduce it?

yes, also did npm cache clean just to be sure

Description

This is a followup of "npm test hangs for newly generated app (#960)". After reading the comment of @hmeerlo that "Jest is trying to watch too many files at once (jestjs/jest#1767 (comment))" I suspect that files in node_modules are unnecessarily being watched.

Expected behavior

npm t should run without error.

Actual behavior

react-scripts test --env=jsdom

2017-05-28 09:18 node[1154] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
2017-05-28 09:18 node[1154] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-22)
events.js:160
      throw er; // Unhandled 'error' event
      ^

Error: Error watching file for changes: EMFILE
    at exports._errnoException (util.js:1018:11)
    at FSEvent.FSWatcher._handle.onchange (fs.js:1420:11)
npm ERR! Test failed.  See above for more details.

Environment

Run these commands in the project folder and fill in their results:

1. npm ls react-scripts (if you haven’t ejected): [email protected]
2. node -v: v6.10.3
3. npm -v: 4.6.1

Then, specify:

1. Operating system: macOS Sierra 10.12.5
2. Browser and version: -
3. watchman never installed

Reproducible Demo

https://github.com/thisconnect/cra-desktop

Reproduce the error

This is an instantiated create-react-app (un-ejected) with electron added as described in PR #1718

git clone https://github.com/thisconnect/cra-desktop
cd cra-desktop
npm install
npm test

Without error (when removing 2 large dependencies)

Assuming react-scripts test is indeed watching files in node_modules, remove electron and electron-packager from devDependencies in package.json, then rm -rf node_modules && npm i && npm t, test mode runs fine. This does not yet prove that files in node_modules are being watched.

Strong indication that node_modules is being watched:

• run any CRA instance without error
• the Terminal should display something like "No tests found related to files changed since last commit" and the "Watch Usage"
• open a random .js file in node_modules
• for example node_modules/mime/mime.js
• watch the Terminal (screenshot 1)
• add a comment on any line i.e. // test
• save the file
• check if the terminal changed (screenshot 2)

Terminal before changing a random file in node_modules (screenshot 1)

Terminal after chaning a random file in node_modules (screenshot 2)

[thisconnect]

Side note: The error does not appear in [email protected] and was introduced with [email protected]

[thisconnect]

I am not exactly sure what the corresponding jest config is, maybe testMatch in https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/scripts/utils/createJestConfig.js ?

My test about react-scripts test watching seems to not only affect node_modules/**/*.js, but also files in /dist/*.js and in /build/*.js.

[ro-savage]

Thanks for creating the example source @thisconnect - I have a PR #2395 that should fix this issue.

Although the underlining issue is with Jest, and will depend if the maintainers of CRA would prefer to wait for Jest to fix, or fix it here until it's fixed in Jest.

[gaearon]

How hard is it to fix in Jest?

[ro-savage]

I am really unsure.

As mentioned in jestjs/jest/issues/1767 when Jest is using the node_watcher it is tells it to watch everything with chosen extensions, rather than just the files in testMatch.

Its possible the fix is just to pass testMatch to the file watcher. Its also possible this breaks a bunch of other stuff. Unfortunately I don't understand why Jest does what it does. It might just be a bug or if there is some greater reason for it.

Hopefully @cpojer or @pugnascotia will have a better answer.

[cpojer]

@ro-savage fyi, if Jest only watches the files matching testMatch, that means that we won't be able to run tests when non-test files change. So that is the expected behavior.

[ro-savage]

@cpojer - Of course, it didn't even click that it was only matching the test files.

Then it should be fine to use both testMatch and modulePathIgnorePatterns?
In fact modulePathIgnorePatterns would be the way correct to tell Jest what files/folders not to watch?

(I understand there might be edge cases where you npm install an updated module and then want the tests to rerun. But seems better to miss that edge case than crash.)

[gaearon]

This might help fix the crashes: amasad/sane#97.
I think we should first direct effort there, and if it doesn’t work out, try strategies in this issue.

[thisconnect]

New sane version works for me amasad/sane#97 (comment)

[thisconnect]

Jest merged sane in jestjs/jest#3918
jestjs/jest@acc783a

[diptendulkar]

Reinstalling watchman fixed this error for me.

brew uninstall watchman
brew install watchman

[thisconnect]

[email protected] should have the fixed sane using fsevents

jestjs/jest#1767 (comment)

[andrew-oxenburgh]

I did a sudo port install watchman, and problem went away.

Apparently you need watchman installed.

[gaearon]

We updated to Jest 22 on the next branch so while we're still watching node_modules, it should no longer cause the crash in this issue. We can't backport this to [email protected] but stay tuned for the alpha versions of [email protected] coming soon.

[thisconnect]

thanks @gaearon !!!
Can I test next by installing facebookincubator/create-react-app#next?

[gaearon]

No, we don’t support installation from GitHub. When it’s available, you will be able to just specify the version we release.

[thisconnect]

ok, thanks

[thisconnect]

@gaearon just FYI I am testing [email protected] (without watchman) and it is actually the first time npm test runs with the electron stuff 🎉 🎊 🎻 🏆 🎸 🍸 🎂 🌈 🚀 (test repo https://github.com/thisconnect/cra-desktop)

Had a minor issue running npm start, commented in #3815 (comment) and another issue with node6 travis CI https://travis-ci.org/thisconnect/cra-desktop/builds/373373379 something about Requires Babel "^7.0.0-0", but was loaded with "6.26.0".
(travis node8 is fine)

I am sorry for commenting on this closed issue, just wanted to report and say thanks. Thanks!