FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module: run require.resolve through module.registerHooks() · nodejs/node@3d160cd · GitHub

/ node Public

Commit 3d160cd

Browse files
joyeecheung
authored and
richardlau
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 bdc1894 commit 3d160cd

15 files changed

Lines changed: 348 additions & 50 deletions

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

Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,41 +1034,63 @@ function getExportsForCircularRequire(module) {
10341034
return module.exports;
10351035
}
10361036

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

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

10741096
// Slow path: has hooks, do the URL conversions and invoke hooks with contexts.
@@ -1097,27 +1119,25 @@ function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks
10971119
} else {
10981120
conditionSet = getCjsConditions();
10991121
}
1100-
defaultResolvedFilename = defaultResolveImpl(specifier, parent, isMain, {
1122+
1123+
const result = defaultResolveImpl(specifier, parent, isMain, {
11011124
__proto__: null,
1125+
paths: requireResolveOptions?.paths,
11021126
conditions: conditionSet,
11031127
});
1128+
// If the default resolver does not return a URL, convert it for the public API.
1129+
result.url ??= convertCJSFilenameToURL(result.filename);
11041130

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

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

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

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

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

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ translators.set('module', function moduleStrategy(url, translateContext, parentU
9494

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

173177
const request = { specifier, __proto__: null, attributes: kEmptyObject };
174-
const { url: resolvedURL } = cascadedLoader.resolveSync(url, request);
178+
// Skip sync hooks in resolveSync since resolveForCJSWithHooks already ran them above.
179+
const { url: resolvedURL } = cascadedLoader.resolveSync(url, request, /* shouldSkipSyncHooks */ true);
175180
return urlToFilename(resolvedURL);
176181
});
177182
setOwnProperty(requireFn, 'main', process.mainModule);
@@ -401,7 +406,7 @@ function cjsPreparseModuleExports(filename, source, format) {
401406
let resolved;
402407
let format;
403408
try {
404-
({ format, filename: resolved } = resolveForCJSWithHooks(reexport, module, false));
409+
({ format, filename: resolved } = resolveForCJSWithHooks(reexport, module, false, kShouldNotSkipModuleHooks));
405410
} catch (e) {
406411
debug(`Failed to resolve '${reexport}', skipping`, e);
407412
continue;

‎lib/internal/modules/helpers.js‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ const {
4242
flushCompileCache,
4343
} = internalBinding('modules');
4444

45+
const lazyCJSLoader = getLazy(() => require('internal/modules/cjs/loader'));
4546
let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
4647
debug = fn;
4748
});
@@ -198,7 +199,23 @@ function makeRequireFunction(mod) {
198199
*/
199200
function resolve(request, options) {
200201
validateString(request, 'request');
201-
return Module._resolveFilename(request, mod, false, options);
202+
const { resolveForCJSWithHooks } = lazyCJSLoader();
203+
// require.resolve() has different behaviors from the internal resolution used by
204+
// Module._load:
205+
// 1. When the request resolves to a non-existent built-in, it throws MODULE_NOT_FOUND
206+
// instead of UNKNOWN_BUILTIN_MODULE. This is handled by resolveForCJSWithHooks.
207+
// 2. If the request is a prefixed built-in, the returned value is also prefixed. This
208+
// is handled below.
209+
const { filename, url } = resolveForCJSWithHooks(
210+
request, mod, /* isMain */ false, {
211+
__proto__: null,
212+
shouldSkipModuleHooks: false,
213+
requireResolveOptions: options ?? {},
214+
});
215+
if (url === request && StringPrototypeStartsWith(request, 'node:')) {
216+
return url;
217+
}
218+
return filename;
202219
}
203220

204221
require.resolve = resolve;

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

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

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

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

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

Lines 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();

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

Lines 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();

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

Lines 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();

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

Lines 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)
Back | FazBrowse Home | New Git URL