fix(jest-haste-map): fix missing import statement #8548
Conversation
There was a problem hiding this comment.
Thanks @dy93 :)
Are there any new negatives we should test for because of the additional pattern alternative? Regex looks good but maybe something like import + 'inv1' or others you could think of? Just want to be extra safe because this is a critical/low-level piece of code
|
@jeysal No, I can't come up with any valid js statements that can pass the regular expression but is not mean to import something. |
Co-Authored-By: Tim Seckinger <seckinger.tim@gmail.com>
|
Yeah I agree the regex looks good, anyway few negative test cases are good to have to proof it for future changes :) |
Codecov Report
@@ Coverage Diff @@
## master #8548 +/- ##
==========================================
+ Coverage 60.57% 60.57% +<.01%
==========================================
Files 269 269
Lines 11054 11055 +1
Branches 2696 2695 -1
==========================================
+ Hits 6696 6697 +1
Misses 3772 3772
Partials 586 586
Continue to review full report at Codecov.
|
| @@ -85,7 +90,8 @@ export function extract(code: string): Set<string> { | |||
| code | |||
| .replace(BLOCK_COMMENT_RE, '') | |||
| .replace(LINE_COMMENT_RE, '') | |||
| .replace(IMPORT_OR_EXPORT_RE, addDependency) | |||
| .replace(IMPORT_OR_EXPORT_FROM_RE, addDependency) | |||
| .replace(IMPORT_RE, addDependency) | |||
There was a problem hiding this comment.
how does that impact performance? could we squeeze this check into a single regex pass so we don't have to parse the text once more?
|
@jeysal LGTM |
|
See #8670 (comment), it's implemented there :) |
|
It seems to me to close this PR and follow #8670 ? |
|
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
Fix #8547
Test plan
run
yarn test