Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 46cfad4

Browse filesBrowse files
joyeecheungaduh95
joyeecheung
authored and
aduh95
committed
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. PR-URL: #62028 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
1 parent 309f37b commit 46cfad4
Copy full SHA for 46cfad4

15 files changed

+348-50Lines changed: 348 additions & 50 deletions
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+60-40Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,41 +1035,63 @@ function getExportsForCircularRequire(module) {
10351035
return module.exports;
10361036
}
10371037

1038+
10381039
/**
1039-
* Resolve a module request for CommonJS, invoking hooks from module.registerHooks()
1040-
* if necessary.
1040+
* Wraps result of Module._resolveFilename to include additional fields for hooks.
1041+
* See resolveForCJSWithHooks.
10411042
* @param {string} specifier
10421043
* @param {Module|undefined} parent
10431044
* @param {boolean} isMain
1044-
* @param {boolean} shouldSkipModuleHooks
1045+
* @param {ResolveFilenameOptions} options
10451046
* @returns {{url?: string, format?: string, parentURL?: string, filename: string}}
10461047
*/
1047-
function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks) {
1048-
let defaultResolvedURL;
1049-
let defaultResolvedFilename;
1050-
let format;
1051-
1052-
function defaultResolveImpl(specifier, parent, isMain, options) {
1053-
// For backwards compatibility, when encountering requests starting with node:,
1054-
// throw ERR_UNKNOWN_BUILTIN_MODULE on failure or return the normalized ID on success
1055-
// without going into Module._resolveFilename.
1056-
let normalized;
1057-
if (StringPrototypeStartsWith(specifier, 'node:')) {
1058-
normalized = BuiltinModule.normalizeRequirableId(specifier);
1059-
if (!normalized) {
1060-
throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier);
1061-
}
1062-
defaultResolvedURL = specifier;
1063-
format = 'builtin';
1064-
return normalized;
1048+
function wrapResolveFilename(specifier, parent, isMain, options) {
1049+
const filename = Module._resolveFilename(specifier, parent, isMain, options).toString();
1050+
return { __proto__: null, url: undefined, format: undefined, filename };
1051+
}
1052+
1053+
/**
1054+
* See resolveForCJSWithHooks.
1055+
* @param {string} specifier
1056+
* @param {Module|undefined} parent
1057+
* @param {boolean} isMain
1058+
* @param {ResolveFilenameOptions} options
1059+
* @returns {{url?: string, format?: string, parentURL?: string, filename: string}}
1060+
*/
1061+
function defaultResolveImplForCJSLoading(specifier, parent, isMain, options) {
1062+
// For backwards compatibility, when encountering requests starting with node:,
1063+
// throw ERR_UNKNOWN_BUILTIN_MODULE on failure or return the normalized ID on success
1064+
// without going into Module._resolveFilename.
1065+
let normalized;
1066+
if (StringPrototypeStartsWith(specifier, 'node:')) {
1067+
normalized = BuiltinModule.normalizeRequirableId(specifier);
1068+
if (!normalized) {
1069+
throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier);
10651070
}
1066-
return Module._resolveFilename(specifier, parent, isMain, options).toString();
1071+
return { __proto__: null, url: specifier, format: 'builtin', filename: normalized };
10671072
}
1073+
return wrapResolveFilename(specifier, parent, isMain, options);
1074+
}
10681075

1076+
/**
1077+
* Resolve a module request for CommonJS, invoking hooks from module.registerHooks()
1078+
* if necessary.
1079+
* @param {string} specifier
1080+
* @param {Module|undefined} parent
1081+
* @param {boolean} isMain
1082+
* @param {object} internalResolveOptions
1083+
* @param {boolean} internalResolveOptions.shouldSkipModuleHooks Whether to skip module hooks.
1084+
* @param {ResolveFilenameOptions} internalResolveOptions.requireResolveOptions Options from require.resolve().
1085+
* Only used when it comes from require.resolve().
1086+
* @returns {{url?: string, format?: string, parentURL?: string, filename: string}}
1087+
*/
1088+
function resolveForCJSWithHooks(specifier, parent, isMain, internalResolveOptions) {
1089+
const { requireResolveOptions, shouldSkipModuleHooks } = internalResolveOptions;
1090+
const defaultResolveImpl = requireResolveOptions ?
1091+
wrapResolveFilename : defaultResolveImplForCJSLoading;
10691092
// Fast path: no hooks, just return simple results.
10701093
if (!resolveHooks.length || shouldSkipModuleHooks) {
1071-
const filename = defaultResolveImpl(specifier, parent, isMain);
1072-
return { __proto__: null, url: defaultResolvedURL, filename, format };
1094+
return defaultResolveImpl(specifier, parent, isMain, requireResolveOptions);
10731095
}
10741096

10751097
// Slow path: has hooks, do the URL conversions and invoke hooks with contexts.
@@ -1098,27 +1120,25 @@ function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks
10981120
} else {
10991121
conditionSet = getCjsConditions();
11001122
}
1101-
defaultResolvedFilename = defaultResolveImpl(specifier, parent, isMain, {
1123+
1124+
const result = defaultResolveImpl(specifier, parent, isMain, {
11021125
__proto__: null,
1126+
paths: requireResolveOptions?.paths,
11031127
conditions: conditionSet,
11041128
});
1129+
// If the default resolver does not return a URL, convert it for the public API.
1130+
result.url ??= convertCJSFilenameToURL(result.filename);
11051131

1106-
defaultResolvedURL = convertCJSFilenameToURL(defaultResolvedFilename);
1107-
return { __proto__: null, url: defaultResolvedURL };
1132+
// Remove filename because it's not part of the public API.
1133+
// TODO(joyeecheung): maybe expose it in the public API to avoid re-conversion for users too.
1134+
return { __proto__: null, url: result.url, format: result.format };
11081135
}
11091136

11101137
const resolveResult = resolveWithHooks(specifier, parentURL, /* importAttributes */ undefined,
11111138
getCjsConditionsArray(), defaultResolve);
1112-
const { url } = resolveResult;
1113-
format = resolveResult.format;
1114-
1115-
let filename;
1116-
if (url === defaultResolvedURL) { // Not overridden, skip the re-conversion.
1117-
filename = defaultResolvedFilename;
1118-
} else {
1119-
filename = convertURLToCJSFilename(url);
1120-
}
1121-
1139+
const { url, format } = resolveResult;
1140+
// Convert the URL from the hook chain back to a filename for internal use.
1141+
const filename = convertURLToCJSFilename(url);
11221142
const result = { __proto__: null, url, format, filename, parentURL };
11231143
debug('resolveForCJSWithHooks', specifier, parent?.id, isMain, shouldSkipModuleHooks, '->', result);
11241144
return result;
@@ -1213,10 +1233,10 @@ function loadBuiltinWithHooks(id, url, format) {
12131233
* @param {string} request Specifier of module to load via `require`
12141234
* @param {Module} parent Absolute path of the module importing the child
12151235
* @param {boolean} isMain Whether the module is the main entry point
1216-
* @param {object|undefined} options Additional options for loading the module
1236+
* @param {object|undefined} internalResolveOptions Additional options for loading the module
12171237
* @returns {object}
12181238
*/
1219-
Module._load = function(request, parent, isMain, options = kEmptyObject) {
1239+
Module._load = function(request, parent, isMain, internalResolveOptions = kEmptyObject) {
12201240
let relResolveCacheIdentifier;
12211241
if (parent) {
12221242
debug('Module._load REQUEST %s parent: %s', request, parent.id);
@@ -1239,7 +1259,7 @@ Module._load = function(request, parent, isMain, options = kEmptyObject) {
12391259
}
12401260
}
12411261

1242-
const resolveResult = resolveForCJSWithHooks(request, parent, isMain, options.shouldSkipModuleHooks);
1262+
const resolveResult = resolveForCJSWithHooks(request, parent, isMain, internalResolveOptions);
12431263
let { format } = resolveResult;
12441264
const { url, filename } = resolveResult;
12451265

Collapse file

‎lib/internal/modules/esm/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js
+5-2Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -722,13 +722,16 @@ class ModuleLoader {
722722
* `module.registerHooks()` hooks.
723723
* @param {string} [parentURL] See {@link resolve}.
724724
* @param {ModuleRequest} request See {@link resolve}.
725+
* @param {boolean} [shouldSkipSyncHooks] Whether to skip the synchronous hooks registered by module.registerHooks().
726+
* This is used to maintain compatibility for the re-invented require.resolve (in imported CJS customized
727+
* by module.register()`) which invokes the CJS resolution separately from the hook chain.
725728
* @returns {{ format: string, url: string }}
726729
*/
727-
resolveSync(parentURL, request) {
730+
resolveSync(parentURL, request, shouldSkipSyncHooks = false) {
728731
const specifier = `${request.specifier}`;
729732
const importAttributes = request.attributes ?? kEmptyObject;
730733

731-
if (syncResolveHooks.length) {
734+
if (!shouldSkipSyncHooks && syncResolveHooks.length) {
732735
// Has module.registerHooks() hooks, chain the asynchronous hooks in the default step.
733736
return resolveWithSyncHooks(specifier, parentURL, importAttributes, this.#defaultConditions,
734737
this.#resolveAndMaybeBlockOnLoaderThread.bind(this));
Collapse file

‎lib/internal/modules/esm/translators.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/translators.js
+10-5Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ translators.set('module', function moduleStrategy(url, translateContext, parentU
9393

9494
const { requestTypes: { kRequireInImportedCJS } } = require('internal/modules/esm/utils');
9595
const kShouldSkipModuleHooks = { __proto__: null, shouldSkipModuleHooks: true };
96+
const kShouldNotSkipModuleHooks = { __proto__: null, shouldSkipModuleHooks: false };
97+
9698
/**
9799
* Loads a CommonJS module via the ESM Loader sync CommonJS translator.
98100
* This translator creates its own version of the `require` function passed into CommonJS modules.
@@ -163,14 +165,17 @@ function loadCJSModule(module, source, url, filename, isMain) {
163165
};
164166
setOwnProperty(requireFn, 'resolve', function resolve(specifier) {
165167
if (!StringPrototypeStartsWith(specifier, 'node:')) {
166-
const path = CJSModule._resolveFilename(specifier, module);
167-
if (specifier !== path) {
168-
specifier = `${pathToFileURL(path)}`;
168+
const {
169+
filename, url: resolvedURL,
170+
} = resolveForCJSWithHooks(specifier, module, false, kShouldNotSkipModuleHooks);
171+
if (specifier !== filename) {
172+
specifier = resolvedURL ?? `${pathToFileURL(filename)}`;
169173
}
170174
}
171175

172176
const request = { specifier, __proto__: null, attributes: kEmptyObject };
173-
const { url: resolvedURL } = cascadedLoader.resolveSync(url, request);
177+
// Skip sync hooks in resolveSync since resolveForCJSWithHooks already ran them above.
178+
const { url: resolvedURL } = cascadedLoader.resolveSync(url, request, /* shouldSkipSyncHooks */ true);
174179
return urlToFilename(resolvedURL);
175180
});
176181
setOwnProperty(requireFn, 'main', process.mainModule);
@@ -400,7 +405,7 @@ function cjsPreparseModuleExports(filename, source, format) {
400405
let resolved;
401406
let format;
402407
try {
403-
({ format, filename: resolved } = resolveForCJSWithHooks(reexport, module, false));
408+
({ format, filename: resolved } = resolveForCJSWithHooks(reexport, module, false, kShouldNotSkipModuleHooks));
404409
} catch (e) {
405410
debug(`Failed to resolve '${reexport}', skipping`, e);
406411
continue;
Collapse file

‎lib/internal/modules/helpers.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/helpers.js
+18-1Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const {
4040
flushCompileCache,
4141
} = internalBinding('modules');
4242

43+
const lazyCJSLoader = getLazy(() => require('internal/modules/cjs/loader'));
4344
let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
4445
debug = fn;
4546
});
@@ -160,7 +161,23 @@ function makeRequireFunction(mod) {
160161
*/
161162
function resolve(request, options) {
162163
validateString(request, 'request');
163-
return Module._resolveFilename(request, mod, false, options);
164+
const { resolveForCJSWithHooks } = lazyCJSLoader();
165+
// require.resolve() has different behaviors from the internal resolution used by
166+
// Module._load:
167+
// 1. When the request resolves to a non-existent built-in, it throws MODULE_NOT_FOUND
168+
// instead of UNKNOWN_BUILTIN_MODULE. This is handled by resolveForCJSWithHooks.
169+
// 2. If the request is a prefixed built-in, the returned value is also prefixed. This
170+
// is handled below.
171+
const { filename, url } = resolveForCJSWithHooks(
172+
request, mod, /* isMain */ false, {
173+
__proto__: null,
174+
shouldSkipModuleHooks: false,
175+
requireResolveOptions: options ?? {},
176+
});
177+
if (url === request && StringPrototypeStartsWith(request, 'node:')) {
178+
return url;
179+
}
180+
return filename;
164181
}
165182

166183
require.resolve = resolve;
Collapse file

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

Copy file name to clipboard
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// Fixture CJS file that calls require.resolve and exports the result.
2+
'use strict';
3+
const resolved = require.resolve('test-require-resolve-hook-target');
4+
module.exports = { resolved };
Collapse file

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

Copy file name to clipboard
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Fixture CJS file that calls require.resolve with the paths option.
2+
'use strict';
3+
const path = require('path');
4+
const fixturesDir = path.resolve(__dirname, '..', '..');
5+
const nodeModules = path.join(fixturesDir, 'node_modules');
6+
7+
// Use the paths option to resolve 'bar' from the fixtures node_modules.
8+
const resolved = require.resolve('bar', { paths: [fixturesDir] });
9+
const expected = path.join(nodeModules, 'bar.js');
10+
if (resolved !== expected) {
11+
throw new Error(`Expected ${expected}, got ${resolved}`);
12+
}
13+
module.exports = { resolved };
Collapse file

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

Copy file name to clipboard
+25Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
'use strict';
2+
3+
// This tests that require.resolve() works with builtin redirection
4+
// via resolve hooks registered with module.registerHooks().
5+
6+
require('../common');
7+
const assert = require('assert');
8+
const { registerHooks } = require('module');
9+
10+
const hook = registerHooks({
11+
resolve(specifier, context, nextResolve) {
12+
if (specifier === 'assert') {
13+
return {
14+
url: 'node:zlib',
15+
shortCircuit: true,
16+
};
17+
}
18+
return nextResolve(specifier, context);
19+
},
20+
});
21+
22+
const resolved = require.resolve('assert');
23+
assert.strictEqual(resolved, 'zlib');
24+
25+
hook.deregister();
Collapse file

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

Copy file name to clipboard
+38Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
'use strict';
2+
3+
// This tests that require.resolve() and require() both go through the same
4+
// resolve hooks registered via module.registerHooks().
5+
6+
require('../common');
7+
const assert = require('assert');
8+
const { registerHooks } = require('module');
9+
const fixtures = require('../common/fixtures');
10+
const { pathToFileURL } = require('url');
11+
12+
const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js');
13+
14+
const resolvedSpecifiers = [];
15+
const hook = registerHooks({
16+
resolve(specifier, context, nextResolve) {
17+
if (specifier === 'test-consistency-target') {
18+
resolvedSpecifiers.push(specifier);
19+
return {
20+
url: pathToFileURL(redirectedPath).href,
21+
shortCircuit: true,
22+
};
23+
}
24+
return nextResolve(specifier, context);
25+
},
26+
});
27+
28+
const resolveResult = require.resolve('test-consistency-target');
29+
const requireResult = require('test-consistency-target');
30+
31+
assert.strictEqual(resolveResult, redirectedPath);
32+
assert.strictEqual(requireResult.exports_for_test, 'redirected assert');
33+
assert.deepStrictEqual(resolvedSpecifiers, [
34+
'test-consistency-target',
35+
'test-consistency-target',
36+
]);
37+
38+
hook.deregister();
Collapse file

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

Copy file name to clipboard
+31Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
'use strict';
2+
3+
// This tests that require.resolve() from a require function returned by
4+
// module.createRequire() goes through resolve hooks registered via
5+
// module.registerHooks().
6+
7+
require('../common');
8+
const assert = require('assert');
9+
const { registerHooks, createRequire } = require('module');
10+
const fixtures = require('../common/fixtures');
11+
const { pathToFileURL } = require('url');
12+
13+
const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js');
14+
15+
const hook = registerHooks({
16+
resolve(specifier, context, nextResolve) {
17+
if (specifier === 'test-create-require-resolve-target') {
18+
return {
19+
url: pathToFileURL(redirectedPath).href,
20+
shortCircuit: true,
21+
};
22+
}
23+
return nextResolve(specifier, context);
24+
},
25+
});
26+
27+
const customRequire = createRequire(__filename);
28+
const resolved = customRequire.resolve('test-create-require-resolve-target');
29+
assert.strictEqual(resolved, redirectedPath);
30+
31+
hook.deregister();
Collapse file

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

Copy file name to clipboard
+19Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
'use strict';
2+
3+
// This tests that require.resolve() falls through to default resolution
4+
// when resolve hooks registered via module.registerHooks() don't override.
5+
6+
require('../common');
7+
const assert = require('assert');
8+
const { registerHooks } = require('module');
9+
10+
const hook = registerHooks({
11+
resolve(specifier, context, nextResolve) {
12+
return nextResolve(specifier, context);
13+
},
14+
});
15+
16+
const resolved = require.resolve('assert');
17+
assert.strictEqual(resolved, 'assert');
18+
19+
hook.deregister();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.