Skip to content

module: use CJS conditions for hook-loaded require#63722

Open
bobu-putheeckal wants to merge 1 commit into
nodejs:mainfrom
bobu-putheeckal:codex-fix-51327
Open

module: use CJS conditions for hook-loaded require#63722
bobu-putheeckal wants to merge 1 commit into
nodejs:mainfrom
bobu-putheeckal:codex-fix-51327

Conversation

@bobu-putheeckal
Copy link
Copy Markdown

This fixes require() / require.resolve() inside a CommonJS module loaded from async loader hooks so async resolve hooks receive CommonJS package export conditions instead of ESM import conditions.

Before this change, the synthetic CommonJS require.resolve() path called through the ESM loader with default ESM conditions, so an async resolve hook saw conditions like:

["node","import","module-sync","node-addons"]

After this change, requests originating from require() in hook-loaded CommonJS pass the CJS conditions array through the sync resolution path and across the async loader worker boundary. The added regression test asserts that the async resolve hook sees require and does not see import.

Verification:

$ node --version && node test/module-hooks/test-async-loader-hooks-cjs-require-conditions.mjs
v25.6.1
AssertionError [ERR_ASSERTION]: Conditions should include "require": ["node","import","module-sync","node-addons"]
$ out/Release/node --version && out/Release/node test/module-hooks/test-async-loader-hooks-cjs-require-conditions.mjs
v27.0.0-pre
# passes
$ python3 tools/test.py -J --mode=release module-hooks/test-async-loader-hooks-cjs-require-conditions
[00:00|% 100|+   1|-   0]: Done
All tests passed.
$ python3 tools/test.py -J --mode=release module-hooks/test-module-hooks-resolve-require-resolve-loaded-with-source
[00:00|% 100|+   1|-   0]: Done
All tests passed.
$ node tools/eslint/node_modules/eslint/bin/eslint.js lib/internal/modules/esm/loader.js lib/internal/modules/esm/hooks.js lib/internal/modules/esm/translators.js test/module-hooks/test-async-loader-hooks-cjs-require-conditions.mjs
# passes

The release build was run from a temporary copy at /tmp/node-codex-src because the original checkout path contains spaces, which breaks the generated Makefile compiler commands on this machine.

Fixes: #51327

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Jun 3, 2026
Pass CommonJS package export conditions through synchronous resolution
that blocks on async loader hooks when the request originates from
require() in a CommonJS module loaded by module.register().

This keeps async resolve hooks from seeing ESM import conditions for
require.resolve().

Fixes: nodejs#51327
Signed-off-by: Bob Put <bobu.work@gmail.com>
import assert from 'node:assert';
import { register } from 'node:module';

const loader = `
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this in the fixtures folder instead of making it inline?

Also please add a test for module.registerHooks - module.register has been deprecated and will be removed soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hooks loading commonjs files uses import condition for require()

3 participants