From c9f8fb7bb252ba9a724cfc0d0d2e0c399d7cf7de Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 10 Jul 2022 18:52:19 +0200 Subject: [PATCH 1/3] esm: fix erroneous re-initialization of ESMLoader --- lib/internal/process/esm_loader.js | 10 ++++++- test/es-module/test-esm-initialization.mjs | 31 ++++++++++++++++++++++ test/fixtures/es-modules/runmain.mjs | 7 +++++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 test/es-module/test-esm-initialization.mjs create mode 100644 test/fixtures/es-modules/runmain.mjs diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 4634ff5a9f5101..eeb6ad77a7ec88 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -40,14 +40,20 @@ async function importModuleDynamicallyCallback(wrap, specifier, assertions) { }; const esmLoader = new ESMLoader(); - exports.esmLoader = esmLoader; +// Module.runMain() causes loadESM() to re-run (which it should do); however, this should NOT cause +// ESM to be re-initialised; doing so causes duplicate custom loaders to be added to the public +// esmLoader. +let isESMInitialized = false; + /** * Causes side-effects: user-defined loader hooks are added to esmLoader. * @returns {void} */ async function initializeLoader() { + if (isESMInitialized) { return; } + const { getOptionValue } = require('internal/options'); const customLoaders = getOptionValue('--experimental-loader'); @@ -75,6 +81,8 @@ async function initializeLoader() { // Hooks must then be added to external/public loader // (so they're triggered in userland) await esmLoader.addCustomLoaders(keyedExportsList); + + isESMInitialized = true; } exports.loadESM = async function loadESM(callback) { diff --git a/test/es-module/test-esm-initialization.mjs b/test/es-module/test-esm-initialization.mjs new file mode 100644 index 00000000000000..c69b2d8f7c804d --- /dev/null +++ b/test/es-module/test-esm-initialization.mjs @@ -0,0 +1,31 @@ +import '../common/index.mjs'; +import fixtures from '../common/fixtures.js'; +import assert from 'node:assert'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; + + +{ // Verify unadulterated source is loaded when there are no loaders + const { status, stderr, stdout } = spawnSync( + process.execPath, + [ + '--loader', + fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), + '--no-warnings', + fileURLToPath(fixtures.fileURL('es-modules', 'runmain.mjs')), + ], + { encoding: 'utf8' }, + ); + + const resolveHookRunCount = [...(stdout.matchAll(/resolve passthru/g) ?? new Array())] + .length - 1; // less 1 because the first is the needle + + assert.strictEqual(stderr, ''); + /** + * resolveHookRunCount = 2: + * 1. fixtures/…/runmain.mjs + * 2. node:module (imported by fixtures/…/runmain.mjs) + */ + assert.strictEqual(resolveHookRunCount, 2); + assert.strictEqual(status, 0); +} diff --git a/test/fixtures/es-modules/runmain.mjs b/test/fixtures/es-modules/runmain.mjs new file mode 100644 index 00000000000000..5ceb86b66c76ce --- /dev/null +++ b/test/fixtures/es-modules/runmain.mjs @@ -0,0 +1,7 @@ +import { runMain } from 'node:module'; + +try { await import.meta.resolve('doesnt-matter.mjs') } catch {} + +runMain(); + +try { await import.meta.resolve('doesnt-matter.mjs') } catch {} From d2f2ec06906c2d67bbc513a2106997a0f69e112d Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 10 Jul 2022 19:48:04 +0200 Subject: [PATCH 2/3] fixup! esm: fix erroneous re-initialization of ESMLoader Antoine fanciness :) --- test/es-module/test-esm-initialization.mjs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/es-module/test-esm-initialization.mjs b/test/es-module/test-esm-initialization.mjs index c69b2d8f7c804d..76aefee337587d 100644 --- a/test/es-module/test-esm-initialization.mjs +++ b/test/es-module/test-esm-initialization.mjs @@ -1,8 +1,7 @@ import '../common/index.mjs'; -import fixtures from '../common/fixtures.js'; +import * as fixtures from '../common/fixtures.mjs'; import assert from 'node:assert'; import { spawnSync } from 'node:child_process'; -import { fileURLToPath } from 'node:url'; { // Verify unadulterated source is loaded when there are no loaders @@ -12,7 +11,7 @@ import { fileURLToPath } from 'node:url'; '--loader', fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'), '--no-warnings', - fileURLToPath(fixtures.fileURL('es-modules', 'runmain.mjs')), + fixtures.path('es-modules', 'runmain.mjs'), ], { encoding: 'utf8' }, ); From 26643e28d43b19c5f9351250a67bd62db294c672 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 10 Jul 2022 17:35:30 -0400 Subject: [PATCH 3/3] fixup: code review improvement for `resolveHookRunCount` Co-authored-by: Antoine du Hamel --- test/es-module/test-esm-initialization.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/es-module/test-esm-initialization.mjs b/test/es-module/test-esm-initialization.mjs index 76aefee337587d..ab756c7a3619e1 100644 --- a/test/es-module/test-esm-initialization.mjs +++ b/test/es-module/test-esm-initialization.mjs @@ -16,8 +16,8 @@ import { spawnSync } from 'node:child_process'; { encoding: 'utf8' }, ); - const resolveHookRunCount = [...(stdout.matchAll(/resolve passthru/g) ?? new Array())] - .length - 1; // less 1 because the first is the needle + // Length minus 1 because the first match is the needle. + const resolveHookRunCount = (stdout.match(/resolve passthru/g)?.length ?? 0) - 1; assert.strictEqual(stderr, ''); /**