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

module: print location of unsettled top-level await in entry points #51999

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 2 additions & 5 deletions lib/internal/main/check_syntax.js
Expand Up @@ -50,8 +50,7 @@ function loadESMIfNeeded(cb) {
const hasModulePreImport = getOptionValue('--import').length > 0;

if (hasModulePreImport) {
const { loadESM } = require('internal/process/esm_loader');
loadESM(cb);
require('internal/modules/run_main').runEntryPointWithESMLoader(cb);
return;
}
cb();
Expand All @@ -76,7 +75,5 @@ async function checkSyntax(source, filename) {
return;
}

const { loadESM } = require('internal/process/esm_loader');
const { handleMainPromise } = require('internal/modules/run_main');
handleMainPromise(loadESM((loader) => wrapSafe(filename, source)));
wrapSafe(filename, source);
}
8 changes: 4 additions & 4 deletions lib/internal/main/eval_stdin.js
Expand Up @@ -10,7 +10,7 @@ const {
const { getOptionValue } = require('internal/options');

const {
evalModule,
evalModuleEntryPoint,
evalScript,
readStdin,
} = require('internal/process/execution');
Expand All @@ -24,15 +24,15 @@ readStdin((code) => {
process._eval = code;

const print = getOptionValue('--print');
const loadESM = getOptionValue('--import').length > 0;
const shouldLoadESM = getOptionValue('--import').length > 0;
if (getOptionValue('--input-type') === 'module' ||
(getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) {
evalModule(code, print);
evalModuleEntryPoint(code, print);
} else {
evalScript('[stdin]',
code,
getOptionValue('--inspect-brk'),
print,
loadESM);
shouldLoadESM);
}
});
8 changes: 4 additions & 4 deletions lib/internal/main/eval_string.js
Expand Up @@ -13,7 +13,7 @@ const {
prepareMainThreadExecution,
markBootstrapComplete,
} = require('internal/process/pre_execution');
const { evalModule, evalScript } = require('internal/process/execution');
const { evalModuleEntryPoint, evalScript } = require('internal/process/execution');
const { addBuiltinLibsToObject } = require('internal/modules/helpers');

const { getOptionValue } = require('internal/options');
Expand All @@ -24,10 +24,10 @@ markBootstrapComplete();

const source = getOptionValue('--eval');
const print = getOptionValue('--print');
const loadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
const shouldLoadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
if (getOptionValue('--input-type') === 'module' ||
(getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) {
evalModule(source, print);
evalModuleEntryPoint(source, print);
} else {
// For backward compatibility, we want the identifier crypto to be the
// `node:crypto` module rather than WebCrypto.
Expand All @@ -54,5 +54,5 @@ if (getOptionValue('--input-type') === 'module' ||
) : source,
getOptionValue('--inspect-brk'),
print,
loadESM);
shouldLoadESM);
}
5 changes: 3 additions & 2 deletions lib/internal/main/repl.js
Expand Up @@ -35,8 +35,7 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
process.exit(kInvalidCommandLineArgument);
}

