From 4c8d8203a28d69f94a7c7f88ae7dc3db870bbf15 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Mon, 30 Sep 2019 14:55:59 -0500 Subject: [PATCH 01/14] module: refactor modules bootstrap --- doc/api/esm.md | 8 +-- lib/internal/bootstrap/loaders.js | 4 +- lib/internal/bootstrap/pre_execution.js | 9 ---- lib/internal/main/main.js | 45 +++++++++++++++++ lib/internal/main/run_main_module.js | 17 ++++--- lib/internal/main/worker_thread.js | 10 +++- lib/internal/modules/cjs/loader.js | 49 ++++++++---------- lib/internal/modules/esm/loader.js | 11 +++- lib/internal/modules/esm/module_job.js | 8 +-- lib/internal/modules/esm/module_map.js | 3 +- lib/internal/modules/esm/translators.js | 8 +-- lib/internal/process/esm_loader.js | 50 +++++++++++-------- lib/internal/process/execution.js | 2 +- lib/repl.js | 15 +++--- node.gyp | 1 + test/addons/async-hooks-promise/test.js | 10 ++-- .../test-esm-cjs-load-error-note.mjs | 26 ---------- test/es-module/test-esm-loader-modulemap.js | 2 +- test/es-module/test-esm-no-extension.js | 7 --- test/message/async_error_eval_esm.out | 2 +- test/parallel/test-bootstrap-modules.js | 1 + .../test-internal-module-map-asserts.js | 8 +-- 22 files changed, 164 insertions(+), 132 deletions(-) create mode 100644 lib/internal/main/main.js diff --git a/doc/api/esm.md b/doc/api/esm.md index e585ae4db63646..e1002f11fb3054 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -886,13 +886,13 @@ _isMain_ is **true** when resolving the Node.js application entry point. > 1. Throw a _Module Not Found_ error. > 1. If _pjson.exports_ is not **null** or **undefined**, then > 1. If _pjson.exports_ is a String or Array, then -> 1. Return _PACKAGE_EXPORTS_TARGET_RESOLVE(packageURL, pjson.exports, -> "")_. +> 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, +> _pjson.exports_, "")_. > 1. If _pjson.exports is an Object, then > 1. If _pjson.exports_ contains a _"."_ property, then > 1. Let _mainExport_ be the _"."_ property in _pjson.exports_. -> 1. Return _PACKAGE_EXPORTS_TARGET_RESOLVE(packageURL, mainExport, -> "")_. +> 1. Return **PACKAGE_EXPORTS_TARGET_RESOLVE**(_packageURL_, +> _mainExport_, "")_. > 1. If _pjson.main_ is a String, then > 1. Let _resolvedMain_ be the URL resolution of _packageURL_, "/", and > _pjson.main_. diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 775fea064ecb63..f0cd89460af0b8 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -155,6 +155,7 @@ function NativeModule(id) { this.loaded = false; this.loading = false; this.canBeRequiredByUsers = !id.startsWith('internal/'); + this.defaultOnly = false; } // To be called during pre-execution when --expose-internals is on. @@ -164,6 +165,7 @@ NativeModule.exposeInternals = function() { // Do not expose this to user land even with --expose-internals. if (id !== loaderId) { mod.canBeRequiredByUsers = true; + mod.defaultOnly = true; } } }; @@ -220,7 +222,7 @@ NativeModule.prototype.compileForPublicLoader = function(needToSyncExports) { this.compile(); if (needToSyncExports) { if (!this.exportKeys) { - this.exportKeys = Object.keys(this.exports); + this.exportKeys = this.defaultOnly ? [] : Object.keys(this.exports); } this.getESMFacade(); this.syncExports(); diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index 1126fbcdd71bce..fd5aa1d1748a95 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -404,7 +404,6 @@ function initializeESMLoader() { 'The ESM module loader is experimental.', 'ExperimentalWarning', undefined); } - const { setImportModuleDynamicallyCallback, setInitializeImportMetaObjectCallback @@ -414,14 +413,6 @@ function initializeESMLoader() { // track of for different ESM modules. setInitializeImportMetaObjectCallback(esm.initializeImportMetaObject); setImportModuleDynamicallyCallback(esm.importModuleDynamicallyCallback); - const userLoader = getOptionValue('--experimental-loader'); - // If --experimental-loader is specified, create a loader with user hooks. - // Otherwise create the default loader. - if (userLoader) { - const { emitExperimentalWarning } = require('internal/util'); - emitExperimentalWarning('--experimental-loader'); - } - esm.initializeLoader(process.cwd(), userLoader); } } diff --git a/lib/internal/main/main.js b/lib/internal/main/main.js new file mode 100644 index 00000000000000..fb726f5751d11f --- /dev/null +++ b/lib/internal/main/main.js @@ -0,0 +1,45 @@ +'use strict'; + +module.exports = function runMainAsync(main) { + const { getOptionValue } = require('internal/options'); + const { toRealPath, Module: CJSModule, readPackageScope } = + require('internal/modules/cjs/loader'); + const path = require('path'); + const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); + const loader = getOptionValue('--experimental-loader'); + + const experimentalModules = getOptionValue('--experimental-modules'); + if (!experimentalModules) return; + + // Note extension resolution for the main entry point can be deprecated in a + // future major. + let mainPath = CJSModule._findPath(path.resolve(main), null, true); + if (!preserveSymlinksMain && mainPath) + mainPath = toRealPath(mainPath); + // Determine the module format of the file + let useLoader = !mainPath || loader || mainPath.endsWith('.mjs'); + if (!useLoader && !mainPath.endsWith('.cjs')) { + const pkg = readPackageScope(mainPath); + useLoader = pkg && pkg.type === 'module'; + } + // Load the main module--the command line argument. + if (useLoader) { + const asyncESM = require('internal/process/esm_loader'); + const { pathToFileURL } = require('internal/url'); + const { hasUncaughtExceptionCaptureCallback } = + require('internal/process/execution'); + return (asyncESM.initializeLoader() || Promise.resolve()).then(() => { + const loader = asyncESM.ESMLoader; + return loader.import(pathToFileURL(mainPath || main).href).catch((e) => { + if (hasUncaughtExceptionCaptureCallback()) { + process._fatalException(e); + return; + } + internalBinding('errors').triggerUncaughtException( + e, + true /* fromPromise */ + ); + }); + }); + } +}; diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js index 2cad569dcce9fd..0a9b0c98c09911 100644 --- a/lib/internal/main/run_main_module.js +++ b/lib/internal/main/run_main_module.js @@ -6,12 +6,15 @@ const { prepareMainThreadExecution(true); -const CJSModule = require('internal/modules/cjs/loader').Module; +const runMainAsync = require('internal/main/main'); +const asyncMain = runMainAsync(process.argv[1]); -markBootstrapComplete(); +// Sync runMain. Async case hands over control to the promise chain. +// Calling runMain here ensures only this file is in stack for backward +// compatibility, instead of adding 'internal/main/main' to the stack. +if (!asyncMain) { + const CJSModule = require('internal/modules/cjs/loader').Module; + CJSModule.runMain(process.argv[1]); +} -// Note: this actually tries to run the module as a ESM first if -// --experimental-modules is on. -// TODO(joyeecheung): can we move that logic to here? Note that this -// is an undocumented method available via `require('module').runMain` -CJSModule.runMain(); +markBootstrapComplete(); diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 50a19be77dd78d..f0ee2b1dfa4a4f 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -140,8 +140,14 @@ port.on('message', (message) => { const { evalScript } = require('internal/process/execution'); evalScript('[worker eval]', filename); } else { - process.argv[1] = filename; // script filename - require('module').runMain(); + // script filename + const runMainAsync = require('internal/main/main'); + process.argv[1] = filename; + const asyncMain = runMainAsync(filename); + if (!asyncMain) { + const CJSModule = require('internal/modules/cjs/loader').Module; + CJSModule.runMain(process.argv[1]); + } } } else if (message.type === STDIO_PAYLOAD) { const { stream, chunk, encoding } = message; diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index a8dd3bbaa3f5ae..c98cebce3e0423 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -71,7 +71,7 @@ const { const { validateString } = require('internal/validators'); const pendingDeprecation = getOptionValue('--pending-deprecation'); -module.exports = { wrapSafe, Module }; +module.exports = { wrapSafe, Module, toRealPath, readPackageScope }; let asyncESM, ModuleJob, ModuleWrap, kInstantiated; @@ -810,6 +810,10 @@ Module.prototype.load = function(filename) { this.paths = Module._nodeModulePaths(path.dirname(filename)); const extension = findLongestRegisteredExtension(filename); + // allow .mjs to be overridden + if (filename.endsWith('.mjs') && !Module._extensions['.mjs']) { + throw new ERR_REQUIRE_ESM(filename); + } Module._extensions[extension](this, filename); this.loaded = true; @@ -823,14 +827,19 @@ Module.prototype.load = function(filename) { if (module !== undefined && module.module !== undefined) { if (module.module.getStatus() >= kInstantiated) module.module.setExport('default', exports); - } else { // preemptively cache + } else { + // Preemptively cache + // We use a function to defer promise creation for async hooks. ESMLoader.moduleMap.set( url, - new ModuleJob(ESMLoader, url, () => + // Module job creation will start promises. + // We make it a function to lazily trigger those promises + // for async hooks compatibility. + () => new ModuleJob(ESMLoader, url, () => new ModuleWrap(url, undefined, ['default'], function() { this.setExport('default', exports); }) - ) + , false, false) ); } } @@ -859,7 +868,7 @@ Module.prototype.require = function(id) { var resolvedArgv; let hasPausedEntry = false; -function wrapSafe(filename, content) { +function wrapSafe(filename, content, cjsModuleInstance) { if (patched) { const wrapper = Module.wrap(content); return vm.runInThisContext(wrapper, { @@ -867,7 +876,7 @@ function wrapSafe(filename, content) { lineOffset: 0, displayErrors: true, importModuleDynamically: experimentalModules ? async (specifier) => { - const loader = await asyncESM.loaderPromise; + const loader = asyncESM.ESMLoader; return loader.import(specifier, normalizeReferrerURL(filename)); } : undefined, }); @@ -892,9 +901,8 @@ function wrapSafe(filename, content) { ] ); } catch (err) { - if (experimentalModules) { + if (experimentalModules && process.mainModule === cjsModuleInstance) enrichCJSError(err); - } throw err; } @@ -902,7 +910,7 @@ function wrapSafe(filename, content) { const { callbackMap } = internalBinding('module_wrap'); callbackMap.set(compiled.cacheKey, { importModuleDynamically: async (specifier) => { - const loader = await asyncESM.loaderPromise; + const loader = asyncESM.ESMLoader; return loader.import(specifier, normalizeReferrerURL(filename)); } }); @@ -925,7 +933,7 @@ Module.prototype._compile = function(content, filename) { } maybeCacheSourceMap(filename, content, this); - const compiledWrapper = wrapSafe(filename, content); + const compiledWrapper = wrapSafe(filename, content, this); var inspectorWrapper = null; if (getOptionValue('--inspect-brk') && process._eval == null) { @@ -1024,26 +1032,9 @@ Module._extensions['.node'] = function(module, filename) { return process.dlopen(module, path.toNamespacedPath(filename)); }; -Module._extensions['.mjs'] = function(module, filename) { - throw new ERR_REQUIRE_ESM(filename); -}; - // Bootstrap main module. -Module.runMain = function() { - // Load the main module--the command line argument. - if (experimentalModules) { - asyncESM.loaderPromise.then((loader) => { - return loader.import(pathToFileURL(process.argv[1]).href); - }) - .catch((e) => { - internalBinding('errors').triggerUncaughtException( - e, - true /* fromPromise */ - ); - }); - return; - } - Module._load(process.argv[1], null, true); +Module.runMain = function(main) { + Module._load(main, null, true); }; function createRequireFromPath(filename) { diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index a62529e9b32250..875fb97f912a85 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -22,6 +22,7 @@ const createDynamicModule = require( 'internal/modules/esm/create_dynamic_module'); const { translators } = require('internal/modules/esm/translators'); const { ModuleWrap } = internalBinding('module_wrap'); +const { getOptionValue } = require('internal/options'); const debug = require('internal/util/debuglog').debuglog('esm'); @@ -118,7 +119,7 @@ class Loader { url = pathToFileURL(`${process.cwd()}/[eval${++this.evalIndex}]`).href ) { const evalInstance = (url) => new ModuleWrap(url, undefined, source, 0, 0); - const job = new ModuleJob(this, url, evalInstance, false); + const job = new ModuleJob(this, url, evalInstance, false, false); this.moduleMap.set(url, job); const { module, result } = await job.run(); return { @@ -146,6 +147,9 @@ class Loader { async getModuleJob(specifier, parentURL) { const { url, format } = await this.resolve(specifier, parentURL); let job = this.moduleMap.get(url); + // CommonJS will set functions for lazy job evaluation. + if (typeof job === 'function') + this.moduleMap.set(url, job = job()); if (job !== undefined) return job; @@ -169,7 +173,10 @@ class Loader { loaderInstance = translators.get(format); } - job = new ModuleJob(this, url, loaderInstance, parentURL === undefined); + const inspectBrk = parentURL === undefined && + format === 'module' && getOptionValue('--inspect-brk'); + job = new ModuleJob(this, url, loaderInstance, parentURL === undefined, + inspectBrk); this.moduleMap.set(url, job); return job; } diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index ef11e2ec833b89..df1edc3810c080 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -9,7 +9,6 @@ const { const { ModuleWrap } = internalBinding('module_wrap'); const { decorateErrorStack } = require('internal/util'); -const { getOptionValue } = require('internal/options'); const assert = require('internal/assert'); const resolvedPromise = SafePromise.resolve(); @@ -22,9 +21,10 @@ let hasPausedEntry = false; class ModuleJob { // `loader` is the Loader instance used for loading dependencies. // `moduleProvider` is a function - constructor(loader, url, moduleProvider, isMain) { + constructor(loader, url, moduleProvider, isMain, inspectBrk) { this.loader = loader; this.isMain = isMain; + this.inspectBrk = inspectBrk; // This is a Promise<{ module, reflect }>, whose fields will be copied // onto `this` by `link()` below once it has been resolved. @@ -83,12 +83,12 @@ class ModuleJob { }; await addJobsToDependencyGraph(this); try { - if (!hasPausedEntry && this.isMain && getOptionValue('--inspect-brk')) { + if (!hasPausedEntry && this.inspectBrk) { hasPausedEntry = true; const initWrapper = internalBinding('inspector').callAndPauseOnStart; initWrapper(this.module.instantiate, this.module); } else { - this.module.instantiate(); + this.module.instantiate(true); } } catch (e) { decorateErrorStack(e); diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js index 01521fb7885ee1..41adc0079ada31 100644 --- a/lib/internal/modules/esm/module_map.js +++ b/lib/internal/modules/esm/module_map.js @@ -16,7 +16,8 @@ class ModuleMap extends SafeMap { } set(url, job) { validateString(url, 'url'); - if (job instanceof ModuleJob !== true) { + if (job instanceof ModuleJob !== true && + typeof job !== 'function') { throw new ERR_INVALID_ARG_TYPE('job', 'ModuleJob', job); } debug(`Storing ${url} in ModuleMap`); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 18ccfb35e81c8a..34a9a140dd7732 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -23,7 +23,6 @@ const fs = require('fs'); const { fileURLToPath, URL } = require('url'); const { debuglog } = require('internal/util/debuglog'); const { promisify } = require('internal/util'); -const esmLoader = require('internal/process/esm_loader'); const { ERR_INVALID_URL, ERR_INVALID_URL_SCHEME, @@ -69,9 +68,12 @@ function initializeImportMeta(meta, { url }) { meta.url = url; } +let esmLoader; async function importModuleDynamically(specifier, { url }) { - const loader = await esmLoader.loaderPromise; - return loader.import(specifier, url); + if (!esmLoader) { + esmLoader = require('internal/process/esm_loader'); + } + return esmLoader.ESMLoader.import(specifier, url); } // Strategy for loading a standard JavaScript module diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 69e1fd00163cdb..28fd85f4c08679 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -3,12 +3,14 @@ const { ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING, } = require('internal/errors').codes; - +const assert = require('internal/assert'); const { Loader } = require('internal/modules/esm/loader'); const { pathToFileURL } = require('internal/url'); const { getModuleFromWrap, } = require('internal/vm/module'); +const { getOptionValue } = require('internal/options'); +const userLoader = getOptionValue('--experimental-loader'); exports.initializeImportMetaObject = function(wrap, meta) { const { callbackMap } = internalBinding('module_wrap'); @@ -21,6 +23,7 @@ exports.initializeImportMetaObject = function(wrap, meta) { }; exports.importModuleDynamicallyCallback = async function(wrap, specifier) { + assert(calledInitialize === true || !userLoader); const { callbackMap } = internalBinding('module_wrap'); if (callbackMap.has(wrap)) { const { importModuleDynamically } = callbackMap.get(wrap); @@ -32,24 +35,31 @@ exports.importModuleDynamicallyCallback = async function(wrap, specifier) { throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING(); }; -let loaderResolve; -exports.loaderPromise = new Promise((resolve) => loaderResolve = resolve); - -exports.ESMLoader = undefined; +let ESMLoader = new Loader(); +exports.ESMLoader = ESMLoader; -exports.initializeLoader = function(cwd, userLoader) { - let ESMLoader = new Loader(); - const loaderPromise = (async () => { - if (userLoader) { - const hooks = await ESMLoader.import( - userLoader, pathToFileURL(`${cwd}/`).href); - ESMLoader = new Loader(); - ESMLoader.hook(hooks); - exports.ESMLoader = ESMLoader; - } - return ESMLoader; +let calledInitialize = false; +exports.initializeLoader = initializeLoader; +function initializeLoader() { + assert(calledInitialize === false); + calledInitialize = true; + if (!userLoader) + return; + let cwd; + try { + cwd = process.cwd() + '/'; + } catch { + cwd = 'file:///'; + } + // If --experimental-loader is specified, create a loader with user hooks. + // Otherwise create the default loader. + const { emitExperimentalWarning } = require('internal/util'); + emitExperimentalWarning('--experimental-loader'); + return (async () => { + const hooks = + await ESMLoader.import(userLoader, pathToFileURL(cwd).href); + ESMLoader = new Loader(); + ESMLoader.hook(hooks); + return exports.ESMLoader = ESMLoader; })(); - loaderResolve(loaderPromise); - - exports.ESMLoader = ESMLoader; -}; +} diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index 06dfbce2958f08..eed554263c213c 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -39,7 +39,7 @@ function evalModule(source, print) { const { log, error } = require('internal/console/global'); const { decorateErrorStack } = require('internal/util'); const asyncESM = require('internal/process/esm_loader'); - asyncESM.loaderPromise.then(async (loader) => { + Promise.resolve(asyncESM.ESMLoader).then(async (loader) => { const { result } = await loader.eval(source); if (print) { log(result); diff --git a/lib/repl.js b/lib/repl.js index bce90a18602e8f..88b0f5658489fc 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -99,6 +99,7 @@ const { } = internalBinding('contextify'); const history = require('internal/repl/history'); +const { setImmediate } = require('timers'); // Lazy-loaded. let processTopLevelAwait; @@ -338,11 +339,9 @@ function REPLServer(prompt, script = vm.createScript(code, { filename: file, displayErrors: true, - importModuleDynamically: experimentalModules ? - async (specifier) => { - return (await asyncESM.loaderPromise).import(specifier, pwd); - } : - undefined + importModuleDynamically: experimentalModules ? async (specifier) => { + return asyncESM.ESMLoader.import(specifier, pwd); + } : undefined }); } catch (e) { debug('parse error %j', code, e); @@ -1080,7 +1079,11 @@ function complete(line, callback) { // All this is only profitable if the nested REPL does not have a // bufferedCommand. if (!magic[kBufferedCommandSymbol]) { - magic._domain.on('error', (err) => { throw err; }); + magic._domain.on('error', (err) => { + setImmediate(() => { + throw err; + }); + }); return magic.complete(line, callback); } } diff --git a/node.gyp b/node.gyp index 9bf8fa2b994da1..1392e1fa4d8b2f 100644 --- a/node.gyp +++ b/node.gyp @@ -138,6 +138,7 @@ 'lib/internal/main/eval_string.js', 'lib/internal/main/eval_stdin.js', 'lib/internal/main/inspect.js', + 'lib/internal/main/main.js', 'lib/internal/main/print_bash_completion.js', 'lib/internal/main/print_help.js', 'lib/internal/main/prof_process.js', diff --git a/test/addons/async-hooks-promise/test.js b/test/addons/async-hooks-promise/test.js index a6c48e94a34f07..4109c75809ce09 100644 --- a/test/addons/async-hooks-promise/test.js +++ b/test/addons/async-hooks-promise/test.js @@ -11,16 +11,18 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) { } // Baseline to make sure the internal field isn't being set. +// Because of bootstrap tracking this will be enabled. assert.strictEqual( - binding.getPromiseField(Promise.resolve(1)), - 0); + binding.getPromiseField(Promise.resolve(1)).isChainedPromise, + undefined); const hook0 = async_hooks.createHook({}).enable(); // Check that no PromiseWrap is created when there are no hook callbacks. +// Because of bootstrap tracking this will be enabled. assert.strictEqual( - binding.getPromiseField(Promise.resolve(1)), - 0); + binding.getPromiseField(Promise.resolve(1)).isChainedPromise, + undefined); hook0.disable(); diff --git a/test/es-module/test-esm-cjs-load-error-note.mjs b/test/es-module/test-esm-cjs-load-error-note.mjs index ce0d1d796969d7..5d5091e4995578 100644 --- a/test/es-module/test-esm-cjs-load-error-note.mjs +++ b/test/es-module/test-esm-cjs-load-error-note.mjs @@ -44,19 +44,6 @@ pExport2.on('close', mustCall((code) => { assert.ok(pExport2Stderr.includes(expectedNote), `${expectedNote} not found in ${pExport2Stderr}`); })); -// The flag --experimental-modules is not used here -// the note must not be included in the output -const pExport3 = spawn(process.execPath, [Export1]); -let pExport3Stderr = ''; -pExport3.stderr.setEncoding('utf8'); -pExport3.stderr.on('data', (data) => { - pExport3Stderr += data; -}); -pExport3.on('close', mustCall((code) => { - assert.strictEqual(code, expectedCode); - assert.ok(!pExport3Stderr.includes(expectedNote), - `${expectedNote} must not be included in ${pExport3Stderr}`); -})); const pImport1 = spawn(process.execPath, ['--experimental-modules', Import1]); let pImport1Stderr = ''; @@ -121,16 +108,3 @@ pImport5.on('close', mustCall((code) => { assert.ok(!pImport5Stderr.includes(expectedNote), `${expectedNote} must not be included in ${pImport5Stderr}`); })); - -// Must exit with zero and not show note -const pImport6 = spawn(process.execPath, [Import1]); -let pImport6Stderr = ''; -pImport6.stderr.setEncoding('utf8'); -pImport6.stderr.on('data', (data) => { - pImport6Stderr += data; -}); -pImport6.on('close', mustCall((code) => { - assert.strictEqual(code, expectedCode); - assert.ok(!pImport6Stderr.includes(expectedNote), - `${expectedNote} must not be included in ${pImport6Stderr}`); -})); diff --git a/test/es-module/test-esm-loader-modulemap.js b/test/es-module/test-esm-loader-modulemap.js index 5493c6c47c9643..70f5a1015907a2 100644 --- a/test/es-module/test-esm-loader-modulemap.js +++ b/test/es-module/test-esm-loader-modulemap.js @@ -1,5 +1,5 @@ 'use strict'; -// Flags: --expose-internals +// Flags: --expose-internals --experimental-modules // This test ensures that the type checking of ModuleMap throws // errors appropriately diff --git a/test/es-module/test-esm-no-extension.js b/test/es-module/test-esm-no-extension.js index e3f30d6e3c2808..81b8e5b4327ad0 100644 --- a/test/es-module/test-esm-no-extension.js +++ b/test/es-module/test-esm-no-extension.js @@ -15,11 +15,6 @@ const child = spawn(process.execPath, [ entry ]); -let stderr = ''; -child.stderr.setEncoding('utf8'); -child.stderr.on('data', (data) => { - stderr += data; -}); let stdout = ''; child.stdout.setEncoding('utf8'); child.stdout.on('data', (data) => { @@ -29,6 +24,4 @@ child.on('close', common.mustCall((code, signal) => { assert.strictEqual(code, 0); assert.strictEqual(signal, null); assert.strictEqual(stdout, 'executed\n'); - assert.strictEqual(stderr, `(node:${child.pid}) ` + - 'ExperimentalWarning: The ESM module loader is experimental.\n'); })); diff --git a/test/message/async_error_eval_esm.out b/test/message/async_error_eval_esm.out index 578a9f5c26160e..ca4baca802b4f0 100644 --- a/test/message/async_error_eval_esm.out +++ b/test/message/async_error_eval_esm.out @@ -1,7 +1,7 @@ Error: test at one (file:*/[eval1]:2:9) at two (file:*/[eval1]:15:9) - at processTicksAndRejections (internal/process/task_queues.js:*:*) + at processTicksAndRejections (internal/process/task_queues.js:93:5) at async three (file:*/[eval1]:18:3) at async four (file:*/[eval1]:22:3) at async main (file:*/[eval1]:28:5) diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 939658a3b0553f..e207bdddce1065 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -46,6 +46,7 @@ const expectedModules = new Set([ 'NativeModule internal/fs/utils', 'NativeModule internal/idna', 'NativeModule internal/linkedlist', + 'NativeModule internal/main/main', 'NativeModule internal/modules/cjs/helpers', 'NativeModule internal/modules/cjs/loader', 'NativeModule internal/options', diff --git a/test/parallel/test-internal-module-map-asserts.js b/test/parallel/test-internal-module-map-asserts.js index 4563fc605e0792..614da43aba0acb 100644 --- a/test/parallel/test-internal-module-map-asserts.js +++ b/test/parallel/test-internal-module-map-asserts.js @@ -12,7 +12,7 @@ const ModuleMap = require('internal/modules/esm/module_map'); code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: /^The "url" argument must be of type string/ - }, 15); + }, 12); const moduleMap = new ModuleMap(); @@ -21,7 +21,7 @@ const ModuleMap = require('internal/modules/esm/module_map'); // but I think it's useless, and was not simple to mock... const job = undefined; - [{}, [], true, 1, () => {}].forEach((value) => { + [{}, [], true, 1].forEach((value) => { assert.throws(() => moduleMap.get(value), errorReg); assert.throws(() => moduleMap.has(value), errorReg); assert.throws(() => moduleMap.set(value, job), errorReg); @@ -34,11 +34,11 @@ const ModuleMap = require('internal/modules/esm/module_map'); code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: /^The "job" argument must be of type ModuleJob/ - }, 5); + }, 4); const moduleMap = new ModuleMap(); - [{}, [], true, 1, () => {}].forEach((value) => { + [{}, [], true, 1].forEach((value) => { assert.throws(() => moduleMap.set('', value), errorReg); }); } From 76b41531c4d28846e167b59da08f2bc28dcb5084 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 11 Oct 2019 16:11:23 -0400 Subject: [PATCH 02/14] bootstrap complete before user code --- lib/internal/main/run_main_module.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js index 0a9b0c98c09911..8f9feb636d7b3d 100644 --- a/lib/internal/main/run_main_module.js +++ b/lib/internal/main/run_main_module.js @@ -7,6 +7,9 @@ const { prepareMainThreadExecution(true); const runMainAsync = require('internal/main/main'); + +markBootstrapComplete(); + const asyncMain = runMainAsync(process.argv[1]); // Sync runMain. Async case hands over control to the promise chain. @@ -16,5 +19,3 @@ if (!asyncMain) { const CJSModule = require('internal/modules/cjs/loader').Module; CJSModule.runMain(process.argv[1]); } - -markBootstrapComplete(); From eb97b1c0aefc3fed5422831470b9792e6e5a7c25 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 11 Oct 2019 17:22:36 -0400 Subject: [PATCH 03/14] fixup for merge --- lib/internal/main/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/main/main.js b/lib/internal/main/main.js index fb726f5751d11f..07306e0da8d591 100644 --- a/lib/internal/main/main.js +++ b/lib/internal/main/main.js @@ -20,7 +20,7 @@ module.exports = function runMainAsync(main) { let useLoader = !mainPath || loader || mainPath.endsWith('.mjs'); if (!useLoader && !mainPath.endsWith('.cjs')) { const pkg = readPackageScope(mainPath); - useLoader = pkg && pkg.type === 'module'; + useLoader = pkg && pkg.data.type === 'module'; } // Load the main module--the command line argument. if (useLoader) { From 31bb33f7260808753dd995f578b770c832d6d4d1 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 11 Oct 2019 17:36:20 -0400 Subject: [PATCH 04/14] avoid unnecessary test change --- test/message/async_error_eval_esm.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/message/async_error_eval_esm.out b/test/message/async_error_eval_esm.out index ca4baca802b4f0..578a9f5c26160e 100644 --- a/test/message/async_error_eval_esm.out +++ b/test/message/async_error_eval_esm.out @@ -1,7 +1,7 @@ Error: test at one (file:*/[eval1]:2:9) at two (file:*/[eval1]:15:9) - at processTicksAndRejections (internal/process/task_queues.js:93:5) + at processTicksAndRejections (internal/process/task_queues.js:*:*) at async three (file:*/[eval1]:18:3) at async four (file:*/[eval1]:22:3) at async main (file:*/[eval1]:28:5) From 7c470665bf670204f0f4c903012936a3baf80914 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 11 Oct 2019 17:50:51 -0400 Subject: [PATCH 05/14] remove unnecessary test change --- .../test-esm-cjs-load-error-note.mjs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/es-module/test-esm-cjs-load-error-note.mjs b/test/es-module/test-esm-cjs-load-error-note.mjs index 5d5091e4995578..ce0d1d796969d7 100644 --- a/test/es-module/test-esm-cjs-load-error-note.mjs +++ b/test/es-module/test-esm-cjs-load-error-note.mjs @@ -44,6 +44,19 @@ pExport2.on('close', mustCall((code) => { assert.ok(pExport2Stderr.includes(expectedNote), `${expectedNote} not found in ${pExport2Stderr}`); })); +// The flag --experimental-modules is not used here +// the note must not be included in the output +const pExport3 = spawn(process.execPath, [Export1]); +let pExport3Stderr = ''; +pExport3.stderr.setEncoding('utf8'); +pExport3.stderr.on('data', (data) => { + pExport3Stderr += data; +}); +pExport3.on('close', mustCall((code) => { + assert.strictEqual(code, expectedCode); + assert.ok(!pExport3Stderr.includes(expectedNote), + `${expectedNote} must not be included in ${pExport3Stderr}`); +})); const pImport1 = spawn(process.execPath, ['--experimental-modules', Import1]); let pImport1Stderr = ''; @@ -108,3 +121,16 @@ pImport5.on('close', mustCall((code) => { assert.ok(!pImport5Stderr.includes(expectedNote), `${expectedNote} must not be included in ${pImport5Stderr}`); })); + +// Must exit with zero and not show note +const pImport6 = spawn(process.execPath, [Import1]); +let pImport6Stderr = ''; +pImport6.stderr.setEncoding('utf8'); +pImport6.stderr.on('data', (data) => { + pImport6Stderr += data; +}); +pImport6.on('close', mustCall((code) => { + assert.strictEqual(code, expectedCode); + assert.ok(!pImport6Stderr.includes(expectedNote), + `${expectedNote} must not be included in ${pImport6Stderr}`); +})); From 0f6c8c0450a591b54694ce39b7c73b15ddf2eef3 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 11 Oct 2019 18:15:20 -0400 Subject: [PATCH 06/14] ensure warning triggers immediately --- lib/internal/modules/cjs/loader.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index c98cebce3e0423..9357adf2bf0dad 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -989,7 +989,11 @@ Module._extensions['.js'] = function(module, filename) { 'files in that package scope as ES modules.\nInstead rename ' + `${basename} to end in .cjs, change the requiring code to use ` + 'import(), or remove "type": "module" from ' + - `${path.resolve(pkg.path, 'package.json')}.` + `${path.resolve(pkg.path, 'package.json')}.`, + undefined, + undefined, + undefined, + true ); warnRequireESM = false; } From 2d4dfef11ffac7f95a7beead48091596220b9917 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 12 Oct 2019 22:55:20 -0400 Subject: [PATCH 07/14] keep CJS runMain, move main to process --- lib/internal/main/main.js | 45 ------------------ lib/internal/main/run_main_module.js | 12 +---- lib/internal/main/worker_thread.js | 9 +--- lib/internal/modules/cjs/loader.js | 15 +++++- lib/internal/process/main.js | 61 +++++++++++++++++++++++++ node.gyp | 2 +- test/parallel/test-bootstrap-modules.js | 2 +- 7 files changed, 80 insertions(+), 66 deletions(-) delete mode 100644 lib/internal/main/main.js create mode 100644 lib/internal/process/main.js diff --git a/lib/internal/main/main.js b/lib/internal/main/main.js deleted file mode 100644 index 07306e0da8d591..00000000000000 --- a/lib/internal/main/main.js +++ /dev/null @@ -1,45 +0,0 @@ -'use strict'; - -module.exports = function runMainAsync(main) { - const { getOptionValue } = require('internal/options'); - const { toRealPath, Module: CJSModule, readPackageScope } = - require('internal/modules/cjs/loader'); - const path = require('path'); - const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); - const loader = getOptionValue('--experimental-loader'); - - const experimentalModules = getOptionValue('--experimental-modules'); - if (!experimentalModules) return; - - // Note extension resolution for the main entry point can be deprecated in a - // future major. - let mainPath = CJSModule._findPath(path.resolve(main), null, true); - if (!preserveSymlinksMain && mainPath) - mainPath = toRealPath(mainPath); - // Determine the module format of the file - let useLoader = !mainPath || loader || mainPath.endsWith('.mjs'); - if (!useLoader && !mainPath.endsWith('.cjs')) { - const pkg = readPackageScope(mainPath); - useLoader = pkg && pkg.data.type === 'module'; - } - // Load the main module--the command line argument. - if (useLoader) { - const asyncESM = require('internal/process/esm_loader'); - const { pathToFileURL } = require('internal/url'); - const { hasUncaughtExceptionCaptureCallback } = - require('internal/process/execution'); - return (asyncESM.initializeLoader() || Promise.resolve()).then(() => { - const loader = asyncESM.ESMLoader; - return loader.import(pathToFileURL(mainPath || main).href).catch((e) => { - if (hasUncaughtExceptionCaptureCallback()) { - process._fatalException(e); - return; - } - internalBinding('errors').triggerUncaughtException( - e, - true /* fromPromise */ - ); - }); - }); - } -}; diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js index 8f9feb636d7b3d..9f7e2bfbdc94cc 100644 --- a/lib/internal/main/run_main_module.js +++ b/lib/internal/main/run_main_module.js @@ -6,16 +6,8 @@ const { prepareMainThreadExecution(true); -const runMainAsync = require('internal/main/main'); +const CJSModule = require('internal/modules/cjs/loader').Module; markBootstrapComplete(); -const asyncMain = runMainAsync(process.argv[1]); - -// Sync runMain. Async case hands over control to the promise chain. -// Calling runMain here ensures only this file is in stack for backward -// compatibility, instead of adding 'internal/main/main' to the stack. -if (!asyncMain) { - const CJSModule = require('internal/modules/cjs/loader').Module; - CJSModule.runMain(process.argv[1]); -} +CJSModule.runMain(process.argv[1]); diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index f0ee2b1dfa4a4f..7cf8ffb8b401f5 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -141,13 +141,8 @@ port.on('message', (message) => { evalScript('[worker eval]', filename); } else { // script filename - const runMainAsync = require('internal/main/main'); - process.argv[1] = filename; - const asyncMain = runMainAsync(filename); - if (!asyncMain) { - const CJSModule = require('internal/modules/cjs/loader').Module; - CJSModule.runMain(process.argv[1]); - } + const CJSModule = require('internal/modules/cjs/loader').Module; + CJSModule.runMain(process.argv[1] = filename); } } else if (message.type === STDIO_PAYLOAD) { const { stream, chunk, encoding } = message; diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 9357adf2bf0dad..67c5d1ecf49469 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -69,6 +69,11 @@ const { ERR_REQUIRE_ESM } = require('internal/errors').codes; const { validateString } = require('internal/validators'); +const { + resolveMainPath, + shouldUseESMLoader, + runMainESM +} = require('internal/process/main'); const pendingDeprecation = getOptionValue('--pending-deprecation'); module.exports = { wrapSafe, Module, toRealPath, readPackageScope }; @@ -1038,7 +1043,13 @@ Module._extensions['.node'] = function(module, filename) { // Bootstrap main module. Module.runMain = function(main) { - Module._load(main, null, true); + const resolvedMain = resolveMainPath(process.argv[1]); + const useESMLoader = shouldUseESMLoader(resolvedMain); + if (useESMLoader) { + runMainESM(resolvedMain || process.argv[1]); + } else { + Module._load(main, null, true); + } }; function createRequireFromPath(filename) { @@ -1159,7 +1170,7 @@ Module.Module = Module; // We have to load the esm things after module.exports! if (experimentalModules) { - asyncESM = require('internal/process/esm_loader'); ModuleJob = require('internal/modules/esm/module_job'); + asyncESM = require('internal/process/esm_loader'); ({ ModuleWrap, kInstantiated } = internalBinding('module_wrap')); } diff --git a/lib/internal/process/main.js b/lib/internal/process/main.js new file mode 100644 index 00000000000000..2948040973b538 --- /dev/null +++ b/lib/internal/process/main.js @@ -0,0 +1,61 @@ +'use strict'; +const path = require('path'); +const { getOptionValue } = require('internal/options'); + +exports.resolveMainPath = resolveMainPath; +function resolveMainPath(main) { + const { toRealPath, Module: CJSModule } = + require('internal/modules/cjs/loader'); + + // Note extension resolution for the main entry point can be deprecated in a + // future major. + let mainPath = CJSModule._findPath(path.resolve(main), null, true); + if (!mainPath) + return; + + const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); + if (!preserveSymlinksMain) + mainPath = toRealPath(mainPath); + + return mainPath; +} + +exports.shouldUseESMLoader = shouldUseESMLoader; +function shouldUseESMLoader(mainPath) { + const experimentalModules = getOptionValue('--experimental-modules'); + if (!experimentalModules) + return false; + const userLoader = getOptionValue('--experimental-loader'); + if (userLoader) + return true; + // Determine the module format of the main + if (mainPath && mainPath.endsWith('.mjs')) + return true; + if (!mainPath || mainPath.endsWith('.cjs')) + return false; + const { readPackageScope } = require('internal/modules/cjs/loader'); + const pkg = readPackageScope(mainPath); + return pkg && pkg.data.type === 'module'; +} + +exports.runMainESM = runMainESM; +function runMainESM(mainPath) { + const esmLoader = require('internal/process/esm_loader'); + const { pathToFileURL } = require('internal/url'); + const { hasUncaughtExceptionCaptureCallback } = + require('internal/process/execution'); + return (esmLoader.initializeLoader() || Promise.resolve()).then(() => { + const main = path.isAbsolute(mainPath) ? + pathToFileURL(mainPath).href : mainPath; + return esmLoader.ESMLoader.import(main).catch((e) => { + if (hasUncaughtExceptionCaptureCallback()) { + process._fatalException(e); + return; + } + internalBinding('errors').triggerUncaughtException( + e, + true /* fromPromise */ + ); + }); + }); +} diff --git a/node.gyp b/node.gyp index 1392e1fa4d8b2f..2cfb1db476fbf2 100644 --- a/node.gyp +++ b/node.gyp @@ -138,7 +138,6 @@ 'lib/internal/main/eval_string.js', 'lib/internal/main/eval_stdin.js', 'lib/internal/main/inspect.js', - 'lib/internal/main/main.js', 'lib/internal/main/print_bash_completion.js', 'lib/internal/main/print_help.js', 'lib/internal/main/prof_process.js', @@ -161,6 +160,7 @@ 'lib/internal/priority_queue.js', 'lib/internal/process/esm_loader.js', 'lib/internal/process/execution.js', + 'lib/internal/process/main.js', 'lib/internal/process/main_thread_only.js', 'lib/internal/process/per_thread.js', 'lib/internal/process/policy.js', diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index e207bdddce1065..ec99bb1b091202 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -46,12 +46,12 @@ const expectedModules = new Set([ 'NativeModule internal/fs/utils', 'NativeModule internal/idna', 'NativeModule internal/linkedlist', - 'NativeModule internal/main/main', 'NativeModule internal/modules/cjs/helpers', 'NativeModule internal/modules/cjs/loader', 'NativeModule internal/options', 'NativeModule internal/priority_queue', 'NativeModule internal/process/execution', + 'NativeModule internal/process/main', 'NativeModule internal/process/per_thread', 'NativeModule internal/process/promises', 'NativeModule internal/process/task_queues', From fe0c923c2f524ebd89d609baac91de2068d4ca01 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 12 Oct 2019 23:03:13 -0400 Subject: [PATCH 08/14] defaultOnly comment --- lib/internal/bootstrap/loaders.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index f0cd89460af0b8..78a0bfe3bcf9c6 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -155,6 +155,8 @@ function NativeModule(id) { this.loaded = false; this.loading = false; this.canBeRequiredByUsers = !id.startsWith('internal/'); + // When using --expose-internals, we do not want to reflect the named exports + // from core modules as this can trigger getters. this.defaultOnly = false; } From c7504883e48c0dbf7de99243503a378fb8a7592d Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 14 Oct 2019 17:33:31 -0400 Subject: [PATCH 09/14] move main functions into pre_execution --- lib/internal/bootstrap/pre_execution.js | 60 ++++++++++++++++++++++++ lib/internal/modules/cjs/loader.js | 2 +- lib/internal/process/main.js | 61 ------------------------- node.gyp | 1 - test/parallel/test-bootstrap-modules.js | 1 - 5 files changed, 61 insertions(+), 64 deletions(-) delete mode 100644 lib/internal/process/main.js diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index fd5aa1d1748a95..c2d72fb0baf272 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -5,6 +5,7 @@ const { Object, SafeWeakMap } = primordials; const { getOptionValue } = require('internal/options'); const { Buffer } = require('buffer'); const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes; +const path = require('path'); function prepareMainThreadExecution(expandArgv1 = false) { // Patch the process object with legacy properties and normalizations @@ -437,11 +438,70 @@ function loadPreloadModules() { } } +function resolveMainPath(main) { + const { toRealPath, Module: CJSModule } = + require('internal/modules/cjs/loader'); + + // Note extension resolution for the main entry point can be deprecated in a + // future major. + let mainPath = CJSModule._findPath(path.resolve(main), null, true); + if (!mainPath) + return; + + const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); + if (!preserveSymlinksMain) + mainPath = toRealPath(mainPath); + + return mainPath; +} + +function shouldUseESMLoader(mainPath) { + const experimentalModules = getOptionValue('--experimental-modules'); + if (!experimentalModules) + return false; + const userLoader = getOptionValue('--experimental-loader'); + if (userLoader) + return true; + // Determine the module format of the main + if (mainPath && mainPath.endsWith('.mjs')) + return true; + if (!mainPath || mainPath.endsWith('.cjs')) + return false; + const { readPackageScope } = require('internal/modules/cjs/loader'); + const pkg = readPackageScope(mainPath); + return pkg && pkg.data.type === 'module'; +} + +function runMainESM(mainPath) { + const esmLoader = require('internal/process/esm_loader'); + const { pathToFileURL } = require('internal/url'); + const { hasUncaughtExceptionCaptureCallback } = + require('internal/process/execution'); + return (esmLoader.initializeLoader() || Promise.resolve()).then(() => { + const main = path.isAbsolute(mainPath) ? + pathToFileURL(mainPath).href : mainPath; + return esmLoader.ESMLoader.import(main).catch((e) => { + if (hasUncaughtExceptionCaptureCallback()) { + process._fatalException(e); + return; + } + internalBinding('errors').triggerUncaughtException( + e, + true /* fromPromise */ + ); + }); + }); +} + + module.exports = { patchProcessObject, + resolveMainPath, + runMainESM, setupCoverageHooks, setupWarningHandler, setupDebugEnv, + shouldUseESMLoader, prepareMainThreadExecution, initializeDeprecations, initializeESMLoader, diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 67c5d1ecf49469..94690e87800a5e 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -73,7 +73,7 @@ const { resolveMainPath, shouldUseESMLoader, runMainESM -} = require('internal/process/main'); +} = require('internal/bootstrap/pre_execution'); const pendingDeprecation = getOptionValue('--pending-deprecation'); module.exports = { wrapSafe, Module, toRealPath, readPackageScope }; diff --git a/lib/internal/process/main.js b/lib/internal/process/main.js deleted file mode 100644 index 2948040973b538..00000000000000 --- a/lib/internal/process/main.js +++ /dev/null @@ -1,61 +0,0 @@ -'use strict'; -const path = require('path'); -const { getOptionValue } = require('internal/options'); - -exports.resolveMainPath = resolveMainPath; -function resolveMainPath(main) { - const { toRealPath, Module: CJSModule } = - require('internal/modules/cjs/loader'); - - // Note extension resolution for the main entry point can be deprecated in a - // future major. - let mainPath = CJSModule._findPath(path.resolve(main), null, true); - if (!mainPath) - return; - - const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); - if (!preserveSymlinksMain) - mainPath = toRealPath(mainPath); - - return mainPath; -} - -exports.shouldUseESMLoader = shouldUseESMLoader; -function shouldUseESMLoader(mainPath) { - const experimentalModules = getOptionValue('--experimental-modules'); - if (!experimentalModules) - return false; - const userLoader = getOptionValue('--experimental-loader'); - if (userLoader) - return true; - // Determine the module format of the main - if (mainPath && mainPath.endsWith('.mjs')) - return true; - if (!mainPath || mainPath.endsWith('.cjs')) - return false; - const { readPackageScope } = require('internal/modules/cjs/loader'); - const pkg = readPackageScope(mainPath); - return pkg && pkg.data.type === 'module'; -} - -exports.runMainESM = runMainESM; -function runMainESM(mainPath) { - const esmLoader = require('internal/process/esm_loader'); - const { pathToFileURL } = require('internal/url'); - const { hasUncaughtExceptionCaptureCallback } = - require('internal/process/execution'); - return (esmLoader.initializeLoader() || Promise.resolve()).then(() => { - const main = path.isAbsolute(mainPath) ? - pathToFileURL(mainPath).href : mainPath; - return esmLoader.ESMLoader.import(main).catch((e) => { - if (hasUncaughtExceptionCaptureCallback()) { - process._fatalException(e); - return; - } - internalBinding('errors').triggerUncaughtException( - e, - true /* fromPromise */ - ); - }); - }); -} diff --git a/node.gyp b/node.gyp index 2cfb1db476fbf2..9bf8fa2b994da1 100644 --- a/node.gyp +++ b/node.gyp @@ -160,7 +160,6 @@ 'lib/internal/priority_queue.js', 'lib/internal/process/esm_loader.js', 'lib/internal/process/execution.js', - 'lib/internal/process/main.js', 'lib/internal/process/main_thread_only.js', 'lib/internal/process/per_thread.js', 'lib/internal/process/policy.js', diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index ec99bb1b091202..939658a3b0553f 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -51,7 +51,6 @@ const expectedModules = new Set([ 'NativeModule internal/options', 'NativeModule internal/priority_queue', 'NativeModule internal/process/execution', - 'NativeModule internal/process/main', 'NativeModule internal/process/per_thread', 'NativeModule internal/process/promises', 'NativeModule internal/process/task_queues', From ad0f5c74758fdb322db08f1f5d389026eabafef0 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 14 Oct 2019 21:15:09 -0400 Subject: [PATCH 10/14] pr feedback --- lib/internal/bootstrap/loaders.js | 9 ++++----- lib/internal/modules/cjs/loader.js | 6 +++--- test/addons/async-hooks-promise/test.js | 2 -- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 78a0bfe3bcf9c6..48afed2556a51b 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -155,9 +155,6 @@ function NativeModule(id) { this.loaded = false; this.loading = false; this.canBeRequiredByUsers = !id.startsWith('internal/'); - // When using --expose-internals, we do not want to reflect the named exports - // from core modules as this can trigger getters. - this.defaultOnly = false; } // To be called during pre-execution when --expose-internals is on. @@ -167,7 +164,6 @@ NativeModule.exposeInternals = function() { // Do not expose this to user land even with --expose-internals. if (id !== loaderId) { mod.canBeRequiredByUsers = true; - mod.defaultOnly = true; } } }; @@ -224,7 +220,10 @@ NativeModule.prototype.compileForPublicLoader = function(needToSyncExports) { this.compile(); if (needToSyncExports) { if (!this.exportKeys) { - this.exportKeys = this.defaultOnly ? [] : Object.keys(this.exports); + // When using --expose-internals, we do not want to reflect the named + // exports from core modules as this can trigger unnecessary getters. + const internal = this.id.startsWith('internal/'); + this.exportKeys = internal ? [] : Object.keys(this.exports); } this.getESMFacade(); this.syncExports(); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 94690e87800a5e..77a74bb77ea0b9 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1042,11 +1042,11 @@ Module._extensions['.node'] = function(module, filename) { }; // Bootstrap main module. -Module.runMain = function(main) { - const resolvedMain = resolveMainPath(process.argv[1]); +Module.runMain = function(main = process.argv[1]) { + const resolvedMain = resolveMainPath(main); const useESMLoader = shouldUseESMLoader(resolvedMain); if (useESMLoader) { - runMainESM(resolvedMain || process.argv[1]); + runMainESM(resolvedMain || main); } else { Module._load(main, null, true); } diff --git a/test/addons/async-hooks-promise/test.js b/test/addons/async-hooks-promise/test.js index 4109c75809ce09..7baa96c8a0b9e2 100644 --- a/test/addons/async-hooks-promise/test.js +++ b/test/addons/async-hooks-promise/test.js @@ -11,7 +11,6 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) { } // Baseline to make sure the internal field isn't being set. -// Because of bootstrap tracking this will be enabled. assert.strictEqual( binding.getPromiseField(Promise.resolve(1)).isChainedPromise, undefined); @@ -19,7 +18,6 @@ assert.strictEqual( const hook0 = async_hooks.createHook({}).enable(); // Check that no PromiseWrap is created when there are no hook callbacks. -// Because of bootstrap tracking this will be enabled. assert.strictEqual( binding.getPromiseField(Promise.resolve(1)).isChainedPromise, undefined); From 29d1d9c376ea4f39271a6c2326aea4efeb1216c7 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 14 Oct 2019 21:21:11 -0400 Subject: [PATCH 11/14] async_hooks fix --- test/addons/async-hooks-promise/test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/addons/async-hooks-promise/test.js b/test/addons/async-hooks-promise/test.js index 7baa96c8a0b9e2..a6c48e94a34f07 100644 --- a/test/addons/async-hooks-promise/test.js +++ b/test/addons/async-hooks-promise/test.js @@ -12,15 +12,15 @@ if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) { // Baseline to make sure the internal field isn't being set. assert.strictEqual( - binding.getPromiseField(Promise.resolve(1)).isChainedPromise, - undefined); + binding.getPromiseField(Promise.resolve(1)), + 0); const hook0 = async_hooks.createHook({}).enable(); // Check that no PromiseWrap is created when there are no hook callbacks. assert.strictEqual( - binding.getPromiseField(Promise.resolve(1)).isChainedPromise, - undefined); + binding.getPromiseField(Promise.resolve(1)), + 0); hook0.disable(); From 91a3772edddfc6938ad43295e6923a18cb09450c Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Tue, 15 Oct 2019 09:56:54 -0400 Subject: [PATCH 12/14] esm loader initialize as async --- lib/internal/bootstrap/pre_execution.js | 2 +- lib/internal/process/esm_loader.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index c2d72fb0baf272..80ac97ee450efa 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -477,7 +477,7 @@ function runMainESM(mainPath) { const { pathToFileURL } = require('internal/url'); const { hasUncaughtExceptionCaptureCallback } = require('internal/process/execution'); - return (esmLoader.initializeLoader() || Promise.resolve()).then(() => { + return esmLoader.initializeLoader().then(() => { const main = path.isAbsolute(mainPath) ? pathToFileURL(mainPath).href : mainPath; return esmLoader.ESMLoader.import(main).catch((e) => { diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 28fd85f4c08679..49463e284c541f 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -40,7 +40,7 @@ exports.ESMLoader = ESMLoader; let calledInitialize = false; exports.initializeLoader = initializeLoader; -function initializeLoader() { +async function initializeLoader() { assert(calledInitialize === false); calledInitialize = true; if (!userLoader) From f638d29526b4624452ff62ec9cb9f4414d2dcd69 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 16 Oct 2019 13:43:27 -0400 Subject: [PATCH 13/14] fixup worker test --- test/parallel/test-bootstrap-modules.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 939658a3b0553f..c963630f5d32ab 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -92,7 +92,6 @@ if (common.isMainThread) { expectedModules.add('NativeModule internal/streams/state'); expectedModules.add('NativeModule internal/worker'); expectedModules.add('NativeModule internal/worker/io'); - expectedModules.add('NativeModule module'); expectedModules.add('NativeModule stream'); expectedModules.add('NativeModule worker_threads'); } From 673257d333253e34e6c2529958adc1ee80da8d0f Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Wed, 16 Oct 2019 13:47:52 -0400 Subject: [PATCH 14/14] feedback --- lib/internal/main/run_main_module.js | 3 +++ lib/internal/modules/cjs/loader.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js index 9f7e2bfbdc94cc..77d997b97a1c7a 100644 --- a/lib/internal/main/run_main_module.js +++ b/lib/internal/main/run_main_module.js @@ -10,4 +10,7 @@ const CJSModule = require('internal/modules/cjs/loader').Module; markBootstrapComplete(); +// Note: this loads the module through the ESM loader if +// --experimental-loader is provided or --experimental-modules is on +// and the module is determined to be an ES module CJSModule.runMain(process.argv[1]); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 77a74bb77ea0b9..a28383690ae2d9 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -844,7 +844,7 @@ Module.prototype.load = function(filename) { new ModuleWrap(url, undefined, ['default'], function() { this.setExport('default', exports); }) - , false, false) + , false /* isMain */, false /* inspectBrk */) ); } }