fix(jest-haste-map): don't throw on missing mapper in Node crawler #8558
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8558 +/- ##
==========================================
+ Coverage 60.57% 60.58% +0.01%
==========================================
Files 269 269
Lines 11054 11052 -2
Branches 2696 2694 -2
==========================================
Hits 6696 6696
+ Misses 3772 3771 -1
+ Partials 586 585 -1
Continue to review full report at Codecov.
|
thymikee
changed the title
thymikee
changed the title
| @@ -139,10 +139,6 @@ export = function nodeCrawl( | |||
| removedFiles: FileData; | |||
| hasteMap: InternalHasteMap; | |||
| }> { | |||
| if (options.mapper) { | |||
There was a problem hiding this comment.
I think this makes sense, not sure why it'd need to throw. Maybe someone has a good reason, though 😀
There was a problem hiding this comment.
Yea. There are other options we ignore in Node crawler (like computeSha1). Maybe it was some kind of development helper that wasn't removed? Anyway, it's gone now :P
|
what's the interplay between this and the referenced metro bug 418 linked above? It just bit me and I'm trying to figure out if this is an unreleased change that needs to be incorporated in a release of jest here, then referenced in metro, or if it's already released here and metro just needs to update? @thymikee you merged this, is it released? @motiz88 this seems needed in metro but I'm not even sure how to propose it... |
|
Update jest-haste-map in your lock file |
|
I don't like messing around in lock files :-), but I'm okay with patch-package It will tell me when it's released, and I can build for now (if anyone else needs this, just install patch-package, make a patches folder and drop this in there without the .txt extension (that was needed so github would accept the attachment) and add patch-package as a postinstall hook The question of how to get it integrated is open though ? with a patch I can move alone for now though |
|
Just remove it from lock file and yarn install again :p |
|
Ah! I see what you mean. Would it be the same to say that you could just delete the whole lock file and it works, or do you have to install, then delete just jest haste map from the lock file, and install again? |
|
Deleting just the |
|
Metro should probably update, though? |
|
Opened up facebook/metro#432 |
|
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. |
Summary
The
mapperoption passed to HasteMap is not supported in Node crawler, so we shouldn't pass it there. It's likely a cause of a regression in React Native 0.60 RC: facebook/react-native#25241 (comment).The regression was a part of this refactoring: https://github.com/facebook/jest/pull/8056/files#diff-5c5055968e3661a18f280d5e3f10c969L738
Test plan
Added extra assertion to validate that mapper is not passed to Node crawler.
cc @scotthovestadt @arcanis