Skip to content

Commit 7478354

Browse files
committed
fixup! fixup! vm: support using the default loader to handle dynamic import()
1 parent f99b2d0 commit 7478354

File tree

5 files changed

+41
-30
lines changed

5 files changed

+41
-30
lines changed

doc/api/vm.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,8 +1684,8 @@ main context:
16841684
1. The module being resolved would be relative to the `filename` option passed
16851685
to `vm.Script` or `vm.compileFunction()`. The resolution can work with a
16861686
`filename` that's either an absolute path or a URL string. If `filename` is
1687-
a string that's neither an absolute path or a URL string, or if it's
1688-
undefined, the resolution will be relative to the current working directory
1687+
a string that's neither an absolute path or a URL, or if it's undefined,
1688+
the resolution will be relative to the current working directory
16891689
of the process. In the case of `vm.createContext()`, the resolution is always
16901690
relative to the current working directory since this option is only used when
16911691
there isn't a referrer script or module.
@@ -1695,7 +1695,9 @@ main context:
16951695
3. For any given `filename` that resolves to a specific path, once the process
16961696
manages to load a particular module from that path, the result may be cached,
16971697
and subsequent load of the same module from the same path would return the
1698-
same thing. There is currently no way to bypass the caching behavior.
1698+
same thing. If the `filename` is a URL string, the cache would not be hit
1699+
if it has different search parameters. For non-URL strings as `filename`s
1700+
there is currently no way to bypass the caching behavior.
16991701
17001702
### When `importModuleDynamically` is a function
17011703

