[v20.x] backport require(esm) bugfixes and improvements#59504

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 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]>

If the macros are used as ERR_*(isolate, message) or THROW_ERR_*(isolate, message) with a single string argument, do run formatter on the message, and allow the caller to pass in a message directly with characters that would otherwise need escaping if used as format string unconditionally. PR-URL: nodejs#57126 Refs: https://github.com/fisker/prettier-issue-17139 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

- Improve the error message that shows up when there is a race from doing require(esm) and import(esm) at the same time. - Improve error message of ERR_REQUIRE_ASYNC_MODULE by showing parent and target file names, if available. Drive-by: split the require(tla) tests since we are modifying the tests already. PR-URL: nodejs#57126 Refs: https://github.com/fisker/prettier-issue-17139 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

This handles two cases caused by using Promise.all() with multiple dynamic import() that can make an asynchronously linked module job that has finished/failed linking but has not yet started actual evaluation appear in the load cache when another require request is in turn to handle it. - When the cached async job has finished linking but has not started its evaluation because the async run() task would be later in line, start the evaluation from require() with runSync(). - When the cached async job have already encountered a linking error that gets wrapped into a rejection, but is still later in line to throw on it, just unwrap and throw the linking error from require(). PR-URL: nodejs#57187Fixes: nodejs#57172 Refs: shufo/prettier-plugin-blade#311 Refs: https://github.com/fisker/prettier-plugin-blade-311 Refs: mochajs/mocha#5290 Refs: https://github.com/JoshuaKGoldberg/repros/tree/mocha-missing-module-cyclic Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

Fixes: nodejs#57451 PR-URL: nodejs#57453 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>

When require(esm) encounters a cached module job that is instantiated but not yet evaluated, run the evaluation. This catches an edge case previously missed in nodejs#57187. PR-URL: nodejs#58067Fixes: nodejs#58061 Reviewed-By: Jacob Smith <[email protected]>

PR-URL: nodejs#56491 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Jacob Smith <[email protected]>

When --import is used, the ESM loader is used to handle even pure CJS entry points, and it can run into CJS module facades in the evaluating state when the parent CJS module is being evaluated. In this case it should be allowed, since the ESM <-> CJS cycles that are meant to be disallowed (for the time being) should already be detected before evaluation and wouldn't get here, and CJS <-> CJS cycles are fine. PR-URL: nodejs#58598Fixes: nodejs#58515 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]>

Re-evaluating an errored ESM should lead to rejecting the rejection again - this is also the case when importing it twice. In the case of retrying with require after import, just throw the cached error. Drive-by: add some debug logs. PR-URL: nodejs#58957Fixes: nodejs#58945 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

PR-URL: #56491 Backport-PR-URL: #59504 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Refs: #52697

When --import is used, the ESM loader is used to handle even pure CJS entry points, and it can run into CJS module facades in the evaluating state when the parent CJS module is being evaluated. In this case it should be allowed, since the ESM <-> CJS cycles that are meant to be disallowed (for the time being) should already be detected before evaluation and wouldn't get here, and CJS <-> CJS cycles are fine. PR-URL: #58598 Backport-PR-URL: #59504Fixes: #58515 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Refs: #52697

