module: expose requestType to user hooks

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
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: Add `context.requestType` so hooks can distinguish a `require()`
+                 call from an `import`. `context.conditions` now reflects the
+                 request type (CJS conditions when `requestType` is `'require'`).
   - version:
     - v23.5.0
     - v22.15.0
@@ -882,11 +893,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}
@@ -961,6 +980,11 @@ registerHooks({ resolve });
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: Add `context.requestType` so hooks can distinguish a `require()`
+                 call from an `import`. `context.conditions` now reflects the
+                 request type (CJS conditions when `requestType` is `'require'`).
   - version:
     - v23.5.0
     - v22.15.0
@@ -970,11 +994,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}
@@ -1382,6 +1413,11 @@ register('./path-to-my-hooks.js', {
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: Add `context.requestType` so hooks can distinguish a `require()`
+                 call from an `import`. `context.conditions` now reflects the
+                 request type (CJS conditions when `requestType` is `'require'`).
   - version:
     - v21.0.0
     - v20.10.0
@@ -1406,11 +1442,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 +1483,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,
@@ -1474,6 +1514,12 @@ export async function resolve(specifier, context, nextResolve) {
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: Add `context.requestType` so hooks can distinguish a `require()`
+                 call from an `import`. `context.conditions` is now populated on
+                 the load context as well (it was previously absent), and
+                 reflects the request type.
   - version: v22.6.0
     pr-url: https://github.com/nodejs/node/pull/56350
     description: Add support for `source` with format `commonjs-typescript` and `module-typescript`.
@@ -1491,11 +1537,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 +2125,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

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;

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

lib/internal/modules/esm/hooks.js

@@ -50,6 +50,9 @@ const {
 const {
   getDefaultConditions,
 } = require('internal/modules/esm/utils');
+const {
+  getConditionsForRequestType,
+} = 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: getConditionsForRequestType(requestType, getDefaultConditions()),
       importAttributes,
       parentURL,
+      requestType,
     };
     const meta = {
       chainFinished: null,
@@ -355,6 +362,19 @@ class AsyncLoaderHooksOnLoaderHookWorker {
    * @returns {Promise<{ format: ModuleFormat, source: ModuleSource }>}
    */
   async load(url, context = {}) {
+    // The context arriving over IPC was constructed on the main thread (see ModuleLoader
+    // #getOrCreateModuleJobAfterResolve) and carries `requestType`. The requestType fallback
+    // here is for the rare case where a caller constructs a load context directly without
+    // going through that path (for example a user hook that calls `nextLoad(url, {})`). Build
+    // a fresh, plain context object for the hook chain so user hooks observe parity with the
+    // sync ModuleLoadContext shape.
+    const requestType = context.requestType ?? 'import';
+    context = {
+      format: context.format,
+      importAttributes: context.importAttributes,
+      conditions: context.conditions ?? getConditionsForRequestType(requestType, getDefaultConditions()),
+      requestType,
+    };
     const chain = this.#chains.load;
     const meta = {
       chainFinished: null,
@@ -834,15 +854,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);
   }
 
   /**