lib/internal/modules/esm/utils.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,14 @@ function initializeImportMetaObject(symbol, meta) {
205205
const getCascadedLoader = getLazy(
206206
() => require('internal/process/esm_loader').esmLoader,
207207
);
208+
209+
/**
210+
* Proxy the dynamic import to the default loader.
211+
* @param {string} specifier - The module specifier string.
212+
* @param {Record<string, string>} attributes - The import attributes object.
213+
* @param {string|null|undefined} referrerName - name of the referrer.
214+
* @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} - The imported module object.
215+
*/
208216
function defaultImportModuleDynamically(specifier, attributes, referrerName) {
209217
const parentURL = normalizeReferrerURL(referrerName);
210218
return getCascadedLoader().import(specifier, parentURL, attributes);
@@ -215,7 +223,7 @@ function defaultImportModuleDynamically(specifier, attributes, referrerName) {
215223
* @param {symbol} referrerSymbol - Referrer symbol of the registered script, function, module, or contextified object.
216224
* @param {string} specifier - The module specifier string.
217225
* @param {Record<string, string>} attributes - The import attributes object.
218-
* @param {string|null} referrerName - name of the referrer.
226+
* @param {string|null|undefined} referrerName - name of the referrer.
219227
* @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} - The imported module object.
220228
* @throws {ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING} - If the callback function is missing.
221229
*/

lib/internal/modules/helpers.js

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@ const { validateString } = require('internal/validators');
2323
const fs = require('fs'); // Import all of `fs` so that it can be monkey-patched.
2424
const internalFS = require('internal/fs/utils');
2525
const path = require('path');
26-
const { pathToFileURL, fileURLToPath, URL } = require('internal/url');
26+
const { pathToFileURL, fileURLToPath } = require('internal/url');
27+
const assert = require('internal/assert');
2728

2829
const { getOptionValue } = require('internal/options');
2930
const { setOwnProperty } = require('internal/util');
31+
const { inspect } = require('internal/util/inspect');
3032

3133
const {
3234
privateSymbols: {
@@ -290,29 +292,31 @@ function addBuiltinLibsToObject(object, dummyModuleName) {
290292

291293
/**
292294
* Normalize the referrer name as a URL.
293-
* If it's an absolute path or a file:// it's normalized as a file:// URL.
294-
* Otherwise it's returned as undefined;
295-
* @param {string | null | undefined | false | any } referrerName
295+
* If it's a string containing an absolute path or a URL it's normalized as
296+
* a URL string.
297+
* Otherwise it's returned as undefined.
298+
* @param {string | null | undefined} referrerName
296299
* @returns {string | undefined}
297300
*/
298301
function normalizeReferrerURL(referrerName) {
299-
if (typeof referrerName !== 'string') {
302+
if (referrerName === null || referrerName === undefined) {
300303
return undefined;
301304
}
302305

303-
if (StringPrototypeStartsWith(referrerName, 'file://')) {
304-
return referrerName;
305-
}
306+
if (typeof referrerName === 'string') {
307+
if (path.isAbsolute(referrerName)) {
308+
return pathToFileURL(referrerName).href;
309+
}
306310

307-
if (path.isAbsolute(referrerName)) {
308-
return pathToFileURL(referrerName).href;
309-
}
311+
if (StringPrototypeStartsWith(referrerName, 'file://') ||
312+
URLCanParse(referrerName)) {
313+
return referrerName;
314+
}
310315

311-
if (URLCanParse(referrerName)) {
312-
return new URL(referrerName).href;
316+
return undefined;
313317
}
314318

315-
return undefined;
319+
assert.fail('Unreachable code reached by ' + inspect(referrerName));
316320
}
317321

318322
module.exports = {

lib/internal/process/pre_execution.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ function prepareWorkerThreadExecution() {
6868
}
6969

7070
function prepareShadowRealmExecution() {
71-
const { registerRealm } = require('internal/modules/esm/utils');
7271
// Patch the process object with legacy properties and normalizations.
7372
// Do not expand argv1 as it is not available in ShadowRealm.
7473
patchProcessObject(false);
@@ -86,9 +85,12 @@ function prepareShadowRealmExecution() {
8685
} = internalBinding('symbols');
8786

8887
// For ShadowRealm.prototype.importValue(), the referrer name is
89-
// always null which would be coerced to an undefined parentURL.
90-
// when we use vm_dynamic_import_default_internal
91-
// to proxy the request to the default handler.
88+
// always null, so the native ImportModuleDynamically() callback would
89+
// always fallback to look up the host-defined option from the
90+
// global object using host_defined_option_symbol. Using
91+
// vm_dynamic_import_default_internal as the host-defined option
92+
// instructs the JS-land importModuleDynamicallyCallback() to
93+
// proxy the request to defaultImportModuleDynamically().
9294
globalThis[host_defined_option_symbol] =
9395
vm_dynamic_import_default_internal;
9496
}

test/es-module/test-vm-main-context-default-loader.js

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ const tmpdir = require('../common/tmpdir');
55
const fixtures = require('../common/fixtures');
66
const url = require('url');
77
const fs = require('fs');
8-
const { inspect } = require('util');
98
const {
109
compileFunction,
1110
Script,
@@ -29,7 +28,6 @@ async function testNotFoundErrors(options) {
2928
// Use try-catch for better async stack traces in the logs.
3029
await assert.rejects(script.runInThisContext(), { code: 'ERR_MODULE_NOT_FOUND' });
3130

32-
result = 'uncaught-fallback';
3331
const imported = compileFunction('return import("./message.mjs")', [], options)();
3432
// Use try-catch for better async stack traces in the logs.
3533
await assert.rejects(imported, { code: 'ERR_MODULE_NOT_FOUND' });
@@ -69,9 +67,7 @@ async function main() {
6967
{
7068
// Importing with absolute path as filename.
7169
tmpdir.refresh();
72-
// Note that we must use different file names for different tests otherwise
73-
// we would get cached results.
74-
const filename = tmpdir.resolve('index1.js');
70+
const filename = tmpdir.resolve('index.js');
7571
const options = {
7672
filename,
7773
importModuleDynamically: USE_MAIN_CONTEXT_DEFAULT_LOADER
@@ -83,9 +79,8 @@ async function main() {
8379
{
8480
// Importing with file:// URL as filename.
8581
tmpdir.refresh();
86-
// Note that we must use different file names for different tests otherwise
87-
// we would get cached results.
88-
const filename = url.pathToFileURL(tmpdir.resolve('index2.js')).href;
82+
// We use a search parameter to bypass caching.
83+
const filename = url.pathToFileURL(tmpdir.resolve('index.js')).href + '?t=1';
8984
const options = {
9085
filename,
9186
importModuleDynamically: USE_MAIN_CONTEXT_DEFAULT_LOADER

0 commit comments

Comments
 (0)