From 800591537a438a1a3813221ca2011eb1e1183055 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sat, 30 Apr 2022 14:19:48 +0200 Subject: [PATCH 01/24] esm: add chaining to loaders --- doc/api/errors.md | 11 + doc/api/esm.md | 214 ++++++----- lib/internal/errors.js | 7 + lib/internal/modules/esm/loader.js | 345 +++++++++++++----- 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 | 305 ++++++++++++++++ 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-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 + ...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 +- .../test-module-parent-deprecation.js | 2 +- 39 files changed, 881 insertions(+), 210 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-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-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 b174db7b6151b6..bb92e76624c35f 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..4ed878d30a25ca 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. @@ -908,6 +918,13 @@ 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. @@ -1035,7 +1052,7 @@ export function load(url, context, nextLoad) { } ``` -```mjs +```js // main.mjs import { VERSION } from 'https://coffeescript.org/browser-compiler-modern/coffeescript.js'; From 6bb5359d70de192bd82c42a43947f59939d8ff05 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 3 May 2022 19:02:32 +0200 Subject: [PATCH 15/24] =?UTF-8?q?fixup:=20update=20internals=20for=20`next?= =?UTF-8?q?()`=20=E2=86=92=20`next()`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/internal/modules/esm/loader.js | 32 ++++++++++----------- test/es-module/test-esm-loader-chaining.mjs | 10 +++---- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 1bb75356734abc..cfd4b3410f965c 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -498,8 +498,8 @@ class ESMLoader { * Provide source that is understood by one of Node's translators. * * Internally, this behaves like a backwards iterator, wherein the stack of - * hooks starts at the top and each call to `next()` moves down 1 step until - * it reaches the bottom or short-circuits. + * 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 @@ -515,7 +515,7 @@ class ESMLoader { let chainFinished = hookIndex === 0; let shortCircuited = false; - const next = async (nextUrl, ctx = context) => { + const nextLoad = async (nextUrl, ctx = context) => { --hookIndex; // `next` has been called, so decrement our pointer ({ @@ -531,7 +531,7 @@ class ESMLoader { // non-strings can be coerced to a url string // validateString() throws a less-specific error throw new ERR_INVALID_ARG_TYPE( - `${hookErrIdentifier} next(url)`, + `${hookErrIdentifier} nextLoad(url)`, 'a url string', nextUrl, ); @@ -543,16 +543,16 @@ class ESMLoader { new URL(nextUrl); } catch { throw new ERR_INVALID_ARG_VALUE( - `${hookErrIdentifier} next(url)`, + `${hookErrIdentifier} nextLoad(url)`, nextUrl, 'should be a url string', ); } } - validateObject(ctx, `${hookErrIdentifier} next(, context)`); + validateObject(ctx, `${hookErrIdentifier} nextLoad(, context)`); - const output = await loader(nextUrl, ctx, next); + const output = await loader(nextUrl, ctx, nextLoad); if (output?.shortCircuit === true) { shortCircuited = true; } @@ -562,7 +562,7 @@ class ESMLoader { const loaded = await loader( url, context, - next, + nextLoad, ); const hookErrIdentifier = `${loaderFilePath} load`; @@ -719,8 +719,8 @@ class ESMLoader { * Resolve the location of the module. * * Internally, this behaves like a backwards iterator, wherein the stack of - * hooks starts at the top and each call to `next()` moves down 1 step until - * it reaches the bottom or short-circuits. + * 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. @@ -763,8 +763,8 @@ class ESMLoader { parentURL, }; - const next = async (suppliedSpecifier, ctx = context) => { - --hookIndex; // `next` has been called, so decrement our pointer + const nextResolve = async (suppliedSpecifier, ctx = context) => { + --hookIndex; // `nextResolve` has been called, so decrement our pointer ({ fn: resolver, @@ -777,12 +777,12 @@ class ESMLoader { validateString( suppliedSpecifier, - `${hookErrIdentifier} next(specifier)`, + `${hookErrIdentifier} nextResolve(specifier)`, ); // non-strings can be coerced to a url string - validateObject(ctx, `${hookErrIdentifier} next(, context)`); + validateObject(ctx, `${hookErrIdentifier} nextResolve(, context)`); - const output = await resolver(suppliedSpecifier, ctx, next); + const output = await resolver(suppliedSpecifier, ctx, nextResolve); if (output?.shortCircuit === true) { shortCircuited = true; } @@ -792,7 +792,7 @@ class ESMLoader { const resolution = await resolver( originalSpecifier, context, - next, + nextResolve, ); const hookErrIdentifier = `${resolverFilePath} resolve`; diff --git a/test/es-module/test-esm-loader-chaining.mjs b/test/es-module/test-esm-loader-chaining.mjs index 8f20d202033c04..42369c732d068e 100644 --- a/test/es-module/test-esm-loader-chaining.mjs +++ b/test/es-module/test-esm-loader-chaining.mjs @@ -268,7 +268,7 @@ const commonArgs = [ assert.match(stderr, /ERR_INVALID_ARG_TYPE/); assert.match(stderr, /loader-resolve-bad-next-specifier\.mjs/); assert.match(stderr, /"resolve"/); - assert.match(stderr, /next\(specifier\)/); + assert.match(stderr, /nextResolve\(specifier\)/); } { // Verify error thrown when invalid `context` argument passed to `resolve…next` @@ -288,7 +288,7 @@ const commonArgs = [ assert.match(stderr, /ERR_INVALID_ARG_TYPE/); assert.match(stderr, /loader-resolve-bad-next-context\.mjs/); assert.match(stderr, /"resolve"/); - assert.match(stderr, /next\(, context\)/); + assert.match(stderr, /nextResolve\(, context\)/); } { // Verify error thrown when invalid `url` argument passed to `load…next` @@ -308,7 +308,7 @@ const commonArgs = [ assert.match(stderr, /ERR_INVALID_ARG_TYPE/); assert.match(stderr, /loader-load-bad-next-url\.mjs/); assert.match(stderr, /"load"/); - assert.match(stderr, /next\(url\)/); + assert.match(stderr, /nextLoad\(url\)/); } { // Verify error thrown when invalid `url` argument passed to `load…next` @@ -328,7 +328,7 @@ const commonArgs = [ assert.match(stderr, /ERR_INVALID_ARG_VALUE/); assert.match(stderr, /loader-load-impersonating-next-url\.mjs/); assert.match(stderr, /"load"/); - assert.match(stderr, /next\(url\)/); + assert.match(stderr, /nextLoad\(url\)/); } { // Verify error thrown when invalid `context` argument passed to `load…next` @@ -348,5 +348,5 @@ const commonArgs = [ assert.match(stderr, /ERR_INVALID_ARG_TYPE/); assert.match(stderr, /loader-load-bad-next-context\.mjs/); assert.match(stderr, /"load"/); - assert.match(stderr, /next\(, context\)/); + assert.match(stderr, /nextLoad\(, context\)/); } From 9fc7ba5326ab68e3e6afea3f00f831e2a62ee603 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 3 May 2022 19:03:20 +0200 Subject: [PATCH 16/24] =?UTF-8?q?fixup:=20`fixtures.path()`=20=E2=86=92=20?= =?UTF-8?q?`fixtures.fileURL()`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/es-module/test-esm-loader-chaining.mjs | 90 ++++++++++----------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/test/es-module/test-esm-loader-chaining.mjs b/test/es-module/test-esm-loader-chaining.mjs index 42369c732d068e..1055c44a156a77 100644 --- a/test/es-module/test-esm-loader-chaining.mjs +++ b/test/es-module/test-esm-loader-chaining.mjs @@ -34,11 +34,11 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-foo-or-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -56,13 +56,13 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-shortcircuit.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-foo.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-foo.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-42.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-foo-or-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -79,15 +79,15 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-shortcircuit.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-42.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-foo-or-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-receiving-modified-context.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-receiving-modified-context.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-passing-modified-context.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-passing-modified-context.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -103,13 +103,13 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-shortcircuit.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-42.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-foo.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-foo.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-foo-or-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -126,11 +126,11 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-incomplete.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-incomplete.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-foo-or-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -148,11 +148,11 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-shortcircuit.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-next-modified.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-next-modified.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-foo-or-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -168,13 +168,13 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-shortcircuit.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-shortcircuit.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-42.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-foo-or-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-next-modified.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-next-modified.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -190,11 +190,11 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-incomplete.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-incomplete.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-foo-or-42.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-foo-or-42.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -212,11 +212,11 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-incomplete.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-incomplete.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -234,11 +234,11 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-incomplete.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-incomplete.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -256,9 +256,9 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-bad-next-specifier.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-bad-next-specifier.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -276,9 +276,9 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-resolve-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-resolve-bad-next-context.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-resolve-bad-next-context.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -296,9 +296,9 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-load-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-bad-next-url.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-bad-next-url.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -316,9 +316,9 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-load-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-impersonating-next-url.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-impersonating-next-url.mjs'), ...commonArgs, ], { encoding: 'utf8' }, @@ -336,9 +336,9 @@ const commonArgs = [ process.execPath, [ '--loader', - fixtures.path('es-module-loaders/loader-load-passthru.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'), '--loader', - fixtures.path('es-module-loaders/loader-load-bad-next-context.mjs'), + fixtures.fileURL('es-module-loaders', 'loader-load-bad-next-context.mjs'), ...commonArgs, ], { encoding: 'utf8' }, From 1c712e99817a0697478818bd376b398acc9e5d73 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 3 May 2022 19:04:12 +0200 Subject: [PATCH 17/24] =?UTF-8?q?fixup:=20code=20doc=20for=20ESMLoader::im?= =?UTF-8?q?port()=20=E2=80=A6=20namespaces?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/internal/modules/esm/loader.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index cfd4b3410f965c..26f29a043203d0 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -464,9 +464,18 @@ class ESMLoader { * @param {Record} importAssertions Validations for the * module import. * @returns {Promise} - * A collection of module export(s). + * 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]; } @@ -481,7 +490,7 @@ class ESMLoader { const namespaces = await PromiseAll(new SafeArrayIterator(jobs)); - if (!wasArr) { return namespaces[0]; } + if (!wasArr) { return namespaces[0]; } // We can skip the pairing below for (let i = 0; i < count; i++) { const namespace = ObjectCreate(null); From 21bd600f158b4d98219741793d037509f46d9988 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 3 May 2022 19:10:18 +0200 Subject: [PATCH 18/24] fixup: update manual anchor links --- doc/api/esm.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 59c1de4ca729e8..2f94dcc3f3501b 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1581,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-next +[load hook]: #loadurl-context-nextload [percent-encoded]: url.md#percent-encoding-in-urls -[resolve hook]: #resolvespecifier-context-next +[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 From c81ad323c1e56861163aaf032a8d0fffb320af2a Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 3 May 2022 19:35:45 +0200 Subject: [PATCH 19/24] fixup: tweak ESM doc `resolve` & `load` hooks' changelogs --- doc/api/esm.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 2f94dcc3f3501b..7564877eaa530a 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -732,9 +732,9 @@ the chain. changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/42623 - description: Add support for chaining resolve hooks. This introduces a - breaking change, wherein a hook that does not call `nextResolve()` MUST - include a `shortCircuit` flag in its return. + description: Add support for chaining resolve hooks. Each hook that does + not call `nextResolve()` MUST include a `shortCircuit` property set to + `true` in its return. - version: - v17.1.0 - v16.14.0 @@ -824,9 +824,9 @@ export async function resolve(specifier, context, nextResolve) { changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/42623 - description: Add support for chaining load hooks. This introduces a - breaking change, wherein a hook that does not call `nextLoad()` MUST - include a `shortCircuit` flag in its return. + description: Add support for chaining load hooks. Each hook that does + not call `nextLoad()` MUST include a `shortCircuit` property set to `true` + in its return. --> > The loaders API is being redesigned. This hook may disappear or its From 96acd20f65bb92edda0c73c29b375a8d4568cd08 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 3 May 2022 19:44:09 +0200 Subject: [PATCH 20/24] fixup: wordsmith ESM hooks' changelogs --- doc/api/esm.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 7564877eaa530a..1e519f599eca7b 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -732,9 +732,9 @@ the chain. changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/42623 - description: Add support for chaining resolve hooks. Each hook that does - not call `nextResolve()` MUST include a `shortCircuit` property set to - `true` in its return. + description: Add support for chaining resolve hooks. Each hook must either + call `nextResolve()` or include a `shortCircuit` property set to `true` + in its return. - version: - v17.1.0 - v16.14.0 @@ -824,9 +824,9 @@ export async function resolve(specifier, context, nextResolve) { changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/42623 - description: Add support for chaining load hooks. Each hook that does - not call `nextLoad()` MUST include a `shortCircuit` property set to `true` - in its return. + description: Add support for chaining load hooks. Each hook must either + call `nextLoad()` or include a `shortCircuit` property set to `true` in + its return. --> > The loaders API is being redesigned. This hook may disappear or its From 4169efb3f660a76b150c5b60231096d8f2fc24dd Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 3 May 2022 13:48:58 -0400 Subject: [PATCH 21/24] fixup: remove extra whitespace character Co-authored-by: Antoine du Hamel --- doc/api/esm.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 1e519f599eca7b..a82ed54b6e19db 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -824,7 +824,7 @@ export async function resolve(specifier, context, nextResolve) { changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/42623 - description: Add support for chaining load hooks. Each hook must either + description: Add support for chaining load hooks. Each hook must either call `nextLoad()` or include a `shortCircuit` property set to `true` in its return. --> From 15eecb6284c970b1425e97319fedc2789758c586 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 3 May 2022 20:21:43 +0200 Subject: [PATCH 22/24] fixup: wordsmith ESM warning for triggering chaining exceptions --- doc/api/esm.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index a82ed54b6e19db..fd128397f4981f 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -721,10 +721,10 @@ functions nest: each one must always return a plain object, and chaining happens as a result of each function calling `next()`, which is a reference to the subsequent loader’s hook. -A hook that fails to return triggers an exception. A hook that returns without -calling `next()` and without returning `shortCircuit: true` also -triggers an exception. These errors are to help prevent unintentional breaks in -the chain. +A hook that returns a value lacking a required property triggers an exception. +A hook that returns without calling `next()` _and_ without returning +`shortCircuit: true` also triggers an exception. These errors are to help +prevent unintentional breaks in the chain. #### `resolve(specifier, context, nextResolve)` From 6dd23ee023b301f98f34d18c4eaac6d291d9ff45 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Wed, 4 May 2022 09:56:35 +0200 Subject: [PATCH 23/24] fixup: optimise resolve's validity check on `url` --- lib/internal/modules/esm/loader.js | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 26f29a043203d0..8bc22ef697db3e 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -525,7 +525,7 @@ class ESMLoader { let shortCircuited = false; const nextLoad = async (nextUrl, ctx = context) => { - --hookIndex; // `next` has been called, so decrement our pointer + --hookIndex; // `nextLoad` has been called, so decrement our pointer. ({ fn: loader, @@ -773,7 +773,7 @@ class ESMLoader { }; const nextResolve = async (suppliedSpecifier, ctx = context) => { - --hookIndex; // `nextResolve` has been called, so decrement our pointer + --hookIndex; // `nextResolve` has been called, so decrement our pointer. ({ fn: resolver, @@ -848,15 +848,18 @@ class ESMLoader { ); } - try { - new URL(url); - } catch { - throw new ERR_INVALID_RETURN_PROPERTY_VALUE( - 'a url string', - hookErrIdentifier, - 'url', - url, - ); + // 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 { From 38e596b614a1dbc57ec4e8234f9262bf0a956c67 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Wed, 4 May 2022 09:58:19 +0200 Subject: [PATCH 24/24] =?UTF-8?q?fixup:=20test=20fixture=20`node:`=20?= =?UTF-8?q?=E2=86=92=20`import.meta.url`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/es-module/test-esm-loader-hooks.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/es-module/test-esm-loader-hooks.mjs b/test/es-module/test-esm-loader-hooks.mjs index 7e6ea6793e227b..d314a4d9aa0a5e 100644 --- a/test/es-module/test-esm-loader-hooks.mjs +++ b/test/es-module/test-esm-loader-hooks.mjs @@ -67,7 +67,7 @@ const { ESMLoader } = esmLoaderModule; resolve: mustCall(resolve), load: mustCall(load), }, - url: 'node:test/es-module/test-esm-loader-hooks.mjs', + url: import.meta.url, }, ];