const esmLoader = require('internal/process/esm_loader');
esmLoader.loadESM(() => {
require('internal/modules/run_main').runEntryPointWithESMLoader(() => {
console.log(`Welcome to Node.js ${process.version}.\n` +
'Type ".help" for more information.');

Expand Down Expand Up @@ -64,5 +63,7 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
getOptionValue('--inspect-brk'),
getOptionValue('--print'));
}
// The TLAs in the REPL are still run as scripts, just transformed as async
// IIFEs for the REPL code itself to await on.
});
}
4 changes: 2 additions & 2 deletions lib/internal/main/worker_thread.js
Expand Up @@ -170,8 +170,8 @@ port.on('message', (message) => {
}

case 'module': {
const { evalModule } = require('internal/process/execution');
PromisePrototypeThen(evalModule(filename), undefined, (e) => {
const { evalModuleEntryPoint } = require('internal/process/execution');
PromisePrototypeThen(evalModuleEntryPoint(filename), undefined, (e) => {
workerOnGlobalUncaughtException(e, true);
});
break;
Expand Down
16 changes: 0 additions & 16 deletions lib/internal/modules/esm/handle_process_exit.js

This file was deleted.

4 changes: 2 additions & 2 deletions lib/internal/modules/esm/hooks.js
Expand Up @@ -146,8 +146,8 @@ class Hooks {
* loader (user-land) to the worker.
*/
async register(urlOrSpecifier, parentURL, data) {
const moduleLoader = require('internal/process/esm_loader').esmLoader;
const keyedExports = await moduleLoader.import(
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
const keyedExports = await cascadedLoader.import(
urlOrSpecifier,
parentURL,
kEmptyObject,
Expand Down
34 changes: 22 additions & 12 deletions lib/internal/modules/esm/loader.js
Expand Up @@ -20,7 +20,7 @@ const {
ERR_UNKNOWN_MODULE_FORMAT,
} = require('internal/errors').codes;
const { getOptionValue } = require('internal/options');
const { pathToFileURL, isURL } = require('internal/url');
const { isURL } = require('internal/url');
const { emitExperimentalWarning } = require('internal/util');
const {
getDefaultConditions,
Expand Down Expand Up @@ -85,11 +85,6 @@ class ModuleLoader {
*/
#defaultConditions = getDefaultConditions();

/**
* The index for assigning unique URLs to anonymous module evaluation
*/
evalIndex = 0;

/**
* Registry of resolved specifiers
*/
Expand Down Expand Up @@ -187,10 +182,7 @@ class ModuleLoader {
}
}

async eval(
source,
url = pathToFileURL(`${process.cwd()}/[eval${++this.evalIndex}]`).href,
) {
async eval(source, url) {
const evalInstance = (url) => {
const { ModuleWrap } = internalBinding('module_wrap');
const { registerModule } = require('internal/modules/esm/utils');
Expand All @@ -214,6 +206,7 @@ class ModuleLoader {
return {
__proto__: null,
namespace: module.getNamespace(),
module,
};
}

Expand Down Expand Up @@ -568,6 +561,23 @@ function getHooksProxy() {
return hooksProxy;
}

let cascadedLoader;
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity: why call it "cascaded" loader?


/**
* This is a singleton ESM loader that integrates the loader hooks, if any.
* It it used by other internal built-ins when they need to load ESM code
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* It it used by other internal built-ins when they need to load ESM code
* It is used by other internal built-ins when they need to load ESM code

* while also respecting hooks.
* When built-ins need access to this loader, they should do
* require('internal/module/esm/loader').getOrInitializeCascadedLoader()
* lazily only right before the loader is actually needed, and don't do it
* in the top-level, to avoid circular dependencies.
* @returns {ModuleLoader}
*/
function getOrInitializeCascadedLoader() {
cascadedLoader ??= createModuleLoader();
return cascadedLoader;
}

/**
* Register a single loader programmatically.
* @param {string|import('url').URL} specifier
Expand Down Expand Up @@ -598,12 +608,11 @@ function getHooksProxy() {
* ```
*/
function register(specifier, parentURL = undefined, options) {
const moduleLoader = require('internal/process/esm_loader').esmLoader;
if (parentURL != null && typeof parentURL === 'object' && !isURL(parentURL)) {
options = parentURL;
parentURL = options.parentURL;
}
moduleLoader.register(
getOrInitializeCascadedLoader().register(
specifier,
parentURL ?? 'data:',
options?.data,
Expand All @@ -614,5 +623,6 @@ function register(specifier, parentURL = undefined, options) {
module.exports = {
createModuleLoader,
MoLow marked this conversation as resolved.
Show resolved Hide resolved
getHooksProxy,
getOrInitializeCascadedLoader,
register,
};
9 changes: 5 additions & 4 deletions lib/internal/modules/esm/translators.js
Expand Up @@ -55,7 +55,6 @@ const {
const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
const moduleWrap = internalBinding('module_wrap');
const { ModuleWrap } = moduleWrap;
const asyncESM = require('internal/process/esm_loader');
const { emitWarningSync } = require('internal/process/warning');
const { internalCompileFunction } = require('internal/vm');
const {
Expand Down Expand Up @@ -157,7 +156,8 @@ function errPath(url) {
* @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} The imported module.
*/
async function importModuleDynamically(specifier, { url }, attributes) {
return asyncESM.esmLoader.import(specifier, url, attributes);
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
return cascadedLoader.import(specifier, url, attributes);
}

// Strategy for loading a standard JavaScript module.
Expand Down Expand Up @@ -243,6 +243,7 @@ function loadCJSModule(module, source, url, filename) {

const compiledWrapper = compileResult.function;

const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
const __dirname = dirname(filename);
// eslint-disable-next-line func-name-matching,func-style
const requireFn = function require(specifier) {
Expand All @@ -261,7 +262,7 @@ function loadCJSModule(module, source, url, filename) {
}
specifier = `${pathToFileURL(path)}`;
}
const job = asyncESM.esmLoader.getModuleJobSync(specifier, url, importAttributes);
const job = cascadedLoader.getModuleJobSync(specifier, url, importAttributes);
job.runSync();
return cjsCache.get(job.url).exports;
};
Expand All @@ -272,7 +273,7 @@ function loadCJSModule(module, source, url, filename) {
specifier = `${pathToFileURL(path)}`;
}
}
const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(specifier, url, kEmptyObject);
const { url: resolvedURL } = cascadedLoader.resolveSync(specifier, url, kEmptyObject);
return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL;
});
setOwnProperty(requireFn, 'main', process.mainModule);
Expand Down
11 changes: 4 additions & 7 deletions lib/internal/modules/esm/utils.js
Expand Up @@ -32,7 +32,6 @@ const {
const {
emitExperimentalWarning,
getCWDURL,
getLazy,
} = require('internal/util');
const {
setImportModuleDynamicallyCallback,
Expand Down Expand Up @@ -181,9 +180,6 @@ function initializeImportMetaObject(symbol, meta) {
}
}
}
const getCascadedLoader = getLazy(
() => require('internal/process/esm_loader').esmLoader,
);

/**
* Proxy the dynamic import to the default loader.
Expand All @@ -194,7 +190,8 @@ const getCascadedLoader = getLazy(
*/
function defaultImportModuleDynamically(specifier, attributes, referrerName) {
const parentURL = normalizeReferrerURL(referrerName);
return getCascadedLoader().import(specifier, parentURL, attributes);
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
return cascadedLoader.import(specifier, parentURL, attributes);
}

/**
Expand Down Expand Up @@ -263,10 +260,10 @@ async function initializeHooks() {
const customLoaderURLs = getOptionValue('--experimental-loader');

const { Hooks } = require('internal/modules/esm/hooks');
const esmLoader = require('internal/process/esm_loader').esmLoader;
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();

const hooks = new Hooks();
esmLoader.setCustomizations(hooks);
cascadedLoader.setCustomizations(hooks);

// We need the loader customizations to be set _before_ we start invoking
// `--require`, otherwise loops can happen because a `--require` script
Expand Down