module: run require.resolve through module.registerHooks()

Previously, require.resolve() called Module._resolveFilename() directly,
bypassing any resolve hooks registered via module.registerHooks(). This
patch fixes that.

Author: joyeecheung

lib/internal/modules/cjs/loader.js

@@ -1041,9 +1041,11 @@ function getExportsForCircularRequire(module) {
  * @param {Module|undefined} parent
  * @param {boolean} isMain
  * @param {boolean} shouldSkipModuleHooks
+ * @param {object} [resolveOptions] Options from require.resolve().
+ * @param {string[]} [resolveOptions.paths] Paths to search for modules in.
  * @returns {{url?: string, format?: string, parentURL?: string, filename: string}}
  */
-function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks) {
+function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks, resolveOptions) {
   let defaultResolvedURL;
   let defaultResolvedFilename;
   let format;
@@ -1067,7 +1069,7 @@ function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks
 
   // Fast path: no hooks, just return simple results.
   if (!resolveHooks.length || shouldSkipModuleHooks) {
-    const filename = defaultResolveImpl(specifier, parent, isMain);
+    const filename = defaultResolveImpl(specifier, parent, isMain, resolveOptions);
     return { __proto__: null, url: defaultResolvedURL, filename, format };
   }
 
@@ -1099,6 +1101,7 @@ function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks
     }
     defaultResolvedFilename = defaultResolveImpl(specifier, parent, isMain, {
       __proto__: null,
+      paths: resolveOptions?.paths,
       conditions: conditionSet,
     });
 

lib/internal/modules/esm/loader.js

