-
-
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
feat(resolver): support node:
prefix when loading core modules
#11331
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼
We should probably throw the correct error if |
is this going into the 26.x release or into 27.x? |
27 |
I forgot it before the release, though 😀 Will try to find the time soonish to finish this |
I'll review once you rebase this. |
Oops, only find out now that there is already a PR for this.
There is also this issue #11637 It is all duplicates of this PR, but it is implemented differently. |
packages/jest-runtime/src/index.ts
Outdated
@@ -1327,15 +1327,19 @@ export default class Runtime { | |||
} | |||
|
|||
private _requireCoreModule(moduleName: string) { | |||
if (moduleName === 'process') { | |||
const moduleWithoutNodePrefix = moduleName.startsWith('node:') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may do not strip the prefix if the runtime supports it:
process.version.slice(1, 3) < 16
semver.lt(process.version, 'v16')
ref: https://nodejs.org/api/modules.html#modules_core_modules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
b1aca2c
to
9caa82f
Compare
9caa82f
to
94d1903
Compare
Possible to make the tests pass again and merge that ASAP? Thanks for your work! 😄 |
94d1903
to
5519f58
Compare
5519f58
to
1865950
Compare
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
Ref nodejs/node#37246 (comment)
I haven't bothered to add any node version guard. So even though only v16 supports it inKeeping track of all the changes to node modules is doing my head in 😅require
, Jest will support it across all runtimes. I think that's fine.Closes #11638
Closes #11686
Test plan
Added unit test