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

feat(config): allow passing option to force node crawl #11264

Merged
merged 4 commits into from Apr 2, 2021

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Apr 2, 2021

Summary

Due to #9351 (comment), I want to allow people to configure it.

We should consider defaulting to the Node crawl, but that's separate. With this, at least people can choose

Test plan

Verified via debug statements the option makes it all the way through from jest config to haste-map

@SimenB SimenB merged commit 6d87f9e into jestjs:master Apr 2, 2021
@SimenB SimenB deleted the force-node-crawl-options branch April 2, 2021 13:42
@jtbandes
Copy link

Thanks for making this change! Just want to add a note that I was benchmarking jest locally (M1 MacBook Air with macOS 11.2.3), and noticed that find is much slower than the fs implementation because it does not exclude any files. The ignore patterns are only applied after find returns results:

https://github.com/facebook/jest/blob/c8b1835b53945412f52b37d9219a8f52d9449ff1/packages/jest-haste-map/src/crawlers/node.ts#L177-L181

In our relatively small monorepo, this results in a find execution that takes several seconds and produces a huge output (43512 lines / 6MB of text).

@SimenB
Copy link
Member Author

SimenB commented Apr 23, 2021

Ah, that's a very good point. Exclusions in find look absolutely horrible, tho 🙈

https://stackoverflow.com/a/4210072/1850276

@jtbandes
Copy link

Yep. I also understand that it may be difficult to translate to a find command because ignore() is currently a function rather than a list of glob patterns. So I wonder if using the native find is even worth it at all — but anyway I appreciate the option to disable it, otherwise it lists out every .js file inside node_modules.

jtbandes added a commit to foxglove/studio that referenced this pull request Apr 24, 2021
On my M1 MacBook Air I can now run **all** the tests in about 16 seconds. Running a single small test file is down from 15sec to <2sec.

There were a few components to this:
- Jest would take several seconds to load `jest.config.ts` files (jestjs/jest#11337). Fixed by replacing them with `jest.config.json` files.
- Jest tries to use the native `find` if available over node fs, but it doesn't pass any exclude options to find, which means that it has to wait a few seconds for `find`, then filter a huge list of all .js files in app/node_modules. Upgrading jest lets us pass a config option to force it to use node fs instead (jestjs/jest#11264 (comment)).
- `jsdom` is no longer the default environment in jest, and there is some overhead to jsdom. We are embracing this change by only enabling jsdom in certain test files that need it. Unfortunately Jest only recognizes the `/** @jest-environment jsdom */` comment if it is at the very beginning of the file, so I had to replace our eslint-plugin-header rule with a custom rule to enforce the presence of license headers.
- Converted the custom `?raw` transformer to .js instead of .ts, since we are not using ts-node anymore.
- Actually executing the test files with ts-jest was slow, because it has to run the TS compiler. Instead we can use babel-jest which doesn't do type checking. kulshekhar/ts-jest#961 (comment) (This also requires enabling `isolatedModules` in tsconfig.) For better or worse, our regular webpack build already typechecks our test files.
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants