From 28055ff235ad0ce765f5242433e1751bad493cbd Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Mon, 12 Jun 2023 23:42:06 +0200 Subject: [PATCH] esm: remove CLI flag limitation to programmatic registration --- lib/internal/errors.js | 5 -- lib/internal/modules/esm/loader.js | 70 ++++++++++--------- lib/internal/process/esm_loader.js | 10 ++- .../test-esm-loader-programmatically.mjs | 31 ++++---- 4 files changed, 63 insertions(+), 53 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 651a2fa99cb715..df3fcb13873015 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1036,11 +1036,6 @@ E('ERR_ENCODING_INVALID_ENCODED_DATA', function(encoding, ret) { }, TypeError); E('ERR_ENCODING_NOT_SUPPORTED', 'The "%s" encoding is not supported', RangeError); -E('ERR_ESM_LOADER_REGISTRATION_UNAVAILABLE', 'Programmatically registering custom ESM loaders ' + - 'currently requires at least one custom loader to have been registered via the --experimental-loader ' + - 'flag. A no-op loader registered via CLI is sufficient (for example: `--experimental-loader ' + - '"data:text/javascript,"` with the necessary trailing comma). A future version of Node.js ' + - 'will remove this requirement.', Error); E('ERR_EVAL_ESM_CANNOT_PRINT', '--print cannot be used with ESM input', Error); E('ERR_EVENT_RECURSION', 'The event "%s" is already being dispatched', Error); E('ERR_FALSY_VALUE_REJECTION', function(reason) { diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index b73ba2eb3c8154..5919faa56afa9c 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -11,16 +11,18 @@ const { } = primordials; const { - ERR_ESM_LOADER_REGISTRATION_UNAVAILABLE, ERR_UNKNOWN_MODULE_FORMAT, } = require('internal/errors').codes; const { getOptionValue } = require('internal/options'); const { pathToFileURL } = require('internal/url'); -const { emitExperimentalWarning } = require('internal/util'); +const { emitExperimentalWarning, kEmptyObject } = require('internal/util'); const { getDefaultConditions, } = require('internal/modules/esm/utils'); let defaultResolve, defaultLoad, importMetaInitializer; +let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { + debug = fn; +}); function newModuleMap() { const ModuleMap = require('internal/modules/esm/module_map'); @@ -106,6 +108,7 @@ class DefaultModuleLoader { setCallbackForWrap(module, { initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), importModuleDynamically: (specifier, { url }, importAssertions) => { + debug('importModuleDynamically: %o', { specifier, url, Loader: this.constructor.name }); return this.import(specifier, url, importAssertions); }, }); @@ -113,8 +116,7 @@ class DefaultModuleLoader { return module; }; const ModuleJob = require('internal/modules/esm/module_job'); - const job = new ModuleJob( - this, url, undefined, evalInstance, false, false); + const job = new ModuleJob(this, url, undefined, evalInstance, false, false); this.moduleMap.set(url, undefined, job); const { module } = await job.run(); @@ -285,10 +287,21 @@ class CustomizedModuleLoader extends DefaultModuleLoader { /** * Instantiate a module loader that uses user-provided custom loader hooks. */ - constructor() { + constructor({ + cjsCache, + evalIndex, + moduleMap, + } = kEmptyObject) { super(); - getHooksProxy(); + if (cjsCache != null) { this.cjsCache = cjsCache; } + if (evalIndex != null) { this.evalIndex = evalIndex; } + if (moduleMap != null) { this.moduleMap = moduleMap; } + + if (!hooksProxy) { + const { HooksProxy } = require('internal/modules/esm/hooks'); + hooksProxy = new HooksProxy(); + } } /** @@ -357,40 +370,29 @@ let emittedExperimentalWarning = false; * A loader instance is used as the main entry point for loading ES modules. Currently, this is a singleton; there is * only one used for loading the main module and everything in its dependency graph, though separate instances of this * class might be instantiated as part of bootstrap for other purposes. - * @param {boolean} useCustomLoadersIfPresent If the user has provided loaders via the --loader flag, use them. + * @param {boolean} forceCustomizedLoaderInMain Ignore whether custom loader(s) have been provided + * via CLI and instantiate a CustomizedModuleLoader instance regardless. * @returns {DefaultModuleLoader | CustomizedModuleLoader} */ -function createModuleLoader(useCustomLoadersIfPresent = true) { - if (useCustomLoadersIfPresent && - // Don't spawn a new worker if we're already in a worker thread created by instantiating CustomizedModuleLoader; - // doing so would cause an infinite loop. - !require('internal/modules/esm/utils').isLoaderWorker()) { - const userLoaderPaths = getOptionValue('--experimental-loader'); - if (userLoaderPaths.length > 0) { +function createModuleLoader(customizationSetup) { + // Don't spawn a new worker if we're already in a worker thread (doing so would cause an infinite loop). + if (!require('internal/modules/esm/utils').isLoaderWorker()) { + if ( + customizationSetup || + getOptionValue('--experimental-loader').length > 0 + ) { if (!emittedExperimentalWarning) { emitExperimentalWarning('Custom ESM Loaders'); emittedExperimentalWarning = true; } - return new CustomizedModuleLoader(); + debug('instantiating CustomizedModuleLoader'); + return new CustomizedModuleLoader(customizationSetup); } } return new DefaultModuleLoader(); } -/** - * Get the HooksProxy instance. If it is not defined, then create a new one. - * @returns {HooksProxy} - */ -function getHooksProxy() { - if (!hooksProxy) { - const { HooksProxy } = require('internal/modules/esm/hooks'); - hooksProxy = new HooksProxy(); - } - - return hooksProxy; -} - /** * Register a single loader programmatically. * @param {string} specifier @@ -405,12 +407,13 @@ function getHooksProxy() { * ``` */ function register(specifier, parentURL = 'data:') { - // TODO: Remove this limitation in a follow-up before `register` is released publicly - if (getOptionValue('--experimental-loader').length < 1) { - throw new ERR_ESM_LOADER_REGISTRATION_UNAVAILABLE(); - } + let moduleLoader = require('internal/process/esm_loader').esmLoader; - const moduleLoader = require('internal/process/esm_loader').esmLoader; + if (!(moduleLoader instanceof CustomizedModuleLoader)) { + debug('register called on DefaultModuleLoader; switching to CustomizedModuleLoader'); + moduleLoader = createModuleLoader(moduleLoader); + require('internal/process/esm_loader').esmLoader = moduleLoader; + } moduleLoader.register(`${specifier}`, parentURL); } @@ -418,6 +421,5 @@ function register(specifier, parentURL = 'data:') { module.exports = { DefaultModuleLoader, createModuleLoader, - getHooksProxy, register, }; diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 9a84ed944e87c4..8d01b1870e6dd1 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -11,15 +11,21 @@ const { } = require('internal/process/execution'); const { pathToFileURL } = require('internal/url'); const { kEmptyObject } = require('internal/util'); +let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { + debug = fn; +}); let esmLoader; module.exports = { get esmLoader() { - return esmLoader ??= createModuleLoader(true); + return esmLoader ??= createModuleLoader(); + }, + set esmLoader(supplantingLoader) { + if (supplantingLoader) esmLoader = supplantingLoader; }, async loadESM(callback) { - esmLoader ??= createModuleLoader(true); + esmLoader ??= createModuleLoader(); try { const userImports = getOptionValue('--import'); if (userImports.length > 0) { diff --git a/test/es-module/test-esm-loader-programmatically.mjs b/test/es-module/test-esm-loader-programmatically.mjs index 0c20bbcb7519f8..26d18c6b53f93c 100644 --- a/test/es-module/test-esm-loader-programmatically.mjs +++ b/test/es-module/test-esm-loader-programmatically.mjs @@ -189,19 +189,26 @@ describe('ESM: programmatically register loaders', { concurrency: true }, () => assert.strictEqual(lines[3], ''); }); - it('does not work without dummy CLI loader', async () => { - const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ - '--input-type=module', - '--eval', - "import { register } from 'node:module';" + - commonEvals.register(fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs')) + - commonEvals.dynamicImport('console.log("Hello from dynamic import");'), - ]); + it('works without a CLI flag', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [ + '--no-warnings', + '--input-type=module', + '--eval', + "import { register } from 'node:module';" + + commonEvals.register(fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs')) + + commonEvals.dynamicImport('console.log("Hello from dynamic import");'), + ]); - assert.strictEqual(stdout, ''); - assert.strictEqual(code, 1); - assert.strictEqual(signal, null); - assert.match(stderr, /ERR_ESM_LOADER_REGISTRATION_UNAVAILABLE/); + assert.strictEqual(stderr, ''); + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + + const lines = stdout.split('\n'); + + assert.match(lines[0], /load passthru/); + assert.match(lines[1], /Hello from dynamic import/); + + assert.strictEqual(lines[2], ''); }); it('does not work with a loader specifier that does not exist', async () => {