modules: fix sync hook short-circuit in require() in imported CJS

[joyeecheung]

• For imported CJS, if it's not customized by asynchronous hooks, make sure it won't use the quirky re-invented require in all cases.
• When the imported CJS module is customized by synchronous hooks, in the synthetic module evalutation step, avoid calling the respective default step again.
• Make the branching of loadCJSModuleWithModuleLoad() and loadCJSModuleWithSpecialRequire() more explicit, and fold the tentative fs read in the 'commonjs' translator into the share createCJSModuleWrap() helper instead of checking it twice in the same path.

Refs: #55808 (comment)
Fixes: #63060

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[joyeecheung]

cc @mcollina as #55808 (comment) referenced https://github.com/platformatic/vfs

[codecov]

Codecov Report

❌ Patch coverage is 98.66071% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.65%. Comparing base (13e90d0) to head (539c680).
⚠️ Report is 52 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/test_runner/mock/mock.js	87.50%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62920      +/-   ##
==========================================
- Coverage   91.49%   89.65%   -1.84%     
==========================================
  Files         360      712     +352     
  Lines      151674   220633   +68959     
  Branches    23919    42293   +18374     
==========================================
+ Hits       138772   197815   +59043     
- Misses      12625    14662    +2037     
- Partials      277     8156    +7879     

Files with missing lines	Coverage Δ
lib/internal/modules/cjs/loader.js	98.27% <100.00%> (+19.07%)	⬆️
lib/internal/modules/esm/load.js	91.40% <ø> (+7.70%)	⬆️
lib/internal/modules/esm/loader.js	99.71% <100.00%> (+40.24%)	⬆️
lib/internal/modules/esm/translators.js	97.51% <100.00%> (+5.99%)	⬆️
lib/internal/test_runner/mock/mock.js	98.54% <87.50%> (+0.22%)	⬆️

... and 471 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/73057/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/73058/

[DivyanshuX9]

Why still some test fails is it a issue in code or it just cope(some infra issue) ?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/73060/

[joyeecheung]

It's just regular flake. In the past ~8 years it would be miracle for a PR to pass the CI fully without a flake 😬 https://github.com/nodejs/reliability/issues

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/62920
✔  Done loading data for nodejs/node/pull/62920
----------------------------------- PR info ------------------------------------
Title      modules: fix sync hook short-circuit in require() in imported CJS (#62920)
Author     Joyee Cheung <joyeec9h3@gmail.com> (@joyeecheung)
Branch     joyeecheung:fix-hooks -> nodejs:main
Labels     module, esm, needs-ci, review wanted
Commits    1
 - module: fix sync hook short-circuit in require() in imported CJS
Committers 1
 - Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/62920
Fixes: https://github.com/nodejs/node/issues/63060
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/62920
Fixes: https://github.com/nodejs/node/issues/63060
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 23 Apr 2026 22:07:13 GMT
   ✔  Approvals: 1
   ✔  - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/62920#pullrequestreview-4211272383
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-05-01T16:34:45Z: https://ci.nodejs.org/job/node-test-pull-request/73060/
- Querying data for job/node-test-pull-request/73060/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 62920
From https://github.com/nodejs/node
 * branch                  refs/pull/62920/merge -> FETCH_HEAD
✔  Fetched commits as 9c0101860e77..53673f349d9a
--------------------------------------------------------------------------------
Auto-merging lib/internal/modules/cjs/loader.js
Auto-merging lib/internal/modules/esm/loader.js
[main a31ccfd3be] module: fix sync hook short-circuit in require() in imported CJS
 Author: Joyee Cheung <joyeec9h3@gmail.com>
 Date: Sat Apr 18 22:10:36 2026 +0100
 6 files changed, 265 insertions(+), 89 deletions(-)
 create mode 100644 test/module-hooks/test-module-hooks-load-import-cjs-custom-source.js
   ✔  Patches applied
--------------------------------------------------------------------------------
--------------------------------- New Message ----------------------------------
module: fix sync hook short-circuit in require() in imported CJS

For imported CJS, if it's not customized by asynchronous hooks,

make sure it won't use the quirky re-invented require in all

cases.
When the imported CJS module is customized by synchronous hooks,

in the synthetic module evalutation step, avoid calling the

respective default step again.
Make the branching of loadCJSModuleWithModuleLoad() and

loadCJSModuleWithSpecialRequire() more explicit, and fold

the tentative fs read in the 'commonjs' translator into the

share createCJSModuleWrap() helper instead of checking it

twice in the same path.

PR-URL: #62920

Fixes: #63060

Reviewed-By: Paolo Insogna <paolo@cowtech.it>
[main ec616a5410] module: fix sync hook short-circuit in require() in imported CJS

Author: Joyee Cheung <joyeec9h3@gmail.com>

Date: Sat Apr 18 22:10:36 2026 +0100

6 files changed, 265 insertions(+), 89 deletions(-)

create mode 100644 test/module-hooks/test-module-hooks-load-import-cjs-custom-source.js

✖  ec616a5410d84de7daae16810ac0f22f8c70a933

✔  0:0      no Assisted-by metadata                   assisted-by-is-trailer

✔  0:0      no Co-authored-by metadata                co-authored-by-is-trailer

✔  14:7     Valid fixes URL.                          fixes-url

✔  0:0      blank line after title                    line-after-title

✔  0:0      line-lengths are valid                    line-length

✔  0:0      metadata is at end of message             metadata-end

✔  13:8     PR-URL is valid.                          pr-url

✔  0:0      reviewers are valid                       reviewers

✖  0:0      Commit must have a "Signed-off-by" trailer signed-off-by

✔  0:0      valid subsystems                          subsystem

✔  0:0      Title is formatted correctly.             title-format

⚠  0:50     Title should be <= 50 columns.            title-length
ℹ  Please fix the commit message and try again.

Please manually ammend the commit message, by running

git commit --amend

Once commit message is fixed, finish the landing command running

git node land --continue

https://github.com/nodejs/node/actions/runs/25235870883

[mcollina]

lgtm

[DivyanshuX9]

I think the issue related must be closed #63060 by now.

[DivyanshuX9]

Btw @joyeecheung , should i look into the flaky test cases and try resolving them so that after 8 year someone might get the miracle done ;)

[joyeecheung]

Rebased and added the signed-off-by field. @mcollina @ShogunPanda @gurgunday can you take a look again? Thanks!

[joyeecheung]

@DivyanshuX9 Help is welcome! See https://github.com/nodejs/reliability#protocols-in-improving-ci-reliability on how to address the flakes. The issue with the flakes is not that nobody knows how to fix them, but that usually nobody has the time to do it, or even if they do for a short while, they won't do it every day/week for years and flakes are a never-ending chase. It might be green for a few days and then it always come back when nobody keeps actively gardening the CI.

[joyeecheung]

Ping @mcollina @ShogunPanda @gurgunday - after amending the commit with the signoff this needs another approval.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/73213/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/73217/

[nodejs-github-bot]

Landed in 4f44c6f