From 09d7c9ae44a00994060cbd9e47b7f91a382c69f9 Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Fri, 22 May 2026 07:52:24 +0900 Subject: [PATCH 1/2] module: expose requestType to user hooks Add `context.requestType` ('import' | 'require') to both resolve and load hook contexts, in sync (`module.registerHooks`) and async (`module.register`) modes. The value is 'require' for `require()` calls (including the `require(esm)` bridge) and 'import' for `import` statements, dynamic `import()`, and `import.meta.resolve()`. The async load hook also now receives `context.conditions` (previously absent), and conditions are computed from the request type so sync hooks no longer see ESM defaults when the load is on behalf of a `require()`. This lets loaders distinguish a `require()` from an `import` without inspecting the call stack, and gives them the correct package-exports conditions for the request. Refs: https://github.com/nodejs/node/issues/51327 --- doc/api/module.md | 43 ++++++-- lib/internal/modules/cjs/loader.js | 7 +- lib/internal/modules/customization_hooks.js | 24 ++-- lib/internal/modules/esm/hooks.js | 33 +++++- lib/internal/modules/esm/loader.js | 52 ++++++--- lib/internal/modules/esm/translators.js | 4 +- .../test-esm-loader-hooks-request-type.mjs | 58 ++++++++++ .../es-module-loaders/hooks-input.mjs | 9 +- .../es-module-loaders/loader-request-type.mjs | 34 ++++++ .../test-module-hooks-request-type.js | 104 ++++++++++++++++++ 10 files changed, 329 insertions(+), 39 deletions(-) create mode 100644 test/es-module/test-esm-loader-hooks-request-type.mjs create mode 100644 test/fixtures/es-module-loaders/loader-request-type.mjs create mode 100644 test/module-hooks/test-module-hooks-request-type.js diff --git a/doc/api/module.md b/doc/api/module.md index 62780536914f96..ec6877c3a221c3 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -882,11 +882,19 @@ changes: * `specifier` {string} * `context` {Object} - * `conditions` {string\[]} Export conditions of the relevant `package.json` + * `conditions` {string\[]} Export conditions of the relevant `package.json`. + When `requestType` is `'require'`, the array contains CJS conditions + (including `'require'`); otherwise it contains ESM conditions + (including `'import'`). * `importAttributes` {Object} An object whose key-value pairs represent the attributes for the module to import * `parentURL` {string|undefined} The module importing this one, or undefined if this is the Node.js entry point + * `requestType` {string} `'import'` if this resolution is on behalf of an + `import` statement, `import()` expression, or `import.meta.resolve()`; + `'require'` if it is on behalf of a `require()` call (including + `require()` in a CommonJS module reached via the [`require(esm)`][] + bridge). * `nextResolve` {Function} The subsequent `resolve` hook in the chain, or the Node.js default `resolve` hook after the last user-supplied `resolve` hook * `specifier` {string} @@ -970,11 +978,18 @@ changes: * `url` {string} The URL returned by the `resolve` chain * `context` {Object} - * `conditions` {string\[]} Export conditions of the relevant `package.json` + * `conditions` {string\[]} Export conditions of the relevant `package.json`. + When `requestType` is `'require'`, the array contains CJS conditions + (including `'require'`); otherwise it contains ESM conditions + (including `'import'`). * `format` {string|null|undefined} The format optionally supplied by the `resolve` hook chain. This can be any string value as an input; input values do not need to conform to the list of acceptable return values described below. * `importAttributes` {Object} + * `requestType` {string} `'import'` if this load is on behalf of an `import` + statement or `import()` expression; `'require'` if it is on behalf of a + `require()` call (including `require()` in a CommonJS module reached via + the [`require(esm)`][] bridge). * `nextLoad` {Function} The subsequent `load` hook in the chain, or the Node.js default `load` hook after the last user-supplied `load` hook * `url` {string} @@ -1406,11 +1421,19 @@ changes: * `specifier` {string} * `context` {Object} - * `conditions` {string\[]} Export conditions of the relevant `package.json` + * `conditions` {string\[]} Export conditions of the relevant `package.json`. + When `requestType` is `'require'`, the array contains CJS conditions + (including `'require'`); otherwise it contains ESM conditions + (including `'import'`). * `importAttributes` {Object} An object whose key-value pairs represent the attributes for the module to import * `parentURL` {string|undefined} The module importing this one, or undefined if this is the Node.js entry point + * `requestType` {string} `'import'` if this resolution is on behalf of an + `import` statement, `import()` expression, or `import.meta.resolve()`; + `'require'` if it is on behalf of a `require()` call (including + `require()` in a CommonJS module reached via the [`require(esm)`][] + bridge). * `nextResolve` {Function} The subsequent `resolve` hook in the chain, or the Node.js default `resolve` hook after the last user-supplied `resolve` hook * `specifier` {string} @@ -1439,10 +1462,6 @@ The asynchronous version works similarly to the synchronous version, only that t > `require()`. Instead, it receives a URL already fully resolved using the default > CommonJS resolution. -> **Warning** In the CommonJS modules that are customized by the asynchronous customization hooks, -> `require.resolve()` and `require()` will use `"import"` export condition instead of -> `"require"`, which may cause unexpected behaviors when loading dual packages. - ```mjs export async function resolve(specifier, context, nextResolve) { // When calling `defaultResolve`, the arguments can be modified. For example, @@ -1491,11 +1510,18 @@ changes: * `url` {string} The URL returned by the `resolve` chain * `context` {Object} - * `conditions` {string\[]} Export conditions of the relevant `package.json` + * `conditions` {string\[]} Export conditions of the relevant `package.json`. + When `requestType` is `'require'`, the array contains CJS conditions + (including `'require'`); otherwise it contains ESM conditions + (including `'import'`). * `format` {string|null|undefined} The format optionally supplied by the `resolve` hook chain. This can be any string value as an input; input values do not need to conform to the list of acceptable return values described below. * `importAttributes` {Object} + * `requestType` {string} `'import'` if this load is on behalf of an `import` + statement or `import()` expression; `'require'` if it is on behalf of a + `require()` call (including `require()` in a CommonJS module reached via + the [`require(esm)`][] bridge). * `nextLoad` {Function} The subsequent `load` hook in the chain, or the Node.js default `load` hook after the last user-supplied `load` hook * `url` {string} @@ -2072,6 +2098,7 @@ returned object contains the following keys: [`module`]: #the-module-object [`os.tmpdir()`]: os.md#ostmpdir [`register`]: #moduleregisterspecifier-parenturl-options +[`require(esm)`]: modules.md#loading-ecmascript-modules-using-require [`util.TextDecoder`]: util.md#class-utiltextdecoder [accepted final formats]: #accepted-final-formats-returned-by-load [asynchronous `load` hook]: #asynchronous-loadurl-context-nextload diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 65a35299eb6552..610baa3f6fad1f 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1165,7 +1165,7 @@ function resolveForCJSWithHooks(specifier, parent, isMain, internalOptions) { } const resolveResult = resolveWithHooks(specifier, parentURL, /* importAttributes */ undefined, - getCjsConditionsArray(), defaultResolve); + getCjsConditionsArray(), 'require', defaultResolve); const { url, format } = resolveResult; // Convert the URL from the hook chain back to a filename for internal use. const filename = convertURLToCJSFilename(url); @@ -1239,7 +1239,8 @@ function loadBuiltinWithHooks(id, url, format) { debug('loadBuiltinWithHooks ', loadHooks.length, id, url, format); // TODO(joyeecheung): do we really want to invoke the load hook for the builtins? resultFromHook = loadWithHooks(url, format || 'builtin', /* importAttributes */ undefined, - getCjsConditionsArray(), getDefaultLoad(url, id), validateLoadStrict); + getCjsConditionsArray(), 'require', + getDefaultLoad(url, id), validateLoadStrict); if (resultFromHook.format && resultFromHook.format !== 'builtin') { debug('loadBuiltinWithHooks overriding module', id, url, resultFromHook); // Format has been overridden, return result for the caller to continue loading. @@ -1906,7 +1907,7 @@ function loadSource(mod, filename, formatFromNode) { const defaultLoad = getDefaultLoad(mod[kURL], filename); const loadResult = loadWithHooks(mod[kURL], mod[kFormat], /* importAttributes */ undefined, - getCjsConditionsArray(), defaultLoad, validateLoadStrict); + getCjsConditionsArray(), 'require', defaultLoad, validateLoadStrict); // Reset the module properties with load hook results. if (loadResult.format !== undefined) { mod[kFormat] = loadResult.format; diff --git a/lib/internal/modules/customization_hooks.js b/lib/internal/modules/customization_hooks.js index c2579269ec6396..29418cbdaf962c 100644 --- a/lib/internal/modules/customization_hooks.js +++ b/lib/internal/modules/customization_hooks.js @@ -328,12 +328,15 @@ class ModuleResolveContext { * @param {string|undefined} parentURL Parent URL. * @param {ImportAttributes|undefined} importAttributes Import attributes. * @param {string[]} conditions Conditions. + * @param {'import'|'require'} requestType Whether the resolution is being done on behalf of + * a `require()` call (`'require'`) or an `import` statement / `import()` expression / + * `import.meta.resolve()` (`'import'`). */ - constructor(parentURL, importAttributes, conditions) { + constructor(parentURL, importAttributes, conditions, requestType) { this.parentURL = parentURL; this.importAttributes = importAttributes; this.conditions = conditions; - // TODO(joyeecheung): a field to differentiate between require and import? + this.requestType = requestType; } }; @@ -343,11 +346,15 @@ class ModuleLoadContext { * @param {string|undefined} format URL. * @param {ImportAttributes|undefined} importAttributes Import attributes. * @param {string[]} conditions Conditions. + * @param {'import'|'require'} requestType Whether the load is being done on behalf of + * a `require()` call (`'require'`) or an `import` statement / `import()` expression + * (`'import'`). */ - constructor(format, importAttributes, conditions) { + constructor(format, importAttributes, conditions, requestType) { this.format = format; this.importAttributes = importAttributes; this.conditions = conditions; + this.requestType = requestType; } }; @@ -358,13 +365,15 @@ let decoder; * @param {string|undefined} originalFormat * @param {ImportAttributes|undefined} importAttributes * @param {string[]} conditions + * @param {'import'|'require'} requestType See {@link ModuleLoadContext}. * @param {(url: string, context: ModuleLoadContext) => ModuleLoadResult} defaultLoad * @param {(url: string, context: ModuleLoadContext, result: ModuleLoadResult) => ModuleLoadResult} validateLoad * @returns {ModuleLoadResult} */ -function loadWithHooks(url, originalFormat, importAttributes, conditions, defaultLoad, validateLoad) { +function loadWithHooks(url, originalFormat, importAttributes, conditions, requestType, + defaultLoad, validateLoad) { debug('loadWithHooks', url, originalFormat); - const context = new ModuleLoadContext(originalFormat, importAttributes, conditions); + const context = new ModuleLoadContext(originalFormat, importAttributes, conditions, requestType); if (loadHooks.length === 0) { return defaultLoad(url, context); } @@ -402,12 +411,13 @@ function loadWithHooks(url, originalFormat, importAttributes, conditions, defaul * @param {string|undefined} parentURL * @param {ImportAttributes|undefined} importAttributes * @param {string[]} conditions + * @param {'import'|'require'} requestType See {@link ModuleResolveContext}. * @param {(specifier: string, context: ModuleResolveContext) => ModuleResolveResult} defaultResolve * @returns {ModuleResolveResult} */ -function resolveWithHooks(specifier, parentURL, importAttributes, conditions, defaultResolve) { +function resolveWithHooks(specifier, parentURL, importAttributes, conditions, requestType, defaultResolve) { debug('resolveWithHooks', specifier, parentURL, importAttributes); - const context = new ModuleResolveContext(parentURL, importAttributes, conditions); + const context = new ModuleResolveContext(parentURL, importAttributes, conditions, requestType); if (resolveHooks.length === 0) { return defaultResolve(specifier, context); } diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 78ffb5f834a989..33a464b7e8ac4e 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -50,6 +50,9 @@ const { const { getDefaultConditions, } = require('internal/modules/esm/utils'); +const { + getCjsConditionsArray, +} = require('internal/modules/helpers'); const { deserializeError } = require('internal/error_serdes'); const { SHARED_MEMORY_BYTE_LENGTH, @@ -219,20 +222,24 @@ class AsyncLoaderHooksOnLoaderHookWorker { * @param {string} [parentURL] The URL path of the module's parent. * @param {ImportAttributes} [importAttributes] Attributes from the import * statement or expression. + * @param {'import'|'require'} [requestType] Whether this resolution is on behalf of a + * `require()` call or an `import`. Defaults to `'import'`. * @returns {Promise<{ format: string, url: URL['href'] }>} */ async resolve( originalSpecifier, parentURL, importAttributes = { __proto__: null }, + requestType = 'import', ) { throwIfInvalidParentURL(parentURL); const chain = this.#chains.resolve; const context = { - conditions: getDefaultConditions(), + conditions: requestType === 'require' ? getCjsConditionsArray() : getDefaultConditions(), importAttributes, parentURL, + requestType, }; const meta = { chainFinished: null, @@ -355,6 +362,15 @@ class AsyncLoaderHooksOnLoaderHookWorker { * @returns {Promise<{ format: ModuleFormat, source: ModuleSource }>} */ async load(url, context = {}) { + // The context arriving here was constructed on the main thread (see ModuleLoader + // #getOrCreateModuleJobAfterResolve) and carries `requestType`. Fill in `conditions` so + // user load hooks have parity with sync hooks (which receive conditions via + // ModuleLoadContext). Default requestType to 'import' if the caller didn't set it. + const requestType = context.requestType ?? 'import'; + context.requestType = requestType; + if (context.conditions === undefined) { + context.conditions = requestType === 'require' ? getCjsConditionsArray() : getDefaultConditions(); + } const chain = this.#chains.load; const meta = { chainFinished: null, @@ -834,15 +850,20 @@ class AsyncLoaderHooksProxiedToLoaderHookWorker { * @param {string} [parentURL] The URL path of the module's parent. * @param {ImportAttributes} importAttributes Attributes from the import * statement or expression. + * @param {'import'|'require'} [requestType] Whether this resolution is on behalf of a `require()` + * call or an `import`. Defaults to `'import'`. * @returns {{ format: string, url: URL['href'] }} */ - resolve(originalSpecifier, parentURL, importAttributes) { - return asyncLoaderHookWorker.makeAsyncRequest('resolve', undefined, originalSpecifier, parentURL, importAttributes); + resolve(originalSpecifier, parentURL, importAttributes, requestType = 'import') { + return asyncLoaderHookWorker.makeAsyncRequest('resolve', undefined, originalSpecifier, parentURL, + importAttributes, requestType); } - resolveSync(originalSpecifier, parentURL, importAttributes) { - // This happens only as a result of `import.meta.resolve` calls, which must be sync per spec. - return asyncLoaderHookWorker.makeSyncRequest('resolve', undefined, originalSpecifier, parentURL, importAttributes); + resolveSync(originalSpecifier, parentURL, importAttributes, requestType = 'import') { + // This happens as a result of `import.meta.resolve` calls (which must be sync per spec) and + // `require()` in imported CJS. + return asyncLoaderHookWorker.makeSyncRequest('resolve', undefined, originalSpecifier, parentURL, + importAttributes, requestType); } /** diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 34a9393ac18f01..edf1d205a1275e 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -60,6 +60,7 @@ const { setImportMetaResolveInitializer, } = internalBinding('module_wrap'); const { + getCjsConditionsArray, urlToFilename, } = require('internal/modules/helpers'); const { @@ -571,7 +572,12 @@ class ModuleLoader { return job; } - const context = { format, importAttributes, __proto__: null }; + // Tell user hooks whether this load is on behalf of a require() call (`'require'`) + // or an import statement / dynamic import() / import.meta.resolve() (`'import'`). + // Only kRequireInImportedCJS represents `require()` semantics here; kImportInRequiredESM + // is the import statement inside a require()'d ESM, which is still an import. + const requestTypeString = requestType === kRequireInImportedCJS ? 'require' : 'import'; + const context = { format, importAttributes, requestType: requestTypeString, __proto__: null }; let moduleOrModulePromise; if (requestType === kRequireInImportedCJS) { @@ -617,13 +623,18 @@ class ModuleLoader { * @returns {Promise|ModuleJobBase} */ getOrCreateModuleJob(parentURL, request, requestType) { + // Tell user hooks whether this resolve is on behalf of a require() call or an import. + // Only kRequireInImportedCJS means "for require()"; kImportInRequiredESM is an + // import statement inside a require()'d ESM and still has import semantics. + const requestTypeString = requestType === kRequireInImportedCJS ? 'require' : 'import'; + let maybePromise; if (requestType === kRequireInImportedCJS || requestType === kImportInRequiredESM) { // In these two cases, resolution must be synchronous. - maybePromise = this.resolveSync(parentURL, request); + maybePromise = this.resolveSync(parentURL, request, false, requestTypeString); assert(!isPromise(maybePromise)); } else { - maybePromise = this.#resolve(parentURL, request); + maybePromise = this.#resolve(parentURL, request, requestTypeString); } const afterResolve = (resolveResult) => { @@ -693,17 +704,19 @@ class ModuleLoader { * @param {string} [parentURL] The URL of the module where the module request is initiated. * It's undefined if it's from the root module. * @param {ModuleRequest} request Module request. + * @param {'import'|'require'} [requestType] Whether this resolve is on behalf of a require() call + * or an import. Defaults to 'import'. * @returns {Promise|ResolveResult} */ - #resolve(parentURL, request) { + #resolve(parentURL, request, requestType = 'import') { if (this.isForAsyncLoaderHookWorker) { const specifier = `${request.specifier}`; const importAttributes = request.attributes ?? kEmptyObject; // TODO(joyeecheung): invoke the synchronous hooks in the default step on loader thread. - return this.#asyncLoaderHooks.resolve(specifier, parentURL, importAttributes); + return this.#asyncLoaderHooks.resolve(specifier, parentURL, importAttributes, requestType); } - return this.resolveSync(parentURL, request); + return this.resolveSync(parentURL, request, false, requestType); } /** @@ -732,13 +745,14 @@ class ModuleLoader { * @param {{isResolvedByDefaultResolve: boolean}} out Output object to track whether the default resolve was used * without polluting the user-visible resolve result. * @param {string|URL} specifier See {@link resolveSync}. - * @param {{ parentURL?: string, importAttributes: ImportAttributes, conditions?: string[]}} context - * See {@link resolveSync}. + * @param {{ parentURL?: string, importAttributes: ImportAttributes, conditions?: string[], + * requestType?: 'import'|'require' }} context See {@link resolveSync}. * @returns {{ format: string, url: string }} */ #resolveAndMaybeBlockOnLoaderThread(out, specifier, context) { if (this.#asyncLoaderHooks?.resolveSync) { - return this.#asyncLoaderHooks.resolveSync(specifier, context.parentURL, context.importAttributes); + return this.#asyncLoaderHooks.resolveSync(specifier, context.parentURL, context.importAttributes, + context.requestType); } out.isResolvedByDefaultResolve = true; return this.#cachedDefaultResolve(specifier, context); @@ -755,28 +769,33 @@ class ModuleLoader { * @param {boolean} [shouldSkipSyncHooks] Whether to skip the synchronous hooks registered by module.registerHooks(). * This is used to maintain compatibility for the re-invented require.resolve (in imported CJS customized * by module.register()`) which invokes the CJS resolution separately from the hook chain. + * @param {'import'|'require'} [requestType] Whether this resolve is on behalf of a `require()` call + * or an `import`. Defaults to `'import'`. When `'require'`, resolution uses CJS conditions instead + * of ESM defaults, matching what the CJS loader would do. * @returns {ResolveResult} */ - resolveSync(parentURL, request, shouldSkipSyncHooks = false) { + resolveSync(parentURL, request, shouldSkipSyncHooks = false, requestType = 'import') { const specifier = `${request.specifier}`; const importAttributes = request.attributes ?? kEmptyObject; // Use an output parameter to track the state and avoid polluting the user-visible resolve results. const out = { isResolvedByDefaultResolve: false, __proto__: null }; + const conditions = requestType === 'require' ? getCjsConditionsArray() : this.#defaultConditions; let result; let isResolvedBySyncHooks = false; if (!shouldSkipSyncHooks && syncResolveHooks.length) { // Has module.registerHooks() hooks, chain the asynchronous hooks in the default step. - result = resolveWithSyncHooks(specifier, parentURL, importAttributes, this.#defaultConditions, + result = resolveWithSyncHooks(specifier, parentURL, importAttributes, conditions, requestType, this.#resolveAndMaybeBlockOnLoaderThread.bind(this, out)); // If the default step ran, sync hooks did not short-circuit the resolution. isResolvedBySyncHooks = !out.isResolvedByDefaultResolve; } else { const context = { ...request, - conditions: this.#defaultConditions, + conditions, parentURL, importAttributes, + requestType, __proto__: null, }; result = this.#resolveAndMaybeBlockOnLoaderThread(out, specifier, context); @@ -840,6 +859,12 @@ class ModuleLoader { isSourceLoadedSynchronously: true, __proto__: null, }; + // Default requestType to 'import' for callers (entry point loading, etc.) that don't set it. + const requestType = context.requestType ?? 'import'; + // For `require(esm)` bridge calls the load is semantically a require(), so user hooks should + // see CJS conditions, matching what the CJS loader would pass. For regular imports and + // imports in required ESM, the conditions remain ESM defaults. + const conditions = requestType === 'require' ? getCjsConditionsArray() : this.#defaultConditions; let result; if (syncLoadHooks.length) { // Has module.registerHooks() hooks, chain the asynchronous hooks in the default step. @@ -849,7 +874,8 @@ class ModuleLoader { url, context.format, context.importAttributes, - this.#defaultConditions, + conditions, + requestType, this.#loadAndMaybeBlockOnLoaderThread.bind(this, out), validateLoadSloppy, ); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index c453e2b54f5957..1584378ae24aa4 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -180,7 +180,9 @@ function loadCJSModuleWithSpecialRequire(module, source, url, filename, isMain, const request = { specifier, __proto__: null, attributes: kEmptyObject }; // Skip sync hooks in resolveSync since resolveForCJSWithHooks already ran them above. - const { url: resolvedURL } = cascadedLoader.resolveSync(url, request, /* shouldSkipSyncHooks */ true); + // This is the re-invented require.resolve() in imported CJS, so the requestType is 'require'. + const { url: resolvedURL } = cascadedLoader.resolveSync( + url, request, /* shouldSkipSyncHooks */ true, 'require'); return urlToFilename(resolvedURL); }); setOwnProperty(requireFn, 'main', process.mainModule); diff --git a/test/es-module/test-esm-loader-hooks-request-type.mjs b/test/es-module/test-esm-loader-hooks-request-type.mjs new file mode 100644 index 00000000000000..2fd504546073c6 --- /dev/null +++ b/test/es-module/test-esm-loader-hooks-request-type.mjs @@ -0,0 +1,58 @@ +// Test that asynchronous module hooks registered via `module.register()` +// receive `context.requestType` (`'import'` or `'require'`) and a populated +// `context.conditions` array in both resolve and load hooks. Previously the +// async load hook received a context with no `conditions` and no signal of +// the request type, making composed loaders unable to distinguish `import` +// from `require` semantics. +// +// Refs: https://github.com/nodejs/node/issues/51327 + +import '../common/index.mjs'; +import assert from 'node:assert'; +import { execPath } from 'node:process'; +import fixtures from '../common/fixtures.js'; +import { spawnSyncAndAssert } from '../common/child_process.js'; + +const loaderURL = fixtures.fileURL('es-module-loaders/loader-request-type.mjs'); +const esmURL = fixtures.fileURL('module-hooks/empty.mjs'); + +spawnSyncAndAssert( + execPath, + [ + '--no-warnings', + '--input-type=module', + '--eval', + `import { register } from 'node:module';\n` + + `register(${JSON.stringify(loaderURL.href)});\n` + + `await import(${JSON.stringify(esmURL.href)});\n`, + ], + { + stderr: '', + stdout(output) { + const records = output.split('\n') + .filter((line) => line.length > 0) + .map((line) => JSON.parse(line)); + + function find(predicate, label) { + const record = records.find(predicate); + assert.ok(record, `expected to find a record for ${label}\nRecords:\n${output}`); + return record; + } + + // import() of an ESM file: requestType: 'import', conditions = ESM defaults. + const esmImportLoad = find( + (r) => r.kind === 'load' && r.url.endsWith('module-hooks/empty.mjs'), + 'load of empty.mjs via import()'); + assert.strictEqual(esmImportLoad.requestType, 'import'); + assert.strictEqual(esmImportLoad.hasImportCondition, true); + assert.strictEqual(esmImportLoad.hasRequireCondition, false); + + const esmImportResolve = find( + (r) => r.kind === 'resolve' && r.specifier.endsWith('module-hooks/empty.mjs'), + 'resolve of empty.mjs'); + assert.strictEqual(esmImportResolve.requestType, 'import'); + assert.strictEqual(esmImportResolve.hasImportCondition, true); + assert.strictEqual(esmImportResolve.hasRequireCondition, false); + }, + }, +); diff --git a/test/fixtures/es-module-loaders/hooks-input.mjs b/test/fixtures/es-module-loaders/hooks-input.mjs index e4eb159b398797..ce255749113321 100644 --- a/test/fixtures/es-module-loaders/hooks-input.mjs +++ b/test/fixtures/es-module-loaders/hooks-input.mjs @@ -39,9 +39,11 @@ export async function resolve(specifier, context, next) { 'conditions', 'importAttributes', 'parentURL', + 'requestType', 'importAssertions', ]); assert.ok(Array.isArray(context.conditions)); + assert.strictEqual(context.requestType, 'import'); assert.strictEqual(typeof next, 'function'); const returnValue = { @@ -73,13 +75,18 @@ export async function load(url, context, next) { } assert.ok(new URL(url)); - // Ensure `context` has all and only the properties it's supposed to + // Ensure `context` has all and only the properties it's supposed to. + // requestType and conditions are set by the load step; importAssertions is the deprecated alias. assert.deepStrictEqual(Reflect.ownKeys(context), [ 'format', 'importAttributes', + 'requestType', + 'conditions', 'importAssertions', ]); assert.strictEqual(context.format, 'test'); + assert.strictEqual(context.requestType, 'import'); + assert.ok(Array.isArray(context.conditions)); assert.strictEqual(typeof next, 'function'); const returnValue = { diff --git a/test/fixtures/es-module-loaders/loader-request-type.mjs b/test/fixtures/es-module-loaders/loader-request-type.mjs new file mode 100644 index 00000000000000..31fc82b93a92c1 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-request-type.mjs @@ -0,0 +1,34 @@ +// Async loader fixture for tests verifying that `context.requestType` and +// `context.conditions` are set correctly in resolve and load hooks. +// +// Each call appends a JSON line to stdout describing the hook kind, the +// specifier or url, requestType, and the relevant subset of conditions. + +import { writeSync } from 'node:fs'; + +function logHook(record) { + writeSync(1, JSON.stringify(record) + '\n'); +} + +export async function resolve(specifier, context, nextResolve) { + logHook({ + kind: 'resolve', + specifier, + requestType: context.requestType, + hasRequireCondition: Array.isArray(context.conditions) && context.conditions.includes('require'), + hasImportCondition: Array.isArray(context.conditions) && context.conditions.includes('import'), + }); + return nextResolve(specifier, context); +} + +export async function load(url, context, nextLoad) { + logHook({ + kind: 'load', + url, + requestType: context.requestType, + format: context.format, + hasRequireCondition: Array.isArray(context.conditions) && context.conditions.includes('require'), + hasImportCondition: Array.isArray(context.conditions) && context.conditions.includes('import'), + }); + return nextLoad(url, context); +} diff --git a/test/module-hooks/test-module-hooks-request-type.js b/test/module-hooks/test-module-hooks-request-type.js new file mode 100644 index 00000000000000..db46e7c88ec1c8 --- /dev/null +++ b/test/module-hooks/test-module-hooks-request-type.js @@ -0,0 +1,104 @@ +// Flags: --require-module +'use strict'; + +// Test that synchronous module hooks (module.registerHooks) receive +// context.requestType ('import' or 'require') in both resolve and load hooks, +// and that context.conditions is the correct set for the request type +// (CJS conditions for require, ESM conditions for import). +// +// Refs: https://github.com/nodejs/node/issues/51327 + +const common = require('../common'); +const assert = require('node:assert'); +const { registerHooks } = require('node:module'); +const { pathToFileURL } = require('node:url'); +const fixtures = require('../common/fixtures'); + +const records = []; + +const hook = registerHooks({ + resolve(specifier, context, nextResolve) { + records.push({ + kind: 'resolve', + specifier, + requestType: context.requestType, + conditions: context.conditions, + }); + return nextResolve(specifier, context); + }, + load(url, context, nextLoad) { + records.push({ + kind: 'load', + url, + requestType: context.requestType, + conditions: context.conditions, + format: context.format, + }); + return nextLoad(url, context); + }, +}); + +function findRecord(predicate) { + return records.find(predicate); +} + +function assertRequireContext(record, label) { + assert.strictEqual(record.requestType, 'require'); + assert.ok(Array.isArray(record.conditions), + `${label}: conditions should be an array`); + assert.ok(record.conditions.includes('require'), + `${label}: conditions should include 'require', got ${JSON.stringify(record.conditions)}`); + assert.ok(!record.conditions.includes('import'), + `${label}: conditions should not include 'import', got ${JSON.stringify(record.conditions)}`); +} + +function assertImportContext(record, label) { + assert.strictEqual(record.requestType, 'import'); + assert.ok(Array.isArray(record.conditions), + `${label}: conditions should be an array`); + assert.ok(record.conditions.includes('import'), + `${label}: conditions should include 'import', got ${JSON.stringify(record.conditions)}`); + assert.ok(!record.conditions.includes('require'), + `${label}: conditions should not include 'require', got ${JSON.stringify(record.conditions)}`); +} + +// 1. require() of a CJS file -> requestType: 'require'. +require(fixtures.path('module-hooks', 'require-esm', 'inner.cjs')); + +const cjsRequireLoad = findRecord((r) => + r.kind === 'load' && r.url.endsWith('require-esm/inner.cjs')); +assert.ok(cjsRequireLoad, 'expected load record for inner.cjs'); +assertRequireContext(cjsRequireLoad, 'require(inner.cjs) load'); + +const cjsRequireResolve = findRecord((r) => + r.kind === 'resolve' && typeof r.specifier === 'string' && r.specifier.endsWith('inner.cjs')); +assert.ok(cjsRequireResolve, 'expected resolve record for inner.cjs'); +assertRequireContext(cjsRequireResolve, 'require(inner.cjs) resolve'); + +// 2. require(esm) bridge of an ESM file -> requestType: 'require'. +// This is the require() in CJS of an ESM module. The hook sees the ESM file +// being loaded but the requestType reflects that the load is for a require(). +require(fixtures.path('module-hooks', 'require-esm', 'inner.mjs')); + +const esmRequireLoad = findRecord((r) => + r.kind === 'load' && r.url.endsWith('require-esm/inner.mjs')); +assert.ok(esmRequireLoad, 'expected load record for require(inner.mjs)'); +assertRequireContext(esmRequireLoad, 'require(inner.mjs) load (bridge)'); + +// 3. import() of an ESM file -> requestType: 'import'. +// Use a fixture path that hasn't been required yet so we avoid the load cache. +(async () => { + await import(pathToFileURL(fixtures.path('module-hooks', 'empty.mjs')).href); + + const esmImportLoad = findRecord((r) => + r.kind === 'load' && r.url.endsWith('module-hooks/empty.mjs')); + assert.ok(esmImportLoad, 'expected load record for import(empty.mjs)'); + assertImportContext(esmImportLoad, 'import(empty.mjs) load'); + + const esmImportResolve = findRecord((r) => + r.kind === 'resolve' && typeof r.specifier === 'string' && r.specifier.endsWith('module-hooks/empty.mjs')); + assert.ok(esmImportResolve, 'expected resolve record for import(empty.mjs)'); + assertImportContext(esmImportResolve, 'import(empty.mjs) resolve'); + + hook.deregister(); +})().then(common.mustCall()); From b5e917ff5913c7c61169d330a47f2415335034d5 Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Fri, 22 May 2026 09:32:11 +0900 Subject: [PATCH 2/2] module: refactor requestType helpers and expand hook tests Pre-submission cleanup based on reviewer-style anticipation: * Extract `toRequestTypeString(requestType)` in esm/utils.js so the `kRequireInImportedCJS` -> `'require'` mapping lives in one place instead of being inlined at three call sites. * Extract `getConditionsForRequestType(requestType, defaultEsmConditions)` in modules/helpers.js so the request-type -> conditions selection logic is shared between sync and async hooks. * In `AsyncLoaderHooksOnLoaderHookWorker.load`, construct a fresh context object rather than mutating the one delivered over IPC. This matches the pattern used by the sibling `resolve` method and avoids surprising callers that pass a context through `nextLoad`. * Add tests covering the kImportInRequiredESM case (import inside a require()'d ESM stays `requestType: 'import'`), `import.meta.resolve()`, chained hooks where an intermediate hook drops the context, and the async-loader CJS bridge regression (the original repro for #51327). * Document `context.requestType` and the new `context.conditions` on the async load hook with YAML `changes:` entries, and cross-link `module.register` to its replacement `module.registerHooks`. Refs: https://github.com/nodejs/node/issues/51327 --- doc/api/module.md | 27 ++++ lib/internal/modules/esm/hooks.js | 24 ++-- lib/internal/modules/esm/loader.js | 34 +++-- lib/internal/modules/esm/utils.js | 14 ++ lib/internal/modules/helpers.js | 15 ++ ...est-esm-loader-hooks-conditions-bridge.mjs | 80 +++++++++++ .../test-esm-loader-hooks-request-type.mjs | 56 ++++++-- .../es-module-loaders/hooks-input.mjs | 2 +- .../test-module-hooks-request-type-chained.js | 85 +++++++++++ ...hooks-request-type-import-meta-resolve.mjs | 36 +++++ .../test-module-hooks-request-type.js | 133 ++++++++++++------ 11 files changed, 426 insertions(+), 80 deletions(-) create mode 100644 test/es-module/test-esm-loader-hooks-conditions-bridge.mjs create mode 100644 test/module-hooks/test-module-hooks-request-type-chained.js create mode 100644 test/module-hooks/test-module-hooks-request-type-import-meta-resolve.mjs diff --git a/doc/api/module.md b/doc/api/module.md index ec6877c3a221c3..1add32bc3ba4c9 100644 --- a/doc/api/module.md +++ b/doc/api/module.md @@ -220,6 +220,12 @@ changes: Register a module that exports [hooks][] that customize Node.js module resolution and loading behavior. See [Customization hooks][]. +This API is runtime-deprecated. New code should use +[`module.registerHooks()`][] instead, which runs hooks synchronously on the +main thread and avoids the worker-thread caveats listed under +[caveats of asynchronous customization hooks][]. Hook context fields are +kept in sync across both APIs to ease migration. + This feature requires `--allow-worker` if used with the [Permission Model][]. ### `module.registerHooks(options)` @@ -873,6 +879,11 @@ via `process.execArgv` inheritance. See [the documentation of `Worker`][] for de