Re-evaluating an errored ESM should lead to rejecting the rejection again - this is also the case when importing it twice. In the case of retrying with require after import, just throw the cached error. Drive-by: add some debug logs. PR-URL: #58957 Backport-PR-URL: #59504Fixes: #58945 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Refs: #52697

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 require(esm) handling (when it's disabled). PR-URL: #55590 Backport-PR-URL: #59504 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]> Refs: #52697

If the macros are used as ERR_*(isolate, message) or THROW_ERR_*(isolate, message) with a single string argument, do run formatter on the message, and allow the caller to pass in a message directly with characters that would otherwise need escaping if used as format string unconditionally. PR-URL: #57126 Backport-PR-URL: #59504 Refs: https://github.com/fisker/prettier-issue-17139 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Refs: #52697

- Improve the error message that shows up when there is a race from doing require(esm) and import(esm) at the same time. - Improve error message of ERR_REQUIRE_ASYNC_MODULE by showing parent and target file names, if available. Drive-by: split the require(tla) tests since we are modifying the tests already. PR-URL: #57126 Backport-PR-URL: #59504 Refs: https://github.com/fisker/prettier-issue-17139 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Refs: #52697

This handles two cases caused by using Promise.all() with multiple dynamic import() that can make an asynchronously linked module job that has finished/failed linking but has not yet started actual evaluation appear in the load cache when another require request is in turn to handle it. - When the cached async job has finished linking but has not started its evaluation because the async run() task would be later in line, start the evaluation from require() with runSync(). - When the cached async job have already encountered a linking error that gets wrapped into a rejection, but is still later in line to throw on it, just unwrap and throw the linking error from require(). PR-URL: #57187 Backport-PR-URL: #59504Fixes: #57172 Refs: shufo/prettier-plugin-blade#311 Refs: https://github.com/fisker/prettier-plugin-blade-311 Refs: mochajs/mocha#5290 Refs: https://github.com/JoshuaKGoldberg/repros/tree/mocha-missing-module-cyclic Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Refs: #52697

Fixes: #57451 PR-URL: #57453 Backport-PR-URL: #59504 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Refs: #52697

When require(esm) encounters a cached module job that is instantiated but not yet evaluated, run the evaluation. This catches an edge case previously missed in #57187. PR-URL: #58067 Backport-PR-URL: #59504Fixes: #58061 Reviewed-By: Jacob Smith <[email protected]> Refs: #52697

PR-URL: #56491 Backport-PR-URL: #59504 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Refs: #52697

When --import is used, the ESM loader is used to handle even pure CJS entry points, and it can run into CJS module facades in the evaluating state when the parent CJS module is being evaluated. In this case it should be allowed, since the ESM <-> CJS cycles that are meant to be disallowed (for the time being) should already be detected before evaluation and wouldn't get here, and CJS <-> CJS cycles are fine. PR-URL: #58598 Backport-PR-URL: #59504Fixes: #58515 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Refs: #52697

Re-evaluating an errored ESM should lead to rejecting the rejection again - this is also the case when importing it twice. In the case of retrying with require after import, just throw the cached error. Drive-by: add some debug logs. PR-URL: #58957 Backport-PR-URL: #59504Fixes: #58945 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Refs: #52697

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 require(esm) handling (when it's disabled). PR-URL: #55590 Backport-PR-URL: #59504 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]> Refs: #52697

If the macros are used as ERR_*(isolate, message) or THROW_ERR_*(isolate, message) with a single string argument, do run formatter on the message, and allow the caller to pass in a message directly with characters that would otherwise need escaping if used as format string unconditionally. PR-URL: #57126 Backport-PR-URL: #59504 Refs: https://github.com/fisker/prettier-issue-17139 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Refs: #52697

- Improve the error message that shows up when there is a race from doing require(esm) and import(esm) at the same time. - Improve error message of ERR_REQUIRE_ASYNC_MODULE by showing parent and target file names, if available. Drive-by: split the require(tla) tests since we are modifying the tests already. PR-URL: #57126 Backport-PR-URL: #59504 Refs: https://github.com/fisker/prettier-issue-17139 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Refs: #52697

This handles two cases caused by using Promise.all() with multiple dynamic import() that can make an asynchronously linked module job that has finished/failed linking but has not yet started actual evaluation appear in the load cache when another require request is in turn to handle it. - When the cached async job has finished linking but has not started its evaluation because the async run() task would be later in line, start the evaluation from require() with runSync(). - When the cached async job have already encountered a linking error that gets wrapped into a rejection, but is still later in line to throw on it, just unwrap and throw the linking error from require(). PR-URL: #57187 Backport-PR-URL: #59504Fixes: #57172 Refs: shufo/prettier-plugin-blade#311 Refs: https://github.com/fisker/prettier-plugin-blade-311 Refs: mochajs/mocha#5290 Refs: https://github.com/JoshuaKGoldberg/repros/tree/mocha-missing-module-cyclic Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Refs: #52697

Fixes: #57451 PR-URL: #57453 Backport-PR-URL: #59504 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Refs: #52697

When require(esm) encounters a cached module job that is instantiated but not yet evaluated, run the evaluation. This catches an edge case previously missed in #57187. PR-URL: #58067 Backport-PR-URL: #59504Fixes: #58061 Reviewed-By: Jacob Smith <[email protected]> Refs: #52697

PR-URL: #56491 Backport-PR-URL: #59504 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Refs: #52697

When --import is used, the ESM loader is used to handle even pure CJS entry points, and it can run into CJS module facades in the evaluating state when the parent CJS module is being evaluated. In this case it should be allowed, since the ESM <-> CJS cycles that are meant to be disallowed (for the time being) should already be detected before evaluation and wouldn't get here, and CJS <-> CJS cycles are fine. PR-URL: #58598 Backport-PR-URL: #59504Fixes: #58515 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Refs: #52697

Re-evaluating an errored ESM should lead to rejecting the rejection again - this is also the case when importing it twice. In the case of retrying with require after import, just throw the cached error. Drive-by: add some debug logs. PR-URL: #58957 Backport-PR-URL: #59504Fixes: #58945 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Refs: #52697

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 require(esm) handling (when it's disabled). PR-URL: #55590 Backport-PR-URL: #59504 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]> Refs: #52697

If the macros are used as ERR_*(isolate, message) or THROW_ERR_*(isolate, message) with a single string argument, do run formatter on the message, and allow the caller to pass in a message directly with characters that would otherwise need escaping if used as format string unconditionally. PR-URL: #57126 Backport-PR-URL: #59504 Refs: https://github.com/fisker/prettier-issue-17139 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Refs: #52697

- Improve the error message that shows up when there is a race from doing require(esm) and import(esm) at the same time. - Improve error message of ERR_REQUIRE_ASYNC_MODULE by showing parent and target file names, if available. Drive-by: split the require(tla) tests since we are modifying the tests already. PR-URL: #57126 Backport-PR-URL: #59504 Refs: https://github.com/fisker/prettier-issue-17139 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Refs: #52697

This handles two cases caused by using Promise.all() with multiple dynamic import() that can make an asynchronously linked module job that has finished/failed linking but has not yet started actual evaluation appear in the load cache when another require request is in turn to handle it. - When the cached async job has finished linking but has not started its evaluation because the async run() task would be later in line, start the evaluation from require() with runSync(). - When the cached async job have already encountered a linking error that gets wrapped into a rejection, but is still later in line to throw on it, just unwrap and throw the linking error from require(). PR-URL: #57187 Backport-PR-URL: #59504Fixes: #57172 Refs: shufo/prettier-plugin-blade#311 Refs: https://github.com/fisker/prettier-plugin-blade-311 Refs: mochajs/mocha#5290 Refs: https://github.com/JoshuaKGoldberg/repros/tree/mocha-missing-module-cyclic Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Refs: #52697

Fixes: #57451 PR-URL: #57453 Backport-PR-URL: #59504 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Refs: #52697

When require(esm) encounters a cached module job that is instantiated but not yet evaluated, run the evaluation. This catches an edge case previously missed in #57187. PR-URL: #58067 Backport-PR-URL: #59504Fixes: #58061 Reviewed-By: Jacob Smith <[email protected]> Refs: #52697

PR-URL: #56491 Backport-PR-URL: #59504 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Refs: #52697

When --import is used, the ESM loader is used to handle even pure CJS entry points, and it can run into CJS module facades in the evaluating state when the parent CJS module is being evaluated. In this case it should be allowed, since the ESM <-> CJS cycles that are meant to be disallowed (for the time being) should already be detected before evaluation and wouldn't get here, and CJS <-> CJS cycles are fine. PR-URL: #58598 Backport-PR-URL: #59504Fixes: #58515 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ethan Arrowood <[email protected]> Refs: #52697

Re-evaluating an errored ESM should lead to rejecting the rejection again - this is also the case when importing it twice. In the case of retrying with require after import, just throw the cached error. Drive-by: add some debug logs. PR-URL: #58957 Backport-PR-URL: #59504Fixes: #58945 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Refs: #52697

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. labels Aug 17, 2025

mcollina approved these changes Aug 17, 2025
View reviewed changes

joyeecheung force-pushed the backport-require-esm-patches branch from 2bd8221 to c059eb2Compare August 18, 2025 12:06

marco-ippolito force-pushed the v20.x-staging branch from a2554ef to 5a3b04bCompare August 18, 2025 12:34

marco-ippolito approved these changes Aug 18, 2025
View reviewed changes

marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 18, 2025

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 18, 2025

joyeecheung force-pushed the backport-require-esm-patches branch from c059eb2 to a65b63eCompare August 19, 2025 16:43

marco-ippolito force-pushed the v20.x-staging branch from 5a3b04b to d65a24cCompare August 20, 2025 09:40

joyeecheungand others added 9 commits August 20, 2025 11:41

module: clarify cjs global-like error on ModuleJobSync
10c4efd
PR-URL: nodejs#56491 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Jacob Smith <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[v20.x] backport require(esm) bugfixes and improvements#59504

[v20.x] backport require(esm) bugfixes and improvements #59504

Uh oh!

joyeecheung commented Aug 17, 2025

Uh oh!

nodejs-github-bot commented Aug 17, 2025

Uh oh!

joyeecheung commented Aug 17, 2025

Uh oh!

nodejs-github-bot commented Aug 17, 2025

Uh oh!

mcollina left a comment

Uh oh!

nodejs-github-bot commented Aug 18, 2025

Uh oh!

marco-ippolito commented Aug 18, 2025

Uh oh!

joyeecheung commented Aug 18, 2025

Uh oh!

nodejs-github-bot commented Aug 18, 2025

Uh oh!

marco-ippolito commented Aug 19, 2025

Uh oh!

joyeecheung commented Aug 19, 2025

Uh oh!

nodejs-github-bot commented Aug 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

[v20.x] backport require(esm) bugfixes and improvements#59504

[v20.x] backport require(esm) bugfixes and improvements #59504

Uh oh!

Conversation

joyeecheung commented Aug 17, 2025

Uh oh!

nodejs-github-bot commented Aug 17, 2025

Uh oh!

joyeecheung commented Aug 17, 2025

Uh oh!

nodejs-github-bot commented Aug 17, 2025

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Aug 18, 2025

Uh oh!

marco-ippolito commented Aug 18, 2025

Uh oh!

joyeecheung commented Aug 18, 2025

Uh oh!

nodejs-github-bot commented Aug 18, 2025

Uh oh!

marco-ippolito commented Aug 19, 2025

Uh oh!

joyeecheung commented Aug 19, 2025

Uh oh!

nodejs-github-bot commented Aug 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants