From 3a0882ad2bb236321bcdbbfc7618edf075903675 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 9 Mar 2025 18:11:46 +0100 Subject: [PATCH 01/10] esm: syncify default path of `ModuleLoader.load` --- lib/internal/modules/esm/load.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index 9661bc716bfa76..13b286dc925772 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -29,13 +29,12 @@ const { * @param {ESModuleContext} context used to decorate error messages * @returns {Promise<{ responseURL: string, source: string | BufferView }>} */ -async function getSource(url, context) { +function getSource(url, context) { const { protocol, href } = url; const responseURL = href; let source; if (protocol === 'file:') { - const { readFile: readFileAsync } = require('internal/fs/promises').exports; - source = await readFileAsync(url); + source = readFileSync(url); } else if (protocol === 'data:') { const result = dataURLProcessor(url); if (result === 'failure') { @@ -80,7 +79,7 @@ function getSourceSync(url, context) { * @param {LoadContext} context * @returns {LoadReturn} */ -async function defaultLoad(url, context = kEmptyObject) { +function defaultLoad(url, context = kEmptyObject) { let responseURL = url; let { importAttributes, @@ -110,13 +109,13 @@ async function defaultLoad(url, context = kEmptyObject) { source = null; } else if (format !== 'commonjs') { if (source == null) { - ({ responseURL, source } = await getSource(urlInstance, context)); + ({ responseURL, source } = getSource(urlInstance, context)); context = { __proto__: context, source }; } if (format == null) { // Now that we have the source for the module, run `defaultGetFormat` to detect its format. - format = await defaultGetFormat(urlInstance, context); + format = defaultGetFormat(urlInstance, context); if (format === 'commonjs') { // For backward compatibility reasons, we need to discard the source in From d633804cd8f6f85995de01b4270d9960a24db46e Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 9 Mar 2025 18:23:40 +0100 Subject: [PATCH 02/10] !fixup: don't force `ModuleLoader.load` to be async (can via hooks) --- lib/internal/modules/esm/loader.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 493e9881a466c3..1f6b47857d0d93 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -804,9 +804,9 @@ class ModuleLoader { * if any. * @param {string} url The URL of the module to be loaded. * @param {object} context Metadata about the module - * @returns {Promise<{ format: ModuleFormat, source: ModuleSource }>} + * @returns {Promise<{ format: ModuleFormat, source: ModuleSource }> | { format: ModuleFormat, source: ModuleSource }} */ - async load(url, context) { + load(url, context) { if (loadHooks.length) { // Has module.registerHooks() hooks, use the synchronous variant that can handle both hooks. return this.#loadSync(url, context); From 4276af12d5a0c31b12e6588f4482d2c428a523d1 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Wed, 12 Mar 2025 01:47:59 +0100 Subject: [PATCH 03/10] !fixup: remove obsolete `getSource` in favour of `getSourceSync` --- lib/internal/modules/esm/load.js | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js index 13b286dc925772..98e14455075d2f 100644 --- a/lib/internal/modules/esm/load.js +++ b/lib/internal/modules/esm/load.js @@ -26,31 +26,7 @@ const { /** * @param {URL} url URL to the module - * @param {ESModuleContext} context used to decorate error messages - * @returns {Promise<{ responseURL: string, source: string | BufferView }>} - */ -function getSource(url, context) { - const { protocol, href } = url; - const responseURL = href; - let source; - if (protocol === 'file:') { - source = readFileSync(url); - } else if (protocol === 'data:') { - const result = dataURLProcessor(url); - if (result === 'failure') { - throw new ERR_INVALID_URL(responseURL, null); - } - source = BufferFrom(result.body); - } else { - const supportedSchemes = ['file', 'data']; - throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(url, supportedSchemes); - } - return { __proto__: null, responseURL, source }; -} - -/** - * @param {URL} url URL to the module - * @param {ESModuleContext} context used to decorate error messages + * @param {LoadContext} context used to decorate error messages * @returns {{ responseURL: string, source: string | BufferView }} */ function getSourceSync(url, context) { @@ -109,7 +85,7 @@ function defaultLoad(url, context = kEmptyObject) { source = null; } else if (format !== 'commonjs') { if (source == null) { - ({ responseURL, source } = getSource(urlInstance, context)); + ({ responseURL, source } = getSourceSync(urlInstance, context)); context = { __proto__: context, source }; } From f60650ff428e9725c5d732cf0e1e8bf2ec937e79 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Mon, 17 Mar 2025 18:50:22 +0100 Subject: [PATCH 04/10] !fixup: account for `node:internal` in `isMain` check Co-Authored-By: Marco Ippolito <36735501+marco-ippolito@users.noreply.github.com> --- lib/internal/modules/esm/resolve.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index bfd9bd3d127404..39e58accad4c61 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -965,7 +965,7 @@ function defaultResolve(specifier, context = {}) { if (protocol === 'node:') { return { __proto__: null, url: specifier }; } - const isMain = parentURL === undefined; + const isMain = parentURL === undefined || StringPrototypeStartsWith(parentURL, 'node:internal/'); if (isMain) { parentURL = getCWDURL().href; From 76c93d567df59cc5393ce007254d9a9499895e08 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sat, 5 Apr 2025 10:27:31 +0200 Subject: [PATCH 05/10] Revert "!fixup: account for `node:internal` in `isMain` check" This reverts commit f60650ff428e9725c5d732cf0e1e8bf2ec937e79. --- lib/internal/modules/esm/resolve.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 39e58accad4c61..bfd9bd3d127404 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -965,7 +965,7 @@ function defaultResolve(specifier, context = {}) { if (protocol === 'node:') { return { __proto__: null, url: specifier }; } - const isMain = parentURL === undefined || StringPrototypeStartsWith(parentURL, 'node:internal/'); + const isMain = parentURL === undefined; if (isMain) { parentURL = getCWDURL().href; From b697946faf9bdb0f4b43d16ebed015eccd5215ef Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 8 Apr 2025 12:25:39 +0200 Subject: [PATCH 06/10] Reapply "!fixup: account for `node:internal` in `isMain` check" This reverts commit 76c93d567df59cc5393ce007254d9a9499895e08. --- lib/internal/modules/esm/resolve.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index bfd9bd3d127404..39e58accad4c61 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -965,7 +965,7 @@ function defaultResolve(specifier, context = {}) { if (protocol === 'node:') { return { __proto__: null, url: specifier }; } - const isMain = parentURL === undefined; + const isMain = parentURL === undefined || StringPrototypeStartsWith(parentURL, 'node:internal/'); if (isMain) { parentURL = getCWDURL().href; From 8f93801a9ad77c5f8fb9ac4bd180b44a8fc138bc Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 8 Apr 2025 12:33:53 +0200 Subject: [PATCH 07/10] fixup!: add todo & note about mysterious fix/failure --- lib/internal/modules/esm/resolve.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 39e58accad4c61..a3fa908d8cf14b 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -965,7 +965,12 @@ function defaultResolve(specifier, context = {}) { if (protocol === 'node:') { return { __proto__: null, url: specifier }; } - const isMain = parentURL === undefined || StringPrototypeStartsWith(parentURL, 'node:internal/'); + const isMain = ( + parentURL === undefined || + // TODO: Discover why this is needed for vm to error with the correct code when esm/defaultLoad + // is sync. @see https://github.com/nodejs/node/pull/57419#discussion_r2027158688 + StringPrototypeStartsWith(parentURL, 'node:internal/') + ); if (isMain) { parentURL = getCWDURL().href; From 558dfffee6f2dcb44d3b3ea1a7d453af1e5c8b6a Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Wed, 9 Apr 2025 20:41:34 +0200 Subject: [PATCH 08/10] fixup!: move isInternal check to specific vm resolve path --- lib/internal/modules/esm/resolve.js | 7 +------ lib/internal/modules/esm/utils.js | 3 +++ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index a3fa908d8cf14b..bfd9bd3d127404 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -965,12 +965,7 @@ function defaultResolve(specifier, context = {}) { if (protocol === 'node:') { return { __proto__: null, url: specifier }; } - const isMain = ( - parentURL === undefined || - // TODO: Discover why this is needed for vm to error with the correct code when esm/defaultLoad - // is sync. @see https://github.com/nodejs/node/pull/57419#discussion_r2027158688 - StringPrototypeStartsWith(parentURL, 'node:internal/') - ); + const isMain = parentURL === undefined; if (isMain) { parentURL = getCWDURL().href; diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index 3c925cb0eea901..365f173967edb4 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -253,6 +253,9 @@ async function importModuleDynamicallyCallback(referrerSymbol, specifier, phase, // and fall back to the default loader. if (referrerSymbol === vm_dynamic_import_main_context_default) { emitExperimentalWarning('vm.USE_MAIN_CONTEXT_DEFAULT_LOADER'); + // TODO: Discover why this is needed for vm to error with the correct code when esm/defaultLoad + // is sync. @see https://github.com/nodejs/node/pull/57419#discussion_r2027158688 + if (referrerName === 'node:internal/process/task_queues') { referrerName = undefined; } return defaultImportModuleDynamicallyForScript(specifier, phase, attributes, referrerName); } // For script compiled internally that should use the default loader to handle dynamic From d765ed30715ee98977c6ee8c9dfcdbbcbcb9b5a0 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 22 Apr 2025 22:10:18 +0200 Subject: [PATCH 09/10] fixup!: remove `referrerName === 'node:internal/process/task_queues'` --- lib/internal/modules/esm/utils.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index 365f173967edb4..3c925cb0eea901 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -253,9 +253,6 @@ async function importModuleDynamicallyCallback(referrerSymbol, specifier, phase, // and fall back to the default loader. if (referrerSymbol === vm_dynamic_import_main_context_default) { emitExperimentalWarning('vm.USE_MAIN_CONTEXT_DEFAULT_LOADER'); - // TODO: Discover why this is needed for vm to error with the correct code when esm/defaultLoad - // is sync. @see https://github.com/nodejs/node/pull/57419#discussion_r2027158688 - if (referrerName === 'node:internal/process/task_queues') { referrerName = undefined; } return defaultImportModuleDynamicallyForScript(specifier, phase, attributes, referrerName); } // For script compiled internally that should use the default loader to handle dynamic From 7c5787a6538fc2ee859a2cbf7d7413937c610628 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Tue, 22 Apr 2025 22:11:37 +0200 Subject: [PATCH 10/10] module: exclude `node:` prefixed referrer as "parsable url" --- lib/internal/modules/helpers.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index c3122118cab75d..00c1060ba4eacc 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -263,8 +263,13 @@ function normalizeReferrerURL(referrerName) { return pathToFileURL(referrerName).href; } - if (StringPrototypeStartsWith(referrerName, 'file://') || - URLCanParse(referrerName)) { + if ( + StringPrototypeStartsWith(referrerName, 'file://') || + ( + !StringPrototypeStartsWith(referrerName, 'node:') && + URLCanParse(referrerName) + ) + ) { return referrerName; }