From d859e9e997a3de9bdd7e116b2878acc2a3cf4bd6 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Wed, 4 May 2022 17:51:12 +0200 Subject: [PATCH] esm: add chaining to loaders PR-URL: https://github.com/nodejs/node/pull/42623 Reviewed-By: Geoffrey Booth Reviewed-By: Antoine du Hamel --- doc/api/errors.md | 11 + doc/api/esm.md | 234 ++++++---- lib/internal/errors.js | 7 + lib/internal/modules/esm/loader.js | 411 +++++++++++++----- lib/internal/modules/run_main.js | 8 +- lib/internal/process/esm_loader.js | 8 +- lib/internal/util/types.js | 2 +- src/node_options.cc | 2 +- src/node_options.h | 2 +- .../test-esm-experimental-warnings.mjs | 27 +- test/es-module/test-esm-loader-chaining.mjs | 352 +++++++++++++++ test/es-module/test-esm-loader-hooks.mjs | 17 +- .../es-module/test-esm-loader-invalid-url.mjs | 4 +- test/es-module/test-esm-tla-unfinished.mjs | 34 +- .../builtin-named-exports-loader.mjs | 2 + .../es-module-loaders/example-loader.mjs | 2 + .../es-module-loaders/hooks-custom.mjs | 6 + .../loader-invalid-format.mjs | 4 +- .../es-module-loaders/loader-invalid-url.mjs | 1 + .../loader-load-bad-next-context.mjs | 3 + .../loader-load-bad-next-url.mjs | 3 + .../loader-load-foo-or-42.mjs | 11 + .../loader-load-impersonating-next-url.mjs | 3 + .../loader-load-incomplete.mjs | 6 + .../loader-load-next-modified.mjs | 11 + .../loader-load-passing-modified-context.mjs | 6 + .../loader-load-passthru.mjs | 4 + ...loader-load-receiving-modified-context.mjs | 4 + .../es-module-loaders/loader-resolve-42.mjs | 4 + .../loader-resolve-bad-next-context.mjs | 3 + .../loader-resolve-bad-next-specifier.mjs | 3 + .../es-module-loaders/loader-resolve-foo.mjs | 4 + .../loader-resolve-incomplete.mjs | 5 + .../loader-resolve-next-modified.mjs | 11 + ...oader-resolve-passing-modified-context.mjs | 6 + .../loader-resolve-passthru.mjs | 4 + ...der-resolve-receiving-modified-context.mjs | 4 + .../loader-resolve-shortcircuit.mjs | 6 + .../es-module-loaders/mock-loader.mjs | 5 + .../es-module-loaders/string-sources.mjs | 7 +- test/parallel/test-module-main-fail.js | 5 +- 41 files changed, 1032 insertions(+), 220 deletions(-) create mode 100644 test/es-module/test-esm-loader-chaining.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-bad-next-context.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-bad-next-url.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-foo-or-42.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-impersonating-next-url.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-incomplete.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-next-modified.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-passing-modified-context.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-passthru.mjs create mode 100644 test/fixtures/es-module-loaders/loader-load-receiving-modified-context.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-42.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-bad-next-context.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-bad-next-specifier.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-foo.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-incomplete.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-next-modified.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-passing-modified-context.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-passthru.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-receiving-modified-context.mjs create mode 100644 test/fixtures/es-module-loaders/loader-resolve-shortcircuit.mjs diff --git a/doc/api/errors.md b/doc/api/errors.md index 5bc949d5b12c44..801f22727f3305 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2092,6 +2092,17 @@ An attempt was made to open an IPC communication channel with a synchronously forked Node.js process. See the documentation for the [`child_process`][] module for more information. + + +### `ERR_LOADER_CHAIN_INCOMPLETE` + + + +An ESM loader hook returned without calling `next()` and without explicitly +signaling a short circuit. + ### `ERR_MANIFEST_ASSERT_INTEGRITY` diff --git a/doc/api/esm.md b/doc/api/esm.md index 3d09cc03bba696..fd128397f4981f 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -7,6 +7,10 @@ > The loaders API is being redesigned. This hook may disappear or its > signature may change. Do not rely on the API described below. @@ -785,15 +835,21 @@ export async function resolve(specifier, context, defaultResolve) { > In a previous version of this API, this was split across 3 separate, now > deprecated, hooks (`getFormat`, `getSource`, and `transformSource`). -* `url` {string} +* `url` {string} The URL returned by the `resolve` chain * `context` {Object} + * `conditions` {string\[]} Export conditions of the relevant `package.json` * `format` {string|null|undefined} The format optionally supplied by the - `resolve` hook. + `resolve` hook chain * `importAssertions` {Object} -* `defaultLoad` {Function} +* `nextLoad` {Function} The subsequent `load` hook in the chain, or the + Node.js default `load` hook after the last user-supplied `load` hook + * `specifier` {string} + * `context` {Object} * Returns: {Object} * `format` {string} - * `source` {string|ArrayBuffer|TypedArray} + * `shortCircuit` {undefined|boolean} A signal that this hook intends to + terminate the chain of `resolve` hooks. **Default:** `false` + * `source` {string|ArrayBuffer|TypedArray} The source for Node.js to evaluate The `load` hook provides a way to define a custom method of determining how a URL should be interpreted, retrieved, and parsed. It is also in charge of @@ -834,20 +890,10 @@ avoid reading files from disk. It could also be used to map an unrecognized format to a supported one, for example `yaml` to `module`. ```js -/** - * @param {string} url - * @param {{ - format: string, - }} context If resolve settled with a `format`, that value is included here. - * @param {Function} defaultLoad - * @returns {Promise<{ - format: string, - source: string | ArrayBuffer | SharedArrayBuffer | Uint8Array, - }>} - */ -export async function load(url, context, defaultLoad) { +export async function load(url, context, nextLoad) { const { format } = context; - if (Math.random() > 0.5) { // Some condition. + + if (Math.random() > 0.5) { // Some condition /* For some or all URLs, do some custom logic for retrieving the source. Always return an object of the form { @@ -857,11 +903,13 @@ export async function load(url, context, defaultLoad) { */ return { format, + shortCircuit: true, source: '...', }; } - // Defer to Node.js for all other URLs. - return defaultLoad(url, context, defaultLoad); + + // Defer to the next hook in the chain. + return nextLoad(url, context); } ``` @@ -870,13 +918,22 @@ source to a supported one (see [Examples](#examples) below). #### `globalPreload()` + + > The loaders API is being redesigned. This hook may disappear or its > signature may change. Do not rely on the API described below. > In a previous version of this API, this hook was named > `getGlobalPreloadCode`. -* Returns: {string} +* `context` {Object} Information to assist the preload code + * `port` {MessagePort} +* Returns: {string} Code to run before application startup Sometimes it might be necessary to run some code inside of the same global scope that the application runs in. This hook allows the return of a string @@ -890,13 +947,7 @@ If the code needs more advanced `require` features, it has to construct its own `require` using `module.createRequire()`. ```js -/** - * @param {{ - port: MessagePort, - }} utilities Things that preload code might find useful - * @returns {string} Code to run before application startup - */ -export function globalPreload(utilities) { +export function globalPreload(context) { return `\ globalThis.someInjectedProperty = 42; console.log('I just set some globals!'); @@ -921,10 +972,6 @@ close normally. /** * This example has the application context send a message to the loader * and sends the message back to the application context - * @param {{ - port: MessagePort, - }} utilities Things that preload code might find useful - * @returns {string} Code to run before application startup */ export function globalPreload({ port }) { port.onmessage = (evt) => { @@ -946,9 +993,11 @@ customizations of Node.js’ code loading and evaluation behaviors. #### HTTPS loader -In current Node.js, specifiers starting with `https://` are unsupported. The -loader below registers hooks to enable rudimentary support for such specifiers. -While this may seem like a significant improvement to Node.js core +In current Node.js, specifiers starting with `https://` are experimental (see +[HTTPS and HTTP imports][]). + +The loader below registers hooks to enable rudimentary support for such +specifiers. While this may seem like a significant improvement to Node.js core functionality, there are substantial downsides to actually using this loader: performance is much slower than loading files from disk, there is no caching, and there is no security. @@ -957,7 +1006,7 @@ and there is no security. // https-loader.mjs import { get } from 'node:https'; -export function resolve(specifier, context, defaultResolve) { +export function resolve(specifier, context, nextResolve) { const { parentURL = null } = context; // Normally Node.js would error on specifiers starting with 'https://', so @@ -965,19 +1014,21 @@ export function resolve(specifier, context, defaultResolve) { // passed along to the later hooks below. if (specifier.startsWith('https://')) { return { + shortCircuit: true, url: specifier }; } else if (parentURL && parentURL.startsWith('https://')) { return { - url: new URL(specifier, parentURL).href + shortCircuit: true, + url: new URL(specifier, parentURL).href, }; } // Let Node.js handle all other specifiers. - return defaultResolve(specifier, context, defaultResolve); + return nextResolve(specifier, context); } -export function load(url, context, defaultLoad) { +export function load(url, context, nextLoad) { // For JavaScript to be loaded over the network, we need to fetch and // return it. if (url.startsWith('https://')) { @@ -989,6 +1040,7 @@ export function load(url, context, defaultLoad) { // This example assumes all network-provided JavaScript is ES module // code. format: 'module', + shortCircuit: true, source: data, })); }).on('error', (err) => reject(err)); @@ -996,7 +1048,7 @@ export function load(url, context, defaultLoad) { } // Let Node.js handle all other URLs. - return defaultLoad(url, context, defaultLoad); + return nextLoad(url, context); } ``` @@ -1036,27 +1088,29 @@ const baseURL = pathToFileURL(`${cwd()}/`).href; // CoffeeScript files end in .coffee, .litcoffee or .coffee.md. const extensionsRegex = /\.coffee$|\.litcoffee$|\.coffee\.md$/; -export async function resolve(specifier, context, defaultResolve) { - const { parentURL = baseURL } = context; - - // Node.js normally errors on unknown file extensions, so return a URL for - // specifiers ending in the CoffeeScript file extensions. +export async function resolve(specifier, context, nextResolve) { if (extensionsRegex.test(specifier)) { + const { parentURL = baseURL } = context; + + // Node.js normally errors on unknown file extensions, so return a URL for + // specifiers ending in the CoffeeScript file extensions. return { + shortCircuit: true, url: new URL(specifier, parentURL).href }; } // Let Node.js handle all other specifiers. - return defaultResolve(specifier, context, defaultResolve); + return nextResolve(specifier, context); } -export async function load(url, context, defaultLoad) { - // Now that we patched resolve to let CoffeeScript URLs through, we need to - // tell Node.js what format such URLs should be interpreted as. Because - // CoffeeScript transpiles into JavaScript, it should be one of the two - // JavaScript formats: 'commonjs' or 'module'. +export async function load(url, context, nextLoad) { if (extensionsRegex.test(url)) { + // Now that we patched resolve to let CoffeeScript URLs through, we need to + // tell Node.js what format such URLs should be interpreted as. Because + // CoffeeScript transpiles into JavaScript, it should be one of the two + // JavaScript formats: 'commonjs' or 'module'. + // CoffeeScript files can be either CommonJS or ES modules, so we want any // CoffeeScript file to be treated by Node.js the same as a .js file at the // same location. To determine how Node.js would interpret an arbitrary .js @@ -1069,25 +1123,26 @@ export async function load(url, context, defaultLoad) { // loader. Avoiding the need for a separate CommonJS handler is a future // enhancement planned for ES module loaders. if (format === 'commonjs') { - return { format }; + return { + format, + shortCircuit: true, + }; } - const { source: rawSource } = await defaultLoad(url, { format }); + const { source: rawSource } = await nextLoad(url, { ...context, format }); // This hook converts CoffeeScript source code into JavaScript source code // for all imported CoffeeScript files. - const transformedSource = CoffeeScript.compile(rawSource.toString(), { - bare: true, - filename: url, - }); + const transformedSource = coffeeCompile(rawSource.toString(), url); return { format, + shortCircuit: true, source: transformedSource, }; } // Let Node.js handle all other URLs. - return defaultLoad(url, context, defaultLoad); + return nextLoad(url, context); } async function getPackageType(url) { @@ -1495,6 +1550,7 @@ success! [Determining module system]: packages.md#determining-module-system [Dynamic `import()`]: https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Dynamic_Imports [ES Module Integration Proposal for WebAssembly]: https://github.com/webassembly/esm-integration +[HTTPS and HTTP imports]: #https-and-http-imports [Import Assertions]: #import-assertions [Import Assertions proposal]: https://github.com/tc39/proposal-import-assertions [JSON modules]: #json-modules @@ -1525,9 +1581,9 @@ success! [`util.TextDecoder`]: util.md#class-utiltextdecoder [cjs-module-lexer]: https://github.com/nodejs/cjs-module-lexer/tree/1.2.2 [custom https loader]: #https-loader -[load hook]: #loadurl-context-defaultload +[load hook]: #loadurl-context-nextload [percent-encoded]: url.md#percent-encoding-in-urls -[resolve hook]: #resolvespecifier-context-defaultresolve +[resolve hook]: #resolvespecifier-context-nextresolve [special scheme]: https://url.spec.whatwg.org/#special-scheme [status code]: process.md#exit-codes [the official standard format]: https://tc39.github.io/ecma262/#sec-modules diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 2185e43a87988b..0afe58843aec5e 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1359,6 +1359,13 @@ E('ERR_IPC_CHANNEL_CLOSED', 'Channel closed', Error); E('ERR_IPC_DISCONNECTED', 'IPC channel is already disconnected', Error); E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe', Error); E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks', Error); +E( + 'ERR_LOADER_CHAIN_INCOMPLETE', + 'The "%s" hook from %s did not call the next hook in its chain and did not' + + ' explicitly signal a short circuit. If this is intentional, include' + + ' `shortCircuit: true` in the hook\'s return.', + Error +); E('ERR_MANIFEST_ASSERT_INTEGRITY', (moduleURL, realIntegrities) => { let msg = `The content of "${ diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 39f8ebca59e9df..8bc22ef697db3e 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -23,12 +23,13 @@ const { const { MessageChannel } = require('internal/worker/io'); const { + ERR_LOADER_CHAIN_INCOMPLETE, ERR_INTERNAL_ASSERTION, ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_INVALID_RETURN_PROPERTY_VALUE, ERR_INVALID_RETURN_VALUE, - ERR_UNKNOWN_MODULE_FORMAT + ERR_UNKNOWN_MODULE_FORMAT, } = require('internal/errors').codes; const { pathToFileURL, isURLInstance, URL } = require('internal/url'); const { emitExperimentalWarning } = require('internal/util'); @@ -36,6 +37,10 @@ const { isAnyArrayBuffer, isArrayBufferView, } = require('internal/util/types'); +const { + validateObject, + validateString, +} = require('internal/validators'); const ModuleMap = require('internal/modules/esm/module_map'); const ModuleJob = require('internal/modules/esm/module_job'); @@ -56,10 +61,39 @@ const { /** - * Prevent the specifier resolution warning from being printed twice + * @typedef {object} ExportedHooks + * @property {Function} globalPreload + * @property {Function} resolve + * @property {Function} load + */ + +/** + * @typedef {Record} ModuleExports + */ + +/** + * @typedef {object} KeyedExports + * @property {ModuleExports} exports + * @property {URL['href']} url + */ + +/** + * @typedef {object} KeyedHook + * @property {Function} fn + * @property {URL['href']} url + */ + +/** + * @typedef {'builtin'|'commonjs'|'json'|'module'|'wasm'} ModuleFormat + */ + +/** + * @typedef {ArrayBuffer|TypedArray|string} ModuleSource */ -let emittedSpecifierResolutionWarning = false; +// [2] `validate...()`s throw the wrong error + +let emittedSpecifierResolutionWarning = false; /** * An ESMLoader instance is used as the main entry point for loading ES modules. @@ -70,27 +104,35 @@ class ESMLoader { /** * Prior to ESM loading. These are called once before any modules are started. * @private - * @property {Function[]} globalPreloaders First-in-first-out list of - * preload hooks. + * @property {KeyedHook[]} globalPreloaders Last-in-first-out + * list of preload hooks. */ #globalPreloaders = []; /** * Phase 2 of 2 in ESM loading. * @private - * @property {Function[]} loaders First-in-first-out list of loader hooks. + * @property {KeyedHook[]} loaders Last-in-first-out + * collection of loader hooks. */ #loaders = [ - defaultLoad, + { + fn: defaultLoad, + url: 'node:internal/modules/esm/load', + }, ]; /** * Phase 1 of 2 in ESM loading. * @private - * @property {Function[]} resolvers First-in-first-out list of resolver hooks + * @property {KeyedHook[]} resolvers Last-in-first-out + * collection of resolver hooks. */ #resolvers = [ - defaultResolve, + { + fn: defaultResolve, + url: 'node:internal/modules/esm/resolve', + }, ]; #importMetaInitializer = initializeImportMeta; @@ -116,13 +158,16 @@ class ESMLoader { translators = translators; constructor() { - if (getOptionValue('--experimental-loader')) { + if (getOptionValue('--experimental-loader').length > 0) { emitExperimentalWarning('Custom ESM Loaders'); } if (getOptionValue('--experimental-network-imports')) { emitExperimentalWarning('Network Imports'); } - if (getOptionValue('--experimental-specifier-resolution') === 'node' && !emittedSpecifierResolutionWarning) { + if ( + !emittedSpecifierResolutionWarning && + getOptionValue('--experimental-specifier-resolution') === 'node' + ) { process.emitWarning( 'The Node.js specifier resolution flag is experimental. It could change or be removed at any time.', 'ExperimentalWarning' @@ -131,6 +176,11 @@ class ESMLoader { } } + /** + * + * @param {ModuleExports} exports + * @returns {ExportedHooks} + */ static pluckHooks({ globalPreload, resolve, @@ -194,34 +244,51 @@ class ESMLoader { /** * Collect custom/user-defined hook(s). After all hooks have been collected, * calls global preload hook(s). - * @param {object | object[]} customLoaders A list of exports from - * user-defined loaders (as returned by ESMLoader.import()). + * @param {KeyedExports} customLoaders + * A list of exports from user-defined loaders (as returned by + * ESMLoader.import()). */ async addCustomLoaders( customLoaders = [], ) { - if (!ArrayIsArray(customLoaders)) customLoaders = [customLoaders]; - for (let i = 0; i < customLoaders.length; i++) { - const exports = customLoaders[i]; + const { + exports, + url, + } = customLoaders[i]; const { globalPreloader, resolver, loader, } = ESMLoader.pluckHooks(exports); - if (globalPreloader) ArrayPrototypePush( - this.#globalPreloaders, - FunctionPrototypeBind(globalPreloader, null), // [1] - ); - if (resolver) ArrayPrototypePush( - this.#resolvers, - FunctionPrototypeBind(resolver, null), // [1] - ); - if (loader) ArrayPrototypePush( - this.#loaders, - FunctionPrototypeBind(loader, null), // [1] - ); + if (globalPreloader) { + ArrayPrototypePush( + this.#globalPreloaders, + { + fn: FunctionPrototypeBind(globalPreloader), // [1] + url, + }, + ); + } + if (resolver) { + ArrayPrototypePush( + this.#resolvers, + { + fn: FunctionPrototypeBind(resolver), // [1] + url, + }, + ); + } + if (loader) { + ArrayPrototypePush( + this.#loaders, + { + fn: FunctionPrototypeBind(loader), // [1] + url, + }, + ); + } } // [1] ensure hook function is not bound to ESMLoader instance @@ -286,7 +353,7 @@ class ESMLoader { // immediately and synchronously url = fetchModule(new URL(url), { parentURL: url }).resolvedHREF; // This should only occur if the module hasn't been fetched yet - if (typeof url !== 'string') { + if (typeof url !== 'string') { // [2] throw new ERR_INTERNAL_ASSERTION(`Base url for module ${url} not loaded.`); } } @@ -308,12 +375,17 @@ class ESMLoader { */ async getModuleJob(specifier, parentURL, importAssertions) { let importAssertionsForResolve; + + // By default, `this.#loaders` contains just the Node default load hook if (this.#loaders.length !== 1) { - // We can skip cloning if there are no user provided loaders because + // We can skip cloning if there are no user-provided loaders because // the Node.js default resolve hook does not use import assertions. - importAssertionsForResolve = - ObjectAssign(ObjectCreate(null), importAssertions); + importAssertionsForResolve = ObjectAssign( + ObjectCreate(null), + importAssertions, + ); } + const { format, url } = await this.resolve(specifier, parentURL, importAssertionsForResolve); @@ -391,11 +463,21 @@ class ESMLoader { * @param {string} parentURL Path of the parent importing the module. * @param {Record} importAssertions Validations for the * module import. - * @returns {Promise} A list of module export(s). + * @returns {Promise} + * A collection of module export(s) or a list of collections of module + * export(s). */ async import(specifiers, parentURL, importAssertions) { + // For loaders, `import` is passed multiple things to process, it returns a + // list pairing the url and exports collected. This is especially useful for + // error messaging, to identity from where an export came. But, in most + // cases, only a single url is being "imported" (ex `import()`), so there is + // only 1 possible url from which the exports were collected and it is + // already known to the caller. Nesting that in a list would only ever + // create redundant work for the caller, so it is later popped off the + // internal list. const wasArr = ArrayIsArray(specifiers); - if (!wasArr) specifiers = [specifiers]; + if (!wasArr) { specifiers = [specifiers]; } const count = specifiers.length; const jobs = new Array(count); @@ -408,36 +490,106 @@ class ESMLoader { const namespaces = await PromiseAll(new SafeArrayIterator(jobs)); - return wasArr ? - namespaces : - namespaces[0]; + if (!wasArr) { return namespaces[0]; } // We can skip the pairing below + + for (let i = 0; i < count; i++) { + const namespace = ObjectCreate(null); + namespace.url = specifiers[i]; + namespace.exports = namespaces[i]; + + namespaces[i] = namespace; + } + + return namespaces; } /** * Provide source that is understood by one of Node's translators. * - * The internals of this WILL change when chaining is implemented, - * depending on the resolution/consensus from #36954 - * @param {string} url The URL/path of the module to be loaded + * Internally, this behaves like a backwards iterator, wherein the stack of + * hooks starts at the top and each call to `nextLoad()` moves down 1 step + * until it reaches the bottom or short-circuits. + * + * @param {URL['href']} url The URL/path of the module to be loaded * @param {object} context Metadata about the module - * @returns {object} + * @returns {{ format: ModuleFormat, source: ModuleSource }} */ async load(url, context = {}) { - const defaultLoader = this.#loaders[0]; + const loaders = this.#loaders; + let hookIndex = loaders.length - 1; + let { + fn: loader, + url: loaderFilePath, + } = loaders[hookIndex]; + let chainFinished = hookIndex === 0; + let shortCircuited = false; + + const nextLoad = async (nextUrl, ctx = context) => { + --hookIndex; // `nextLoad` has been called, so decrement our pointer. + + ({ + fn: loader, + url: loaderFilePath, + } = loaders[hookIndex]); + + if (hookIndex === 0) { chainFinished = true; } + + const hookErrIdentifier = `${loaderFilePath} "load"`; + + if (typeof nextUrl !== 'string') { + // non-strings can be coerced to a url string + // validateString() throws a less-specific error + throw new ERR_INVALID_ARG_TYPE( + `${hookErrIdentifier} nextLoad(url)`, + 'a url string', + nextUrl, + ); + } - const loader = this.#loaders.length === 1 ? - defaultLoader : - this.#loaders[1]; - const loaded = await loader(url, context, defaultLoader); + // Try to avoid expensive URL instantiation for known-good urls + if (!this.moduleMap.has(nextUrl)) { + try { + new URL(nextUrl); + } catch { + throw new ERR_INVALID_ARG_VALUE( + `${hookErrIdentifier} nextLoad(url)`, + nextUrl, + 'should be a url string', + ); + } + } + + validateObject(ctx, `${hookErrIdentifier} nextLoad(, context)`); + + const output = await loader(nextUrl, ctx, nextLoad); + + if (output?.shortCircuit === true) { shortCircuited = true; } - if (typeof loaded !== 'object') { + return output; + }; + + const loaded = await loader( + url, + context, + nextLoad, + ); + + const hookErrIdentifier = `${loaderFilePath} load`; + + if (typeof loaded !== 'object') { // [2] throw new ERR_INVALID_RETURN_VALUE( - 'object', - 'loader load', + 'an object', + hookErrIdentifier, loaded, ); } + if (loaded?.shortCircuit === true) { shortCircuited = true; } + + if (!chainFinished && !shortCircuited) { + throw new ERR_LOADER_CHAIN_INCOMPLETE('load', loaderFilePath); + } + const { format, source, @@ -454,10 +606,10 @@ class ESMLoader { url); } - if (typeof format !== 'string') { + if (typeof format !== 'string') { // [2] throw new ERR_INVALID_RETURN_PROPERTY_VALUE( - 'string', - 'loader resolve', + 'a string', + hookErrIdentifier, 'format', format, ); @@ -468,12 +620,14 @@ class ESMLoader { typeof source !== 'string' && !isAnyArrayBuffer(source) && !isArrayBufferView(source) - ) throw ERR_INVALID_RETURN_PROPERTY_VALUE( - 'string, an ArrayBuffer, or a TypedArray', - 'loader load', - 'source', - source - ); + ) { + throw ERR_INVALID_RETURN_PROPERTY_VALUE( + 'a string, an ArrayBuffer, or a TypedArray', + hookErrIdentifier, + 'source', + source + ); + } return { format, @@ -482,10 +636,7 @@ class ESMLoader { } preload() { - const count = this.#globalPreloaders.length; - if (!count) return; - - for (let i = 0; i < count; i++) { + for (let i = this.#globalPreloaders.length - 1; i >= 0; i--) { const channel = new MessageChannel(); const { port1: insidePreload, @@ -495,16 +646,23 @@ class ESMLoader { insidePreload.unref(); insideLoader.unref(); - const preload = this.#globalPreloaders[i]({ - port: insideLoader + const { + fn: preloader, + url: specifier, + } = this.#globalPreloaders[i]; + + const preload = preloader({ + port: insideLoader, }); - if (preload == null) return; + if (preload == null) { return; } + + const hookErrIdentifier = `${specifier} globalPreload`; - if (typeof preload !== 'string') { + if (typeof preload !== 'string') { // [2] throw new ERR_INVALID_RETURN_VALUE( - 'string', - 'loader globalPreloadCode', + 'a string', + hookErrIdentifier, preload, ); } @@ -569,14 +727,16 @@ class ESMLoader { /** * Resolve the location of the module. * - * The internals of this WILL change when chaining is implemented, - * depending on the resolution/consensus from #36954. + * Internally, this behaves like a backwards iterator, wherein the stack of + * hooks starts at the top and each call to `nextResolve()` moves down 1 step + * until it reaches the bottom or short-circuits. + * * @param {string} originalSpecifier The specified URL path of the module to * be resolved. * @param {string} [parentURL] The URL path of the module's parent. * @param {ImportAssertions} [importAssertions] Assertions from the import * statement or expression. - * @returns {{ url: string }} + * @returns {{ format: string, url: URL['href'] }} */ async resolve( originalSpecifier, @@ -589,61 +749,118 @@ class ESMLoader { !isMain && typeof parentURL !== 'string' && !isURLInstance(parentURL) - ) throw new ERR_INVALID_ARG_TYPE( - 'parentURL', - ['string', 'URL'], + ) { + throw new ERR_INVALID_ARG_TYPE( + 'parentURL', + ['string', 'URL'], + parentURL, + ); + } + const resolvers = this.#resolvers; + + let hookIndex = resolvers.length - 1; + let { + fn: resolver, + url: resolverFilePath, + } = resolvers[hookIndex]; + let chainFinished = hookIndex === 0; + let shortCircuited = false; + + const context = { + conditions: DEFAULT_CONDITIONS, + importAssertions, parentURL, - ); + }; + + const nextResolve = async (suppliedSpecifier, ctx = context) => { + --hookIndex; // `nextResolve` has been called, so decrement our pointer. + + ({ + fn: resolver, + url: resolverFilePath, + } = resolvers[hookIndex]); + + if (hookIndex === 0) { chainFinished = true; } - const conditions = DEFAULT_CONDITIONS; + const hookErrIdentifier = `${resolverFilePath} "resolve"`; - const defaultResolver = this.#resolvers[0]; + validateString( + suppliedSpecifier, + `${hookErrIdentifier} nextResolve(specifier)`, + ); // non-strings can be coerced to a url string + + validateObject(ctx, `${hookErrIdentifier} nextResolve(, context)`); + + const output = await resolver(suppliedSpecifier, ctx, nextResolve); + + if (output?.shortCircuit === true) { shortCircuited = true; } + + return output; + }; - const resolver = this.#resolvers.length === 1 ? - defaultResolver : - this.#resolvers[1]; const resolution = await resolver( originalSpecifier, - { - conditions, - importAssertions, - parentURL, - }, - defaultResolver, + context, + nextResolve, ); - if (typeof resolution !== 'object') { + const hookErrIdentifier = `${resolverFilePath} resolve`; + + if (typeof resolution !== 'object') { // [2] throw new ERR_INVALID_RETURN_VALUE( - 'object', - 'loader resolve', + 'an object', + hookErrIdentifier, resolution, ); } - const { format, url } = resolution; + if (resolution?.shortCircuit === true) { shortCircuited = true; } + + if (!chainFinished && !shortCircuited) { + throw new ERR_LOADER_CHAIN_INCOMPLETE('resolve', resolverFilePath); + } + + const { + format, + url, + } = resolution; if ( format != null && - typeof format !== 'string' + typeof format !== 'string' // [2] ) { throw new ERR_INVALID_RETURN_PROPERTY_VALUE( - 'string', - 'loader resolve', + 'a string', + hookErrIdentifier, 'format', format, ); } - if (typeof url !== 'string') { // non-strings can be coerced to a url string + if (typeof url !== 'string') { + // non-strings can be coerced to a url string + // validateString() throws a less-specific error throw new ERR_INVALID_RETURN_PROPERTY_VALUE( - 'string', - 'loader resolve', + 'a url string', + hookErrIdentifier, 'url', url, ); } - new URL(url); // Intentionally trigger error if `url` is invalid + // Try to avoid expensive URL instantiation for known-good urls + if (!this.moduleMap.has(url)) { + try { + new URL(url); + } catch { + throw new ERR_INVALID_RETURN_PROPERTY_VALUE( + 'a url string', + hookErrIdentifier, + 'url', + url, + ); + } + } return { format, diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 924c4836bb2aa2..5a50d5d6afab6e 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -28,8 +28,12 @@ function resolveMainPath(main) { } function shouldUseESMLoader(mainPath) { - const userLoader = getOptionValue('--experimental-loader'); - if (userLoader) + /** + * @type {string[]} userLoaders A list of custom loaders registered by the user + * (or an empty list when none have been registered). + */ + const userLoaders = getOptionValue('--experimental-loader'); + if (userLoaders.length > 0) return true; const esModuleSpecifierResolution = getOptionValue('--experimental-specifier-resolution'); diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 20021134ce40f3..4634ff5a9f5101 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -49,11 +49,9 @@ exports.esmLoader = esmLoader; */ async function initializeLoader() { const { getOptionValue } = require('internal/options'); - // customLoaders CURRENTLY can be only 1 (a string) - // Once chaining is implemented, it will be string[] const customLoaders = getOptionValue('--experimental-loader'); - if (!customLoaders.length) return; + if (customLoaders.length === 0) return; let cwd; try { @@ -68,7 +66,7 @@ async function initializeLoader() { const internalEsmLoader = new ESMLoader(); // Importation must be handled by internal loader to avoid poluting userland - const exports = await internalEsmLoader.import( + const keyedExportsList = await internalEsmLoader.import( customLoaders, pathToFileURL(cwd).href, ObjectCreate(null), @@ -76,7 +74,7 @@ async function initializeLoader() { // Hooks must then be added to external/public loader // (so they're triggered in userland) - await esmLoader.addCustomLoaders(exports); + await esmLoader.addCustomLoaders(keyedExportsList); } exports.loadESM = async function loadESM(callback) { diff --git a/lib/internal/util/types.js b/lib/internal/util/types.js index aca7dbc4b2f2b6..6671e87b66dd08 100644 --- a/lib/internal/util/types.js +++ b/lib/internal/util/types.js @@ -68,7 +68,7 @@ module.exports = { isFloat32Array, isFloat64Array, isBigInt64Array, - isBigUint64Array + isBigUint64Array, }; let isCryptoKey; diff --git a/src/node_options.cc b/src/node_options.cc index f7551248f7d889..febadaafdecb32 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -346,7 +346,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { AddOption("--experimental-json-modules", "", NoOp{}, kAllowedInEnvironment); AddOption("--experimental-loader", "use the specified module as a custom loader", - &EnvironmentOptions::userland_loader, + &EnvironmentOptions::userland_loaders, kAllowedInEnvironment); AddAlias("--loader", "--experimental-loader"); AddOption("--experimental-modules", "", NoOp{}, kAllowedInEnvironment); diff --git a/src/node_options.h b/src/node_options.h index a5af1684d38d06..71c937aca08bef 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -161,7 +161,7 @@ class EnvironmentOptions : public Options { bool trace_warnings = false; bool extra_info_on_fatal_exception = true; std::string unhandled_rejections; - std::string userland_loader; + std::vector userland_loaders; bool verify_base_objects = #ifdef DEBUG true; diff --git a/test/es-module/test-esm-experimental-warnings.mjs b/test/es-module/test-esm-experimental-warnings.mjs index bf92b158e485eb..b6ef757a88302e 100644 --- a/test/es-module/test-esm-experimental-warnings.mjs +++ b/test/es-module/test-esm-experimental-warnings.mjs @@ -1,10 +1,33 @@ import { mustCall } from '../common/index.mjs'; import { fileURL } from '../common/fixtures.mjs'; -import { match, strictEqual } from 'assert'; +import { doesNotMatch, match, strictEqual } from 'assert'; import { spawn } from 'child_process'; import { execPath } from 'process'; -// Verify experimental warnings are printed +// Verify no warnings are printed when no experimental features are enabled or used +{ + const input = `import ${JSON.stringify(fileURL('es-module-loaders', 'module-named-exports.mjs'))}`; + const child = spawn(execPath, [ + '--input-type=module', + '--eval', + input, + ]); + + let stderr = ''; + child.stderr.setEncoding('utf8'); + child.stderr.on('data', (data) => { stderr += data; }); + child.on('close', mustCall((code, signal) => { + strictEqual(code, 0); + strictEqual(signal, null); + doesNotMatch( + stderr, + /ExperimentalWarning/, + new Error('No experimental warning(s) should be emitted when no experimental feature is enabled') + ); + })); +} + +// Verify experimental warning is printed when experimental feature is enabled for ( const [experiment, arg] of [ [/Custom ESM Loaders/, `--experimental-loader=${fileURL('es-module-loaders', 'hooks-custom.mjs')}`], diff --git a/test/es-module/test-esm-loader-chaining.mjs b/test/es-module/test-esm-loader-chaining.mjs new file mode 100644 index 00000000000000..1055c44a156a77 --- /dev/null +++ b/test/es-module/test-esm-loader-chaining.mjs @@ -0,0 +1,352 @@ +import '../common/index.mjs'; +import fixtures from '../common/fixtures.js'; +import assert from 'node:assert'; +import { spawnSync } from 'node:child_process'; + +const setupArgs = [ + '--no-warnings', + '--input-type=module', + '--eval', +]; +const commonInput = 'import fs from "node:fs"; console.log(fs)'; +const commonArgs = [ + ...setupArgs, + commonInput, +]; + +{ // Verify unadulterated source is loaded when there are no loaders + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + ...setupArgs, + 'import fs from "node:fs"; console.log(typeof fs?.constants?.F_OK )', + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(stderr, ''); + assert.strictEqual(status, 0); + assert.match(stdout, /number/); // node:fs is an object +} + +{ // Verify loaded source is properly different when only load changes something + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(stderr, ''); + assert.match(stdout, /load passthru/); + assert.match(stdout, /resolve passthru/); + assert.strictEqual(status, 0); + assert.match(stdout, /foo/); +} + +{ // Verify multiple changes from hooks result in proper output + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-foo.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-42.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(stderr, ''); + assert.match(stdout, /resolve 42/); // It did go thru resolve-42 + assert.strictEqual(status, 0); + assert.match(stdout, /foo/); // LIFO, so resolve-foo won +} + +{ // Verify modifying context within resolve chain is respected + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-42.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-receiving-modified-context.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-passing-modified-context.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(stderr, ''); + assert.match(stdout, /bar/); + assert.strictEqual(status, 0); +} + +{ // Verify multiple changes from hooks result in proper output + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-42.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-foo.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(stderr, ''); + assert.match(stdout, /resolve foo/); // It did go thru resolve-foo + assert.strictEqual(status, 0); + assert.match(stdout, /42/); // LIFO, so resolve-42 won +} + +{ // Verify error thrown for incomplete resolve chain, citing errant loader & hook + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-incomplete.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.match(stdout, /resolve passthru/); + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_LOADER_CHAIN_INCOMPLETE/); + assert.match(stderr, /loader-resolve-incomplete\.mjs/); + assert.match(stderr, /"resolve"/); +} + +{ // Verify error NOT thrown when nested resolve hook signaled a short circuit + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-next-modified.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(stderr, ''); + assert.strictEqual(stdout.trim(), 'foo'); + assert.strictEqual(status, 0); +} + +{ // Verify error NOT thrown when nested load hook signaled a short circuit + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-42.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-next-modified.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(stderr, ''); + assert.match(stdout, /421/); + assert.strictEqual(status, 0); +} + +{ // Verify chain does break and throws appropriately + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-incomplete.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.doesNotMatch(stdout, /resolve passthru/); + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_LOADER_CHAIN_INCOMPLETE/); + assert.match(stderr, /loader-resolve-incomplete\.mjs/); + assert.match(stderr, /"resolve"/); +} + +{ // Verify error thrown for incomplete load chain, citing errant loader & hook + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-incomplete.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.match(stdout, /load passthru/); + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_LOADER_CHAIN_INCOMPLETE/); + assert.match(stderr, /loader-load-incomplete\.mjs/); + assert.match(stderr, /"load"/); +} + +{ // Verify chain does break and throws appropriately + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-incomplete.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.doesNotMatch(stdout, /load passthru/); + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_LOADER_CHAIN_INCOMPLETE/); + assert.match(stderr, /loader-load-incomplete\.mjs/); + assert.match(stderr, /"load"/); +} + +{ // Verify error thrown when invalid `specifier` argument passed to `resolve…next` + const { status, stderr } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-bad-next-specifier.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_INVALID_ARG_TYPE/); + assert.match(stderr, /loader-resolve-bad-next-specifier\.mjs/); + assert.match(stderr, /"resolve"/); + assert.match(stderr, /nextResolve\(specifier\)/); +} + +{ // Verify error thrown when invalid `context` argument passed to `resolve…next` + const { status, stderr } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-bad-next-context.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_INVALID_ARG_TYPE/); + assert.match(stderr, /loader-resolve-bad-next-context\.mjs/); + assert.match(stderr, /"resolve"/); + assert.match(stderr, /nextResolve\(, context\)/); +} + +{ // Verify error thrown when invalid `url` argument passed to `load…next` + const { status, stderr } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-bad-next-url.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_INVALID_ARG_TYPE/); + assert.match(stderr, /loader-load-bad-next-url\.mjs/); + assert.match(stderr, /"load"/); + assert.match(stderr, /nextLoad\(url\)/); +} + +{ // Verify error thrown when invalid `url` argument passed to `load…next` + const { status, stderr } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-impersonating-next-url.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_INVALID_ARG_VALUE/); + assert.match(stderr, /loader-load-impersonating-next-url\.mjs/); + assert.match(stderr, /"load"/); + assert.match(stderr, /nextLoad\(url\)/); +} + +{ // Verify error thrown when invalid `context` argument passed to `load…next` + const { status, stderr } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-load-bad-next-context.mjs'), + ...commonArgs, + ], + { encoding: 'utf8' }, + ); + + assert.strictEqual(status, 1); + assert.match(stderr, /ERR_INVALID_ARG_TYPE/); + assert.match(stderr, /loader-load-bad-next-context\.mjs/); + assert.match(stderr, /"load"/); + assert.match(stderr, /nextLoad\(, context\)/); +} diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 57a203342ac49c..d314a4d9aa0a5e 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -35,6 +35,7 @@ const { ESMLoader } = esmLoaderModule; return { format: suggestedFormat, + shortCircuit: true, url: resolvedURL, }; } @@ -54,15 +55,21 @@ const { ESMLoader } = esmLoaderModule; // This doesn't matter (just to avoid errors) return { format: 'module', + shortCircuit: true, source: '', }; } - const customLoader = { - // Ensure ESMLoader actually calls the custom hooks - resolve: mustCall(resolve), - load: mustCall(load), - }; + const customLoader = [ + { + exports: { + // Ensure ESMLoader actually calls the custom hooks + resolve: mustCall(resolve), + load: mustCall(load), + }, + url: import.meta.url, + }, + ]; esmLoader.addCustomLoaders(customLoader); diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs index 6294e57404c8bb..1ba7c621f7e92a 100644 --- a/test/es-module/test-esm-loader-invalid-url.mjs +++ b/test/es-module/test-esm-loader-invalid-url.mjs @@ -4,7 +4,7 @@ import assert from 'assert'; import('../fixtures/es-modules/test-esm-ok.mjs') .then(assert.fail, (error) => { - expectsError({ code: 'ERR_INVALID_URL' })(error); - assert.strictEqual(error.input, '../fixtures/es-modules/test-esm-ok.mjs'); + expectsError({ code: 'ERR_INVALID_RETURN_PROPERTY_VALUE' })(error); + assert.match(error.message, /loader-invalid-url\.mjs/); }) .then(mustCall()); diff --git a/test/es-module/test-esm-tla-unfinished.mjs b/test/es-module/test-esm-tla-unfinished.mjs index d7658c19e98e1c..a7b6e620d0620a 100644 --- a/test/es-module/test-esm-tla-unfinished.mjs +++ b/test/es-module/test-esm-tla-unfinished.mjs @@ -3,11 +3,17 @@ import assert from 'assert'; import child_process from 'child_process'; import fixtures from '../common/fixtures.js'; +const commonArgs = [ + '--no-warnings', + '--input-type=module', + '--eval', +]; + { // Unresolved TLA promise, --eval const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - ['--input-type=module', '--eval', 'await new Promise(() => {})'], + [...commonArgs, 'await new Promise(() => {})'], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout, stderr], [13, '', '']); } @@ -16,7 +22,7 @@ import fixtures from '../common/fixtures.js'; // Rejected TLA promise, --eval const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - ['--input-type=module', '-e', 'await Promise.reject(new Error("Xyz"))'], + [...commonArgs, 'await Promise.reject(new Error("Xyz"))'], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout], [1, '']); assert.match(stderr, /Error: Xyz/); @@ -26,8 +32,10 @@ import fixtures from '../common/fixtures.js'; // Unresolved TLA promise with explicit exit code, --eval const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - ['--input-type=module', '--eval', - 'process.exitCode = 42;await new Promise(() => {})'], + [ + ...commonArgs, + 'process.exitCode = 42;await new Promise(() => {})', + ], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout, stderr], [42, '', '']); } @@ -36,8 +44,10 @@ import fixtures from '../common/fixtures.js'; // Rejected TLA promise with explicit exit code, --eval const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - ['--input-type=module', '-e', - 'process.exitCode = 42;await Promise.reject(new Error("Xyz"))'], + [ + ...commonArgs, + 'process.exitCode = 42;await Promise.reject(new Error("Xyz"))', + ], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout], [1, '']); assert.match(stderr, /Error: Xyz/); @@ -47,7 +57,7 @@ import fixtures from '../common/fixtures.js'; // Unresolved TLA promise, module file const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - [fixtures.path('es-modules/tla/unresolved.mjs')], + ['--no-warnings', fixtures.path('es-modules/tla/unresolved.mjs')], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout, stderr], [13, '', '']); } @@ -56,7 +66,7 @@ import fixtures from '../common/fixtures.js'; // Rejected TLA promise, module file const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - [fixtures.path('es-modules/tla/rejected.mjs')], + ['--no-warnings', fixtures.path('es-modules/tla/rejected.mjs')], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout], [1, '']); assert.match(stderr, /Error: Xyz/); @@ -66,7 +76,7 @@ import fixtures from '../common/fixtures.js'; // Unresolved TLA promise, module file const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - [fixtures.path('es-modules/tla/unresolved-withexitcode.mjs')], + ['--no-warnings', fixtures.path('es-modules/tla/unresolved-withexitcode.mjs')], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout, stderr], [42, '', '']); } @@ -75,7 +85,7 @@ import fixtures from '../common/fixtures.js'; // Rejected TLA promise, module file const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - [fixtures.path('es-modules/tla/rejected-withexitcode.mjs')], + ['--no-warnings', fixtures.path('es-modules/tla/rejected-withexitcode.mjs')], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout], [1, '']); assert.match(stderr, /Error: Xyz/); @@ -85,7 +95,7 @@ import fixtures from '../common/fixtures.js'; // Calling process.exit() in .mjs should return status 0 const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - [fixtures.path('es-modules/tla/process-exit.mjs')], + ['--no-warnings', fixtures.path('es-modules/tla/process-exit.mjs')], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout, stderr], [0, '', '']); } @@ -94,7 +104,7 @@ import fixtures from '../common/fixtures.js'; // Calling process.exit() in worker thread shouldn't influence main thread const { status, stdout, stderr } = child_process.spawnSync( process.execPath, - [fixtures.path('es-modules/tla/unresolved-with-worker-process-exit.mjs')], + ['--no-warnings', fixtures.path('es-modules/tla/unresolved-with-worker-process-exit.mjs')], { encoding: 'utf8' }); assert.deepStrictEqual([status, stdout, stderr], [13, '', '']); } diff --git a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs index 8790811c7e7bd6..e303ec196f6c6d 100644 --- a/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs +++ b/test/fixtures/es-module-loaders/builtin-named-exports-loader.mjs @@ -18,6 +18,7 @@ export async function resolve(specifier, context, next) { if (def.url.startsWith('node:')) { return { + shortCircuit: true, url: `custom-${def.url}`, importAssertions: context.importAssertions, }; @@ -29,6 +30,7 @@ export function load(url, context, next) { if (url.startsWith('custom-node:')) { const urlObj = new URL(url); return { + shortCircuit: true, source: generateBuiltinModule(urlObj.pathname), format: 'module', }; diff --git a/test/fixtures/es-module-loaders/example-loader.mjs b/test/fixtures/es-module-loaders/example-loader.mjs index be4808738035f9..f87054c8b70502 100644 --- a/test/fixtures/es-module-loaders/example-loader.mjs +++ b/test/fixtures/es-module-loaders/example-loader.mjs @@ -11,6 +11,7 @@ baseURL.pathname = process.cwd() + '/'; export function resolve(specifier, { parentURL = baseURL }, defaultResolve) { if (builtinModules.includes(specifier)) { return { + shortCircuit: true, url: 'node:' + specifier }; } @@ -22,6 +23,7 @@ export function resolve(specifier, { parentURL = baseURL }, defaultResolve) { } const resolved = new URL(specifier, parentURL); return { + shortCircuit: true, url: resolved.href }; } diff --git a/test/fixtures/es-module-loaders/hooks-custom.mjs b/test/fixtures/es-module-loaders/hooks-custom.mjs index 5173b97387905a..4c4014db01ef3f 100644 --- a/test/fixtures/es-module-loaders/hooks-custom.mjs +++ b/test/fixtures/es-module-loaders/hooks-custom.mjs @@ -29,25 +29,30 @@ export function load(url, context, next) { if (url.endsWith('esmHook/badReturnFormatVal.mjs')) return { format: Array(0), + shortCircuit: true, source: '', } if (url.endsWith('esmHook/unsupportedReturnFormatVal.mjs')) return { format: 'foo', // Not one of the allowable inputs: no translator named 'foo' + shortCircuit: true, source: '', } if (url.endsWith('esmHook/badReturnSourceVal.mjs')) return { format: 'module', + shortCircuit: true, source: Array(0), } if (url.endsWith('esmHook/preknownFormat.pre')) return { format: context.format, + shortCircuit: true, source: `const msg = 'hello world'; export default msg;` }; if (url.endsWith('esmHook/virtual.mjs')) return { format: 'module', + shortCircuit: true, source: `export const message = 'Woohoo!'.toUpperCase();`, }; @@ -63,6 +68,7 @@ export function resolve(specifier, context, next) { if (specifier.startsWith('esmHook')) return { format, + shortCircuit: true, url: pathToFileURL(specifier).href, importAssertions: context.importAssertions, }; diff --git a/test/fixtures/es-module-loaders/loader-invalid-format.mjs b/test/fixtures/es-module-loaders/loader-invalid-format.mjs index 0210f73b554382..438d50dacba433 100644 --- a/test/fixtures/es-module-loaders/loader-invalid-format.mjs +++ b/test/fixtures/es-module-loaders/loader-invalid-format.mjs @@ -1,7 +1,8 @@ export async function resolve(specifier, { parentURL, importAssertions }, defaultResolve) { if (parentURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { return { - url: 'file:///asdf' + shortCircuit: true, + url: 'file:///asdf', }; } return defaultResolve(specifier, {parentURL, importAssertions}, defaultResolve); @@ -11,6 +12,7 @@ export async function load(url, context, next) { if (url === 'file:///asdf') { return { format: 'esm', + shortCircuit: true, source: '', } } diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs index a7cefeca3da37a..87d1a6a564b461 100644 --- a/test/fixtures/es-module-loaders/loader-invalid-url.mjs +++ b/test/fixtures/es-module-loaders/loader-invalid-url.mjs @@ -1,6 +1,7 @@ export async function resolve(specifier, { parentURL, importAssertions }, defaultResolve) { if (parentURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs') { return { + shortCircuit: true, url: specifier, importAssertions, }; diff --git a/test/fixtures/es-module-loaders/loader-load-bad-next-context.mjs b/test/fixtures/es-module-loaders/loader-load-bad-next-context.mjs new file mode 100644 index 00000000000000..fe38da04f9ff91 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-bad-next-context.mjs @@ -0,0 +1,3 @@ +export async function load(url, context, next) { + return next(url, []); +} diff --git a/test/fixtures/es-module-loaders/loader-load-bad-next-url.mjs b/test/fixtures/es-module-loaders/loader-load-bad-next-url.mjs new file mode 100644 index 00000000000000..4f53b695327dd1 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-bad-next-url.mjs @@ -0,0 +1,3 @@ +export async function load(url, context, next) { + return next([], context); +} diff --git a/test/fixtures/es-module-loaders/loader-load-foo-or-42.mjs b/test/fixtures/es-module-loaders/loader-load-foo-or-42.mjs new file mode 100644 index 00000000000000..8d408223e66a0a --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-foo-or-42.mjs @@ -0,0 +1,11 @@ +export async function load(url) { + const val = url.includes('42') + ? '42' + : '"foo"'; + + return { + format: 'module', + shortCircuit: true, + source: `export default ${val}`, + }; +} diff --git a/test/fixtures/es-module-loaders/loader-load-impersonating-next-url.mjs b/test/fixtures/es-module-loaders/loader-load-impersonating-next-url.mjs new file mode 100644 index 00000000000000..f98b091c8b9ff5 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-impersonating-next-url.mjs @@ -0,0 +1,3 @@ +export async function load(url, context, next) { + return next('not/a/url', context); +} diff --git a/test/fixtures/es-module-loaders/loader-load-incomplete.mjs b/test/fixtures/es-module-loaders/loader-load-incomplete.mjs new file mode 100644 index 00000000000000..d6242488e5738e --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-incomplete.mjs @@ -0,0 +1,6 @@ +export async function load() { + return { + format: 'module', + source: 'export default 42', + }; +} diff --git a/test/fixtures/es-module-loaders/loader-load-next-modified.mjs b/test/fixtures/es-module-loaders/loader-load-next-modified.mjs new file mode 100644 index 00000000000000..1f2382467f7122 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-next-modified.mjs @@ -0,0 +1,11 @@ +export async function load(url, context, next) { + const { + format, + source, + } = await next(url, context); + + return { + format, + source: source + 1, + }; +} diff --git a/test/fixtures/es-module-loaders/loader-load-passing-modified-context.mjs b/test/fixtures/es-module-loaders/loader-load-passing-modified-context.mjs new file mode 100644 index 00000000000000..7676be766af575 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-passing-modified-context.mjs @@ -0,0 +1,6 @@ +export async function load(url, context, next) { + return next(url, { + ...context, + foo: 'bar', + }); +} diff --git a/test/fixtures/es-module-loaders/loader-load-passthru.mjs b/test/fixtures/es-module-loaders/loader-load-passthru.mjs new file mode 100644 index 00000000000000..8cfbcb6a3a5d0b --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-passthru.mjs @@ -0,0 +1,4 @@ +export async function load(url, context, next) { + console.log('load passthru'); // This log is deliberate + return next(url, context); +} diff --git a/test/fixtures/es-module-loaders/loader-load-receiving-modified-context.mjs b/test/fixtures/es-module-loaders/loader-load-receiving-modified-context.mjs new file mode 100644 index 00000000000000..2d7bc350bd8775 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-load-receiving-modified-context.mjs @@ -0,0 +1,4 @@ +export async function load(url, context, next) { + console.log(context.foo); // This log is deliberate + return next(url, context); +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-42.mjs b/test/fixtures/es-module-loaders/loader-resolve-42.mjs new file mode 100644 index 00000000000000..f4dffd70fc94ad --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-42.mjs @@ -0,0 +1,4 @@ +export async function resolve(specifier, context, next) { + console.log('resolve 42'); // This log is deliberate + return next('file:///42.mjs', context); +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-bad-next-context.mjs b/test/fixtures/es-module-loaders/loader-resolve-bad-next-context.mjs new file mode 100644 index 00000000000000..881f5875dd0206 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-bad-next-context.mjs @@ -0,0 +1,3 @@ +export async function resolve(specifier, context, next) { + return next(specifier, []); +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-bad-next-specifier.mjs b/test/fixtures/es-module-loaders/loader-resolve-bad-next-specifier.mjs new file mode 100644 index 00000000000000..a23785d3d956db --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-bad-next-specifier.mjs @@ -0,0 +1,3 @@ +export async function resolve(specifier, context, next) { + return next([], context); +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-foo.mjs b/test/fixtures/es-module-loaders/loader-resolve-foo.mjs new file mode 100644 index 00000000000000..595385e12a0cf7 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-foo.mjs @@ -0,0 +1,4 @@ +export async function resolve(specifier, context, next) { + console.log('resolve foo'); // This log is deliberate + return next('file:///foo.mjs', context); +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-incomplete.mjs b/test/fixtures/es-module-loaders/loader-resolve-incomplete.mjs new file mode 100644 index 00000000000000..9eb1617f30130e --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-incomplete.mjs @@ -0,0 +1,5 @@ +export async function resolve() { + return { + url: 'file:///incomplete-resolve-chain.js', + }; +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-next-modified.mjs b/test/fixtures/es-module-loaders/loader-resolve-next-modified.mjs new file mode 100644 index 00000000000000..a973345a82ff21 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-next-modified.mjs @@ -0,0 +1,11 @@ +export async function resolve(url, context, next) { + const { + format, + url: nextUrl, + } = await next(url, context); + + return { + format, + url: `${nextUrl}?foo`, + }; +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-passing-modified-context.mjs b/test/fixtures/es-module-loaders/loader-resolve-passing-modified-context.mjs new file mode 100644 index 00000000000000..6a92a6cd8f6a8e --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-passing-modified-context.mjs @@ -0,0 +1,6 @@ +export async function resolve(specifier, context, next) { + return next(specifier, { + ...context, + foo: 'bar', + }); +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-passthru.mjs b/test/fixtures/es-module-loaders/loader-resolve-passthru.mjs new file mode 100644 index 00000000000000..1a373bab90ba57 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-passthru.mjs @@ -0,0 +1,4 @@ +export async function resolve(specifier, context, next) { + console.log('resolve passthru'); // This log is deliberate + return next(specifier, context); +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-receiving-modified-context.mjs b/test/fixtures/es-module-loaders/loader-resolve-receiving-modified-context.mjs new file mode 100644 index 00000000000000..83aa83104e96e4 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-receiving-modified-context.mjs @@ -0,0 +1,4 @@ +export async function resolve(specifier, context, next) { + console.log(context.foo); // This log is deliberate + return next(specifier, context); +} diff --git a/test/fixtures/es-module-loaders/loader-resolve-shortcircuit.mjs b/test/fixtures/es-module-loaders/loader-resolve-shortcircuit.mjs new file mode 100644 index 00000000000000..d886b3dfcbf237 --- /dev/null +++ b/test/fixtures/es-module-loaders/loader-resolve-shortcircuit.mjs @@ -0,0 +1,6 @@ +export async function resolve(specifier) { + return { + shortCircuit: true, + url: specifier, + } +} diff --git a/test/fixtures/es-module-loaders/mock-loader.mjs b/test/fixtures/es-module-loaders/mock-loader.mjs index 7c4592aca96834..062be39603e851 100644 --- a/test/fixtures/es-module-loaders/mock-loader.mjs +++ b/test/fixtures/es-module-loaders/mock-loader.mjs @@ -171,6 +171,7 @@ export function globalPreload({port}) { export async function resolve(specifier, context, defaultResolve) { if (specifier === 'node:mock') { return { + shortCircuit: true, url: specifier }; } @@ -180,10 +181,12 @@ export async function resolve(specifier, context, defaultResolve) { // Do nothing, let it get the "real" module } else if (mockedModuleExports.has(def.url)) { return { + shortCircuit: true, url: `mock-facade:${currentMockVersion}:${encodeURIComponent(def.url)}` }; }; return { + shortCircuit: true, url: def.url, }; } @@ -196,6 +199,7 @@ export async function load(url, context, defaultLoad) { * channel with preloadCode */ return { + shortCircuit: true, source: 'export default import.meta.doMock', format: 'module' }; @@ -210,6 +214,7 @@ export async function load(url, context, defaultLoad) { decodeURIComponent(encodedTargetURL) )); return { + shortCircuit: true, source: ret, format: 'module' }; diff --git a/test/fixtures/es-module-loaders/string-sources.mjs b/test/fixtures/es-module-loaders/string-sources.mjs index 384098d6d9e822..1fc2b7a8d6f7e3 100644 --- a/test/fixtures/es-module-loaders/string-sources.mjs +++ b/test/fixtures/es-module-loaders/string-sources.mjs @@ -22,7 +22,11 @@ const SOURCES = { } export function resolve(specifier, context, next) { if (specifier.startsWith('test:')) { - return { url: specifier, importAssertions: context.importAssertions }; + return { + importAssertions: context.importAssertions, + shortCircuit: true, + url: specifier, + }; } return next(specifier, context); } @@ -31,6 +35,7 @@ export function load(href, context, next) { if (href.startsWith('test:')) { return { format: 'module', + shortCircuit: true, source: SOURCES[href], }; } diff --git a/test/parallel/test-module-main-fail.js b/test/parallel/test-module-main-fail.js index c66b6f2f7a843f..2b6f188dd4cbee 100644 --- a/test/parallel/test-module-main-fail.js +++ b/test/parallel/test-module-main-fail.js @@ -10,7 +10,10 @@ for (const entryPoint of entryPoints) { try { execFileSync(node, [entryPoint], { stdio: 'pipe' }); } catch (e) { - assert(e.toString().match(/Error: Cannot find module/)); + const error = e.toString(); + assert.match(error, /MODULE_NOT_FOUND/); + assert.match(error, /Cannot find module/); + assert(error.includes(entryPoint)); continue; } assert.fail('Executing node with inexistent entry point should ' +