-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
module: unify TypeScript and .mjs handling in CommonJS #55590
Conversation
This refactors the CommonJS loading a bit to create a center point that handles source loading (`loadSource`) and make format detection more consistent to pave the way for future synchronous hooks. - Handle .mjs in the .js handler, similar to how .cjs has been handled. - Generate the legacy ERR_REQUIRE_ESM in a getRequireESMError() for both .mts and require(esm) handling (when it's disabled).
Review requested:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55590 +/- ##
=======================================
Coverage 88.43% 88.43%
=======================================
Files 654 654
Lines 187674 187698 +24
Branches 36124 36135 +11
=======================================
+ Hits 165969 165997 +28
+ Misses 14952 14944 -8
- Partials 6753 6757 +4
|
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.
lgtm
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.
LGTM
Landed in d080f0d |
This refactors the CommonJS loading a bit to create a center point that handles source loading (`loadSource`) and make format detection more consistent to pave the way for future synchronous hooks. - Handle .mjs in the .js handler, similar to how .cjs has been handled. - Generate the legacy ERR_REQUIRE_ESM in a getRequireESMError() for both .mts and require(esm) handling (when it's disabled). PR-URL: #55590 Refs: nodejs/loaders#198 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
This refactors the CommonJS loading a bit to create a center point that handles source loading (`loadSource`) and make format detection more consistent to pave the way for future synchronous hooks. - Handle .mjs in the .js handler, similar to how .cjs has been handled. - Generate the legacy ERR_REQUIRE_ESM in a getRequireESMError() for both .mts and require(esm) handling (when it's disabled). PR-URL: nodejs#55590 Refs: nodejs/loaders#198 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
This refactors the CommonJS loading a bit to create a center point that handles source loading (`loadSource`) and make format detection more consistent to pave the way for future synchronous hooks. - Handle .mjs in the .js handler, similar to how .cjs has been handled. - Generate the legacy ERR_REQUIRE_ESM in a getRequireESMError() for both .mts and require(esm) handling (when it's disabled). PR-URL: nodejs#55590 Refs: nodejs/loaders#198 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]>
This commit does not land cleanly on |
This refactors the CommonJS loading a bit to create a center point that handles source loading (
loadSource
) and make format detection more consistent to pave the way for future synchronous hooks.Refs: nodejs/loaders#198