Do not include from information when its not valid
#5972
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thanks! Mind including the integration test from #5241 as well? |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
FWIW, I think this change makes sense 🙂 If you rebase on master, it will fix an issue with CI which will crop up when you fix the flow errors currently reported |
|
There's an integration test added in #5241. Could you copy it over and include it in this PR? |
If you include the information then when the _execModule routine loads the module without a from context then it will incorrectly setup a circular dependency by declaring the parent is itself. By checking for a module name and not including that information when passed in, the issue is avoided.
Done at the request of @SimenB from PR jestjs#5972
stieg
force-pushed
the
jest_parent_loop_fix
branch
from
1c087a8 to
89d612a
Compare
|
Right... rebased on master; included those integration tests at your request, fixed linter and babel errors... only a couple of failures locally but they appear unrelated. Over to you @SimenB |
|
Local failures are probably missing mercurial (hg) installation, seeing as CI passed :) |
Codecov Report
@@ Coverage Diff @@
## master #5972 +/- ##
==========================================
+ Coverage 64.34% 64.35% +0.01%
==========================================
Files 217 217
Lines 8288 8291 +3
Branches 4 3 -1
==========================================
+ Hits 5333 5336 +3
Misses 2954 2954
Partials 1 1
Continue to review full report at Codecov.
|
|
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. |
If you include the information then when the _execModule routine loads the
module without a from context then it will incorrectly setup a circular
dependency by declaring the parent is itself. By checking for a module name
and not including that information when passed in, the issue is avoided.
Summary
Fixes #5235
Test plan
Before I go through and finish this up I want someone with more knowledge of this project to review this (arguably trivial) change and ensure it makes sense. I have a headache from trying to figure all this out and AFAICT this is the best solution to solve the issue.
That said... its simple enough to load a mock test and trace up to the parent module and ensure it is null.