module: use CJS conditions for hook-loaded require#63722
Open
bobu-putheeckal wants to merge 1 commit into
Open
module: use CJS conditions for hook-loaded require#63722bobu-putheeckal wants to merge 1 commit into
bobu-putheeckal wants to merge 1 commit into
Conversation
Collaborator
|
Review requested:
|
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>
551c77a to
0f4c3dd
Compare
joyeecheung
reviewed
Jun 3, 2026
| import assert from 'node:assert'; | ||
| import { register } from 'node:module'; | ||
|
|
||
| const loader = ` |
Member
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes
require()/require.resolve()inside a CommonJS module loaded from async loader hooks so asyncresolvehooks 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 asyncresolvehook saw conditions like: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 asyncresolvehook seesrequireand does not seeimport.Verification:
The release build was run from a temporary copy at
/tmp/node-codex-srcbecause the original checkout path contains spaces, which breaks the generated Makefile compiler commands on this machine.Fixes: #51327