Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vm: support using the default loader to handle dynamic import() #51244

Merged
merged 5 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
412 changes: 307 additions & 105 deletions doc/api/vm.md

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion lib/internal/bootstrap/switches/is_main_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ require('url'); // eslint-disable-line no-restricted-modules
internalBinding('module_wrap');
require('internal/modules/cjs/loader');
require('internal/modules/esm/utils');
require('internal/vm/module');

// Needed to refresh the time origin.
require('internal/perf/utils');
Expand Down
18 changes: 5 additions & 13 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ const {
SafeMap,
SafeWeakMap,
String,
Symbol,
StringPrototypeCharAt,
StringPrototypeCharCodeAt,
StringPrototypeEndsWith,
Expand Down Expand Up @@ -114,7 +113,6 @@ const {
initializeCjsConditions,
loadBuiltinModule,
makeRequireFunction,
normalizeReferrerURL,
stripBOM,
toRealPath,
} = require('internal/modules/helpers');
Expand All @@ -125,12 +123,10 @@ const policy = getLazy(
);
const shouldReportRequiredModules = getLazy(() => process.env.WATCH_REPORT_DEPENDENCIES);

const getCascadedLoader = getLazy(
() => require('internal/process/esm_loader').esmLoader,
);

const permission = require('internal/process/permission');

const {
vm_dynamic_import_default_internal,
} = internalBinding('symbols');
// Whether any user-provided CJS modules had been loaded (executed).
// Used for internal assertions.
let hasLoadedAnyUserCJSModule = false;
Expand Down Expand Up @@ -1257,12 +1253,8 @@ let hasPausedEntry = false;
* @param {object} codeCache The SEA code cache
*/
function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
const hostDefinedOptionId = Symbol(`cjs:${filename}`);
async function importModuleDynamically(specifier, _, importAttributes) {
const cascadedLoader = getCascadedLoader();
return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
importAttributes);
}
const hostDefinedOptionId = vm_dynamic_import_default_internal;
const importModuleDynamically = vm_dynamic_import_default_internal;
if (patched) {
const wrapped = Module.wrap(content);
const script = makeContextifyScript(
Expand Down
14 changes: 7 additions & 7 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const {
StringPrototypeReplaceAll,
StringPrototypeSlice,
StringPrototypeStartsWith,
Symbol,
SyntaxErrorPrototype,
globalThis: { WebAssembly },
} = primordials;
Expand Down Expand Up @@ -59,7 +58,9 @@ const { ModuleWrap } = moduleWrap;
const asyncESM = require('internal/process/esm_loader');
const { emitWarningSync } = require('internal/process/warning');
const { internalCompileFunction } = require('internal/vm');

const {
vm_dynamic_import_default_internal,
} = internalBinding('symbols');
// Lazy-loading to avoid circular dependencies.
let getSourceSync;
/**
Expand Down Expand Up @@ -206,9 +207,8 @@ function enrichCJSError(err, content, filename) {
*/
function loadCJSModule(module, source, url, filename) {
let compiledWrapper;
async function importModuleDynamically(specifier, _, importAttributes) {
return asyncESM.esmLoader.import(specifier, url, importAttributes);
}
const hostDefinedOptionId = vm_dynamic_import_default_internal;
const importModuleDynamically = vm_dynamic_import_default_internal;
try {
compiledWrapper = internalCompileFunction(
source, // code,
Expand All @@ -226,8 +226,8 @@ function loadCJSModule(module, source, url, filename) {
'__filename',
'__dirname',
],
Symbol(`cjs:${filename}`), // hostDefinedOptionsId
importModuleDynamically, // importModuleDynamically
hostDefinedOptionId, // hostDefinedOptionsId
importModuleDynamically, // importModuleDynamically
).function;
} catch (err) {
enrichCJSError(err, source, filename);
Expand Down
71 changes: 44 additions & 27 deletions lib/internal/modules/esm/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const {
ArrayIsArray,
SafeSet,
SafeWeakMap,
Symbol,
ObjectFreeze,
} = primordials;

Expand All @@ -14,8 +13,10 @@ const {
},
} = internalBinding('util');
const {
default_host_defined_options,
vm_dynamic_import_default_internal,
vm_dynamic_import_main_context_default,
vm_dynamic_import_missing_flag,
vm_dynamic_import_no_callback,
} = internalBinding('symbols');

const {
Expand All @@ -28,12 +29,19 @@ const {
loadPreloadModules,
initializeFrozenIntrinsics,
} = require('internal/process/pre_execution');
const { getCWDURL } = require('internal/util');
const {
emitExperimentalWarning,
getCWDURL,
getLazy,
} = require('internal/util');
const {
setImportModuleDynamicallyCallback,
setInitializeImportMetaObjectCallback,
} = internalBinding('module_wrap');
const assert = require('internal/assert');
const {
normalizeReferrerURL,
} = require('internal/modules/helpers');

let defaultConditions;
/**
Expand Down Expand Up @@ -145,8 +153,10 @@ const moduleRegistries = new SafeWeakMap();
*/
function registerModule(referrer, registry) {
const idSymbol = referrer[host_defined_option_symbol];
if (idSymbol === default_host_defined_options ||
idSymbol === vm_dynamic_import_missing_flag) {
if (idSymbol === vm_dynamic_import_no_callback ||
idSymbol === vm_dynamic_import_missing_flag ||
idSymbol === vm_dynamic_import_main_context_default ||
idSymbol === vm_dynamic_import_default_internal) {
// The referrer is compiled without custom callbacks, so there is
// no registry to hold on to. We'll throw
// ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING when a callback is
Expand All @@ -158,26 +168,6 @@ function registerModule(referrer, registry) {
moduleRegistries.set(idSymbol, registry);
}

/**
* Registers the ModuleRegistry for dynamic import() calls with a realm
* as the referrer. Similar to {@link registerModule}, but this function
* generates a new id symbol instead of using the one from the referrer
* object.
* @param {globalThis} globalThis The globalThis object of the realm.
* @param {ModuleRegistry} registry
*/
function registerRealm(globalThis, registry) {
let idSymbol = globalThis[host_defined_option_symbol];
// If the per-realm host-defined options is already registered, do nothing.
if (idSymbol) {
return;
}
// Otherwise, register the per-realm host-defined options.
idSymbol = Symbol('Realm globalThis');
globalThis[host_defined_option_symbol] = idSymbol;
moduleRegistries.set(idSymbol, registry);
}

/**
* Defines the `import.meta` object for a given module.
* @param {symbol} symbol - Reference to the module.
Expand All @@ -191,16 +181,44 @@ function initializeImportMetaObject(symbol, meta) {
}
}
}
const getCascadedLoader = getLazy(
() => require('internal/process/esm_loader').esmLoader,
);

/**
* Proxy the dynamic import to the default loader.
* @param {string} specifier - The module specifier string.
* @param {Record<string, string>} attributes - The import attributes object.
* @param {string|null|undefined} referrerName - name of the referrer.
* @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} - The imported module object.
*/
function defaultImportModuleDynamically(specifier, attributes, referrerName) {
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
const parentURL = normalizeReferrerURL(referrerName);
return getCascadedLoader().import(specifier, parentURL, attributes);
}

/**
* Asynchronously imports a module dynamically using a callback function. The native callback.
* @param {symbol} referrerSymbol - Referrer symbol of the registered script, function, module, or contextified object.
* @param {string} specifier - The module specifier string.
* @param {Record<string, string>} attributes - The import attributes object.
* @param {string|null|undefined} referrerName - name of the referrer.
* @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} - The imported module object.
* @throws {ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING} - If the callback function is missing.
*/
async function importModuleDynamicallyCallback(referrerSymbol, specifier, attributes) {
async function importModuleDynamicallyCallback(referrerSymbol, specifier, attributes, referrerName) {
// For user-provided vm.constants.USE_MAIN_CONTEXT_DEFAULT_LOADER, emit the warning
// and fall back to the default loader.
if (referrerSymbol === vm_dynamic_import_main_context_default) {
emitExperimentalWarning('vm.USE_MAIN_CONTEXT_DEFAULT_LOADER');
return defaultImportModuleDynamically(specifier, attributes, referrerName);
}
// For script compiled internally that should use the default loader to handle dynamic
// import, proxy the request to the default loader without the warning.
if (referrerSymbol === vm_dynamic_import_default_internal) {
return defaultImportModuleDynamically(specifier, attributes, referrerName);
}

if (moduleRegistries.has(referrerSymbol)) {
const { importModuleDynamically, callbackReferrer } = moduleRegistries.get(referrerSymbol);
if (importModuleDynamically !== undefined) {
Expand Down Expand Up @@ -273,7 +291,6 @@ async function initializeHooks() {

module.exports = {
registerModule,
registerRealm,
initializeESM,
initializeHooks,
getDefaultConditions,
Expand Down
35 changes: 28 additions & 7 deletions lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,19 @@ const { validateString } = require('internal/validators');
const fs = require('fs'); // Import all of `fs` so that it can be monkey-patched.
const internalFS = require('internal/fs/utils');
const path = require('path');
const { pathToFileURL, fileURLToPath, URL } = require('internal/url');
const { pathToFileURL, fileURLToPath } = require('internal/url');
const assert = require('internal/assert');

const { getOptionValue } = require('internal/options');
const { setOwnProperty } = require('internal/util');
const { inspect } = require('internal/util/inspect');

const {
privateSymbols: {
require_private_symbol,
},
} = internalBinding('util');
const { canParse: URLCanParse } = internalBinding('url');

let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
debug = fn;
Expand Down Expand Up @@ -288,14 +291,32 @@ function addBuiltinLibsToObject(object, dummyModuleName) {
}

/**
* If a referrer is an URL instance or absolute path, convert it into an URL string.
* @param {string | URL} referrer
* Normalize the referrer name as a URL.
* If it's a string containing an absolute path or a URL it's normalized as
* a URL string.
* Otherwise it's returned as undefined.
* @param {string | null | undefined} referrerName
* @returns {string | undefined}
*/
function normalizeReferrerURL(referrer) {
if (typeof referrer === 'string' && path.isAbsolute(referrer)) {
return pathToFileURL(referrer).href;
function normalizeReferrerURL(referrerName) {
if (referrerName === null || referrerName === undefined) {
return undefined;
}
return new URL(referrer).href;

if (typeof referrerName === 'string') {
if (path.isAbsolute(referrerName)) {
return pathToFileURL(referrerName).href;
}

if (StringPrototypeStartsWith(referrerName, 'file://') ||
URLCanParse(referrerName)) {
return referrerName;
}

return undefined;
}

assert.fail('Unreachable code reached by ' + inspect(referrerName));
}

module.exports = {
Expand Down
26 changes: 17 additions & 9 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,31 @@ function prepareWorkerThreadExecution() {
}

function prepareShadowRealmExecution() {
const { registerRealm } = require('internal/modules/esm/utils');
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
// Patch the process object with legacy properties and normalizations.
// Do not expand argv1 as it is not available in ShadowRealm.
patchProcessObject(false);
setupDebugEnv();

// Disable custom loaders in ShadowRealm.
setupUserModules(true);
registerRealm(globalThis, {
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
__proto__: null,
importModuleDynamically: (specifier, _referrer, attributes) => {
// The handler for `ShadowRealm.prototype.importValue`.
const { esmLoader } = require('internal/process/esm_loader');
// `parentURL` is not set in the case of a ShadowRealm top-level import.
return esmLoader.import(specifier, undefined, attributes);
const {
privateSymbols: {
host_defined_option_symbol,
},
});
} = internalBinding('util');
const {
vm_dynamic_import_default_internal,
} = internalBinding('symbols');

// For ShadowRealm.prototype.importValue(), the referrer name is
// always null, so the native ImportModuleDynamically() callback would
// always fallback to look up the host-defined option from the
// global object using host_defined_option_symbol. Using
// vm_dynamic_import_default_internal as the host-defined option
// instructs the JS-land importModuleDynamicallyCallback() to
// proxy the request to defaultImportModuleDynamically().
globalThis[host_defined_option_symbol] =
vm_dynamic_import_default_internal;
}

function prepareExecution(options) {
Expand Down
8 changes: 3 additions & 5 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,10 @@ function extractSourceMapURLMagicComment(content) {
function maybeCacheSourceMap(filename, content, cjsModuleInstance, isGeneratedSource, sourceURL, sourceMapURL) {
const sourceMapsEnabled = getSourceMapsEnabled();
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
try {
const { normalizeReferrerURL } = require('internal/modules/helpers');
filename = normalizeReferrerURL(filename);
} catch (err) {
const { normalizeReferrerURL } = require('internal/modules/helpers');
filename = normalizeReferrerURL(filename);
if (filename === undefined) {
Comment on lines 107 to +112
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function could really use a JSDoc, if you know what it should be. What is filename? Is it a path string? Because normalizeReferrerURL returns an URL string; shouldn’t it be one or the other?

This feature should support data URLs and --experimental-network-imports, so path strings should probably be normalized to URL strings so that non-file:// URLs work.

Copy link
Member Author

@joyeecheung joyeecheung Dec 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function could really use a JSDoc, if you know what it should be.

I don't know fully what this does but filename is supposed to be always a string because it's used as keys to SafeMaps below.

shouldn’t it be one or the other?

The current behavior is that "it doesn't matter as long as this string identifies a module that contains the sourceURL that V8 gives back to us". I think there can be some cache misses caused by this but that's probably out of scope of this PR, because this PR just tries to preserve the existing mapping behavior in normalizeReferrerURL.

This feature should support data URLs and --experimental-network-imports, so path strings should probably be normalized to URL strings so that non-file:// URLs work.

Supporting those for source maps seems to be out of scope for this PR? Again this is just preserving the existing behavior. Whether additional normalization should happen and how that should work with other components should probably be discussed elsewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filename here can either be an absolute path string (for CJS) or a URL string (for ESM) and is normalized to a URL string by normalizeReferrerURL. I can add JSDoc for source map related functions in a follow-up PR.

// This is most likely an invalid filename in sourceURL of [eval]-wrapper.
debug(err);
return;
}

Expand Down
Loading
Loading