modules: fix sync hook short-circuit in require() in imported CJS by joyeecheung · Pull Request #62920 · nodejs/node

joyeecheung · 2026-04-23T22:07:13Z

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 · 2026-04-23T22:07:18Z

Review requested:

@nodejs/loaders

joyeecheung · 2026-04-23T22:46:05Z

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

codecov · 2026-04-24T00:07:06Z

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 51 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 · 2026-05-01T13:45:32Z

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

nodejs-github-bot · 2026-05-01T14:48:45Z

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

DivyanshuX9 · 2026-05-01T16:16:46Z

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

nodejs-github-bot · 2026-05-01T16:34:45Z

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

joyeecheung · 2026-05-01T16:35:23Z

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 · 2026-05-01T22:31:26Z

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 · 2026-05-02T09:34:51Z

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

DivyanshuX9 · 2026-05-02T09:44:31Z

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 ;)

- 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. Signed-off-by: Joyee Cheung <joyeec9h3@gmail.com>

joyeecheung · 2026-05-02T18:20:17Z

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

joyeecheung · 2026-05-02T18:22:35Z

@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 · 2026-05-06T13:12:38Z

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

nodejs-github-bot · 2026-05-06T20:42:55Z

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

nodejs-github-bot · 2026-05-06T22:48:42Z

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

nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. labels Apr 23, 2026

joyeecheung force-pushed the fix-hooks branch from d86b142 to 7630a01 Compare April 23, 2026 22:45

joyeecheung force-pushed the fix-hooks branch 3 times, most recently from 1ef6e6e to 53673f3 Compare April 24, 2026 22:59

joyeecheung mentioned this pull request May 1, 2026

CJS module customized by synchronous customization hooks uses synthetic require with any use of --loader #63060

Open

joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. review wanted PRs that need reviews. and removed request-ci Add this label to start a Jenkins CI on a PR. labels May 1, 2026

ShogunPanda approved these changes May 1, 2026

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label May 1, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 1, 2026

joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label May 1, 2026

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 1, 2026

mcollina approved these changes May 2, 2026

View reviewed changes

gurgunday approved these changes May 2, 2026

View reviewed changes

joyeecheung force-pushed the fix-hooks branch from 53673f3 to 539c680 Compare May 2, 2026 18:09

joyeecheung removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label May 4, 2026

ShogunPanda approved these changes May 6, 2026

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 6, 2026

Uh oh!

Conversation

joyeecheung commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 23, 2026

Uh oh!

joyeecheung commented Apr 23, 2026

Uh oh!

codecov Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented May 1, 2026

Uh oh!

nodejs-github-bot commented May 1, 2026

Uh oh!

DivyanshuX9 commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented May 1, 2026

Uh oh!

joyeecheung commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented May 1, 2026

PR-URL: #62920 Fixes: #63060 Reviewed-By: Paolo Insogna <paolo@cowtech.it>

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

DivyanshuX9 commented May 2, 2026

Uh oh!

DivyanshuX9 commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented May 2, 2026

Uh oh!

joyeecheung commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented May 6, 2026

Uh oh!

nodejs-github-bot commented May 6, 2026

Uh oh!

nodejs-github-bot commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

joyeecheung commented Apr 23, 2026 •

edited

Loading

codecov Bot commented Apr 24, 2026 •

edited

Loading

DivyanshuX9 commented May 1, 2026 •

edited

Loading

joyeecheung commented May 1, 2026 •

edited

Loading

PR-URL: #62920
Fixes: #63060
Reviewed-By: Paolo Insogna <paolo@cowtech.it>

DivyanshuX9 commented May 2, 2026 •

edited

Loading

joyeecheung commented May 2, 2026 •

edited

Loading