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

[v20.x] Revert "module: print location of unsettled top-level await in entry points" #52898

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
/doc/api/packages.md @nodejs/loaders
/lib/internal/bootstrap/realm.js @nodejs/loaders
/lib/internal/modules/* @nodejs/loaders
/lib/internal/process/esm_loader.js @nodejs/loaders
/lib/internal/process/execution.js @nodejs/loaders
/lib/module.js @nodejs/loaders
/src/module_wrap* @nodejs/loaders @nodejs/vm
Expand Down
7 changes: 5 additions & 2 deletions lib/internal/main/check_syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ function loadESMIfNeeded(cb) {
const hasModulePreImport = getOptionValue('--import').length > 0;

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

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

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

const print = getOptionValue('--print');
const shouldLoadESM = getOptionValue('--import').length > 0;
const loadESM = getOptionValue('--import').length > 0;
if (getOptionValue('--input-type') === 'module' ||
(getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) {
evalModuleEntryPoint(code, print);
evalModule(code, print);
} else {
evalScript('[stdin]',
code,
getOptionValue('--inspect-brk'),
print,
shouldLoadESM);
loadESM);
}
});
8 changes: 4 additions & 4 deletions lib/internal/main/eval_string.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const {
prepareMainThreadExecution,
markBootstrapComplete,
} = require('internal/process/pre_execution');
const { evalModuleEntryPoint, evalScript } = require('internal/process/execution');
const { evalModule, 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 shouldLoadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
const loadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
if (getOptionValue('--input-type') === 'module' ||
(getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) {
evalModuleEntryPoint(source, print);
evalModule(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,
shouldLoadESM);
loadESM);
}
5 changes: 2 additions & 3 deletions lib/internal/main/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
process.exit(kInvalidCommandLineArgument);
}

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

Expand Down Expand Up @@ -63,7 +64,5 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ port.on('message', (message) => {
}

case 'module': {
const { evalModuleEntryPoint } = require('internal/process/execution');
PromisePrototypeThen(evalModuleEntryPoint(filename), undefined, (e) => {
const { evalModule } = require('internal/process/execution');
PromisePrototypeThen(evalModule(filename), undefined, (e) => {
workerOnGlobalUncaughtException(e, true);
});
break;
Expand Down
16 changes: 16 additions & 0 deletions lib/internal/modules/esm/handle_process_exit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict';

const { exitCodes: { kUnfinishedTopLevelAwait } } = internalBinding('errors');

/**
* Handle a Promise from running code that potentially does Top-Level Await.
* In that case, it makes sense to set the exit code to a specific non-zero value
* if the main code never finishes running.
*/
function handleProcessExit() {
process.exitCode ??= kUnfinishedTopLevelAwait;
}

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

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

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

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

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

let cascadedLoader;

/**
* 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
* 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 @@ -608,11 +598,12 @@ function getOrInitializeCascadedLoader() {
* ```
*/
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;
}
getOrInitializeCascadedLoader().register(
moduleLoader.register(
specifier,
parentURL ?? 'data:',
options?.data,
Expand All @@ -623,6 +614,5 @@ function register(specifier, parentURL = undefined, options) {
module.exports = {
createModuleLoader,
getHooksProxy,
getOrInitializeCascadedLoader,
register,
};
9 changes: 4 additions & 5 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ 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');

// Lazy-loading to avoid circular dependencies.
Expand Down Expand Up @@ -157,8 +158,7 @@ function errPath(url) {
* @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} The imported module.
*/
async function importModuleDynamically(specifier, { url }, attributes) {
const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
return cascadedLoader.import(specifier, url, attributes);
return asyncESM.esmLoader.import(specifier, url, attributes);
}

// Strategy for loading a standard JavaScript module.
Expand Down Expand Up @@ -224,7 +224,6 @@ 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 @@ -243,7 +242,7 @@ function loadCJSModule(module, source, url, filename) {
}
specifier = `${pathToFileURL(path)}`;
}
const job = cascadedLoader.getModuleJobSync(specifier, url, importAttributes);
const job = asyncESM.esmLoader.getModuleJobSync(specifier, url, importAttributes);
job.runSync();
return cjsCache.get(job.url).exports;
};
Expand All @@ -254,7 +253,7 @@ function loadCJSModule(module, source, url, filename) {
specifier = `${pathToFileURL(path)}`;
}
}
const { url: resolvedURL } = cascadedLoader.resolveSync(specifier, url, kEmptyObject);
const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(specifier, url, kEmptyObject);
return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL;
});
setOwnProperty(requireFn, 'main', process.mainModule);
Expand Down
11 changes: 7 additions & 4 deletions lib/internal/modules/esm/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const {
const {
emitExperimentalWarning,
getCWDURL,
getLazy,
} = require('internal/util');
const {
setImportModuleDynamicallyCallback,
Expand Down Expand Up @@ -180,6 +181,9 @@ function initializeImportMetaObject(symbol, meta) {
}
}
}
const getCascadedLoader = getLazy(
() => require('internal/process/esm_loader').esmLoader,
);

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

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

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

const hooks = new Hooks();
cascadedLoader.setCustomizations(hooks);
esmLoader.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