@@ -722,13 +722,16 @@ class ModuleLoader {
    * `module.registerHooks()` hooks.
    * @param {string} [parentURL] See {@link resolve}.
    * @param {ModuleRequest} request See {@link resolve}.
+   * @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.
    * @returns {{ format: string, url: string }}
    */
-  resolveSync(parentURL, request) {
+  resolveSync(parentURL, request, shouldSkipSyncHooks = false) {
     const specifier = `${request.specifier}`;
     const importAttributes = request.attributes ?? kEmptyObject;
 
-    if (syncResolveHooks.length) {
+    if (!shouldSkipSyncHooks && syncResolveHooks.length) {
       // Has module.registerHooks() hooks, chain the asynchronous hooks in the default step.
       return resolveWithSyncHooks(specifier, parentURL, importAttributes, this.#defaultConditions,
                                   this.#resolveAndMaybeBlockOnLoaderThread.bind(this));

lib/internal/modules/esm/translators.js

@@ -164,14 +164,15 @@ function loadCJSModule(module, source, url, filename, isMain) {
   };
   setOwnProperty(requireFn, 'resolve', function resolve(specifier) {
     if (!StringPrototypeStartsWith(specifier, 'node:')) {
-      const path = CJSModule._resolveFilename(specifier, module);
-      if (specifier !== path) {
-        specifier = `${pathToFileURL(path)}`;
+      const { filename } = resolveForCJSWithHooks(specifier, module, false, false);
+      if (specifier !== filename) {
+        specifier = `${pathToFileURL(filename)}`;
       }
     }
 
     const request = { specifier, __proto__: null, attributes: kEmptyObject };
-    const { url: resolvedURL } = cascadedLoader.resolveSync(url, request);
+    // Skip sync hooks in resolveSync since resolveForCJSWithHooks already ran them above.
+    const { url: resolvedURL } = cascadedLoader.resolveSync(url, request, /* shouldSkipSyncHooks */ true);
     return urlToFilename(resolvedURL);
   });
   setOwnProperty(requireFn, 'main', process.mainModule);

lib/internal/modules/helpers.js

@@ -42,6 +42,7 @@ const {
   flushCompileCache,
 } = internalBinding('modules');
 
+const lazyCJSLoader = getLazy(() => require('internal/modules/cjs/loader'));
 let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
   debug = fn;
 });
@@ -198,7 +199,10 @@ function makeRequireFunction(mod) {
    */
   function resolve(request, options) {
     validateString(request, 'request');
-    return Module._resolveFilename(request, mod, false, options);
+    const { resolveForCJSWithHooks } = lazyCJSLoader();
+    const { filename } = resolveForCJSWithHooks(
+      request, mod, /* isMain */ false, /* shouldSkipModuleHooks */ false, options);
+    return filename;
   }
 
   require.resolve = resolve;

test/fixtures/module-hooks/require-resolve-caller.js

@@ -0,0 +1,4 @@
+// Fixture CJS file that calls require.resolve and exports the result.
+'use strict';
+const resolved = require.resolve('test-require-resolve-hook-target');
+module.exports = { resolved };

test/fixtures/module-hooks/require-resolve-paths-caller.js

@@ -0,0 +1,13 @@
+// Fixture CJS file that calls require.resolve with the paths option.
+'use strict';
+const path = require('path');
+const fixturesDir = path.resolve(__dirname, '..', '..');
+const nodeModules = path.join(fixturesDir, 'node_modules');
+
+// Use the paths option to resolve 'bar' from the fixtures node_modules.
+const resolved = require.resolve('bar', { paths: [fixturesDir] });
+const expected = path.join(nodeModules, 'bar.js');
+if (resolved !== expected) {
+  throw new Error(`Expected ${expected}, got ${resolved}`);
+}
+module.exports = { resolved };

test/module-hooks/test-module-hooks-resolve-require-resolve-builtin.js

@@ -0,0 +1,25 @@
+'use strict';
+
+// This tests that require.resolve() works with builtin redirection
+// via resolve hooks registered with module.registerHooks().
+
+require('../common');
+const assert = require('assert');
+const { registerHooks } = require('module');
+
+const hook = registerHooks({
+  resolve(specifier, context, nextResolve) {
+    if (specifier === 'assert') {
+      return {
+        url: 'node:zlib',
+        shortCircuit: true,
+      };
+    }
+    return nextResolve(specifier, context);
+  },
+});
+
+const resolved = require.resolve('assert');
+assert.strictEqual(resolved, 'zlib');
+
+hook.deregister();

test/module-hooks/test-module-hooks-resolve-require-resolve-consistency.js

@@ -0,0 +1,37 @@
+'use strict';
+
+// This tests that require.resolve() and require() both go through the same
+// resolve hooks registered via module.registerHooks().
+
+require('../common');
+const assert = require('assert');
+const { registerHooks } = require('module');
+const fixtures = require('../common/fixtures');
+
+const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js');
+
+const resolvedSpecifiers = [];
+const hook = registerHooks({
+  resolve(specifier, context, nextResolve) {
+    if (specifier === 'test-consistency-target') {
+      resolvedSpecifiers.push(specifier);
+      return {
+        url: `file://${redirectedPath}`,
+        shortCircuit: true,
+      };
+    }
+    return nextResolve(specifier, context);
+  },
+});
+
+const resolveResult = require.resolve('test-consistency-target');
+const requireResult = require('test-consistency-target');
+
+assert.strictEqual(resolveResult, redirectedPath);
+assert.strictEqual(requireResult.exports_for_test, 'redirected assert');
+assert.deepStrictEqual(resolvedSpecifiers, [
+  'test-consistency-target',
+  'test-consistency-target',
+]);
+
+hook.deregister();

test/module-hooks/test-module-hooks-resolve-require-resolve-create-require.js

@@ -0,0 +1,30 @@
+'use strict';
+
+// This tests that require.resolve() from a require function returned by
+// module.createRequire() goes through resolve hooks registered via
+// module.registerHooks().
+
+require('../common');
+const assert = require('assert');
+const { registerHooks, createRequire } = require('module');
+const fixtures = require('../common/fixtures');
+
+const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js');
+
+const hook = registerHooks({
+  resolve(specifier, context, nextResolve) {
+    if (specifier === 'test-create-require-resolve-target') {
+      return {
+        url: `file://${redirectedPath}`,
+        shortCircuit: true,
+      };
+    }
+    return nextResolve(specifier, context);
+  },
+});
+
+const customRequire = createRequire(__filename);
+const resolved = customRequire.resolve('test-create-require-resolve-target');
+assert.strictEqual(resolved, redirectedPath);
+
+hook.deregister();

test/module-hooks/test-module-hooks-resolve-require-resolve-fallthrough.js

@@ -0,0 +1,19 @@
+'use strict';
+
+// This tests that require.resolve() falls through to default resolution
+// when resolve hooks registered via module.registerHooks() don't override.
+
+require('../common');
+const assert = require('assert');
+const { registerHooks } = require('module');
+
+const hook = registerHooks({
+  resolve(specifier, context, nextResolve) {
+    return nextResolve(specifier, context);
+  },
+});
+
+const resolved = require.resolve('assert');
+assert.strictEqual(resolved, 'assert');
+
+hook.deregister();