Make jest-haste-map compute SHA-1s for excluded files too#6264
Make jest-haste-map compute SHA-1s for excluded files too#6264mjesun merged 1 commit intojestjs:masterfrom mjesun:jest-node-modules
Conversation
| let threw = false; | ||
|
|
||
| try { | ||
| await getSha1({computeSha1: true, filePath: '/i/dont/exist.js'}); |
There was a problem hiding this comment.
I think you can use
await expect(getSha1({computeSha1: true, filePath: '/i/dont/exist.js'})).rejects.toThrow()
packages/jest-haste-map/src/index.js
Outdated
| } | ||
|
|
||
| // $FlowFixMe: checking error code is OK if error comes from "fs". | ||
| if (['ENOENT', 'EACCES'].indexOf(error.code) < 0) { |
There was a problem hiding this comment.
.includes instead of .indexOf?
There was a problem hiding this comment.
That code was moved, but ok :)
There was a problem hiding this comment.
I've just seen it a minute later :D But still why not used explicit API :)
|
I added an integration test too, forcing the Node crawler, which always returns |
| const SkipOnWindows = require('../../scripts/SkipOnWindows'); | ||
| const DIR = path.resolve(os.tmpdir(), 'haste_map_sha1'); | ||
|
|
||
| SkipOnWindows.suite(); |
There was a problem hiding this comment.
maybe let's try without skipping windows
|
Needs a |
|
Rebase + changelog and I think we're good 👍 |
|
Rebase should fix CI. This also should have a changelog entry 😀 |
|
Rebased over |
|
:| WTF did we move the integration tests away? |
|
@mjesun moved them to |
|
Yeah found that 😄 I also moved mine ;) |
Codecov Report
@@ Coverage Diff @@
## master #6264 +/- ##
==========================================
+ Coverage 63.63% 63.72% +0.09%
==========================================
Files 226 226
Lines 8648 8665 +17
Branches 4 3 -1
==========================================
+ Hits 5503 5522 +19
+ Misses 3144 3142 -2
Partials 1 1
Continue to review full report at Codecov.
|
ghost
mentioned this pull request
|
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. |
6 participants
Although Jest itself does not use SHA-1s yet, this part of the code is used by https://github.com/facebook/metro, where provided SHA-1s are used to compute cache keys.
When using
watchmanas a crawler, everything works well, but when using the Node crawler, Metro breaks on the firstnode_modulesfile it finds. This is because the Node crawler does not compute SHA-1s, and leaves this duty to the worker. Becausenode_modulesare not processed to extract dependencies, their SHA-1 was never computed and Metro crashed.This diff introduces a new, lightweight method in
worker.js, which sole mission is to compute the SHA-1. It is called instead ofworkerwhen the file will be dropped from heavy processing, and only when SHA-1s are requested.Note: all tests pass, but I haven't tested this internally yet, I'll comment once I know it fully works.