-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix: add require.main
when using isolateModules
#10621
Conversation
mainModule
instance variable to runtimemainModule
instance variable to runtime
mainModule
instance variable to runtimerequire.main
when using isolateModules
Thanks again, @flozender! |
In 26.5.3 |
I should clarify, in the test file
|
Here is a code sandbox reproduction: |
@jsg2021 I'm able to run it with no issues locally, I don't know what's happening on CodeSandbox. Have you tried running it locally? |
I created it after local tests started failing. |
Are there environment vars in your local env set that effect jest? |
No, I don't think so. |
@jsg2021 Looks like CodeSandbox doesn't work on |
I have had a really hard time pinning jest to any minor version... the |
@flozender So, its still failing because all the jest dependencies of the |
Interesting. I downloaded the code from the sandbox and ran it, passed... so that made me compare configs, and I found the code fails if I add this to the package.json: "jest": {
"resetModules": true
} |
Interesting, maybe you should open an issue for this! |
My pre-existing tests still pass on 26.4.2 (I have a package-lock) but fail on 26.5.3. My jest config does not enable that flag. |
Can confirm it fails on using |
@jsg2021 here's a sample repo you can add to the issue: https://github.com/flozender/jest-resetModules |
Done! #10625 |
@jsg2021 are you able to put together a repository showing your failure? Even if it's a work project you should be able to copy over the lockfile and just delete everything not |
Ah! thanks for the tip @SimenB! Here is a repo that exemplifies my passing state: Side note: I figured out why it looked like I had Anyways... the test cases we were trying to make before were directly importing the module... I didn't think that mattered, so I didn't think anything of it, but I added a redirection layer and the tests started passing for that... they still fail for the direct case and probably shouldn't... so if you delete the lock and install latest, all tests fail. |
@SimenB I think so. That repo will reproduce my exact issue where it was passing with the lock file, but broken when updating. It also has the original attempt to reproduce the issue where we simply wrote a test that inspected |
Sounds good, thanks! @flozender wanna send a PR for the extra test cases? 😀 |
@flozender I saw that, but the indirect case is missing for each variation. |
The indirect case is where the test verifies that |
Oh, I see. I'll send in a PR shortly, thanks. |
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
Aims to close #9470.
Modules required through
jest.isolateModules()
do not have arequire.main
property on them.I have attached a
mainModule
variable to the runtime instance. It will benull
at first, and the first module seen will be assigned to themainModule
variable. Required modules will get themainModule
attached to them asmain
.Test plan
Emulate the scenario from the issue as a test.