From 49558a114dd9f9538b6ebc98aa6bfc5234c6d57b Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sat, 12 Mar 2022 17:34:00 +0100 Subject: [PATCH 1/8] esm: emit experimental warnings in common place --- lib/internal/modules/esm/loader.js | 13 ++++++++ lib/internal/modules/esm/resolve.js | 8 ----- lib/internal/process/esm_loader.js | 3 -- .../test-esm-experimental-warnings.mjs | 31 +++++++++++++++++++ ...est-esm-specifiers-legacy-flag-warning.mjs | 24 -------------- 5 files changed, 44 insertions(+), 35 deletions(-) create mode 100644 test/es-module/test-esm-experimental-warnings.mjs delete mode 100644 test/es-module/test-esm-specifiers-legacy-flag-warning.mjs diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 16b832527f7c64..f05d7ba5d432fa 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -31,6 +31,7 @@ const { ERR_UNKNOWN_MODULE_FORMAT } = require('internal/errors').codes; const { pathToFileURL, isURLInstance, URL } = require('internal/url'); +const { emitExperimentalWarning } = require('internal/util'); const { isAnyArrayBuffer, isArrayBufferView, @@ -107,6 +108,18 @@ class ESMLoader { */ translators = translators; + constructor() { + if (getOptionValue('--experimental-loader')) { + emitExperimentalWarning('Custom ESM Loaders'); + } + if (getOptionValue('--experimental-network-imports')) { + emitExperimentalWarning('Network Imports'); + } + if (getOptionValue('--experimental-specifier-resolution') === 'node') { + emitExperimentalWarning('Specifier Resolution'); + } + } + static pluckHooks({ globalPreload, resolve, diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 53085f327469b3..e640947450c973 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -364,7 +364,6 @@ function resolveDirectoryEntry(search) { } const encodedSepRegEx = /%2F|%5C/i; -let experimentalSpecifierResolutionWarned = false; /** * @param {URL} resolved * @param {string | URL | undefined} base @@ -379,13 +378,6 @@ function finalizeResolution(resolved, base, preserveSymlinks) { let path = fileURLToPath(resolved); if (getOptionValue('--experimental-specifier-resolution') === 'node') { - if (!experimentalSpecifierResolutionWarned) { - process.emitWarning( - 'The Node.js specifier resolution flag is experimental. It could change or be removed at any time.', - 'ExperimentalWarning'); - experimentalSpecifierResolutionWarned = true; - } - let file = resolveExtensionsWithTryExactName(resolved); // Directory diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 73385a85b4e106..20021134ce40f3 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -55,9 +55,6 @@ async function initializeLoader() { if (!customLoaders.length) return; - const { emitExperimentalWarning } = require('internal/util'); - emitExperimentalWarning('--experimental-loader'); - let cwd; try { cwd = process.cwd() + '/'; diff --git a/test/es-module/test-esm-experimental-warnings.mjs b/test/es-module/test-esm-experimental-warnings.mjs new file mode 100644 index 00000000000000..2b8cf9216d4017 --- /dev/null +++ b/test/es-module/test-esm-experimental-warnings.mjs @@ -0,0 +1,31 @@ +import { mustCall } from '../common/index.mjs'; +import { fileURL } from '../common/fixtures.mjs'; +import { ok, strictEqual } from 'assert'; +import { spawn } from 'child_process'; +import { execPath } from 'process'; + +// Verify experimental warnings are printed +for ( + const [experiment, arg] of [ + ['Custom ESM Loaders', `--experimental-loader=${fileURL('es-module-loaders', 'hooks-custom.mjs')}`], + ['Network Imports', '--experimental-network-imports'], + ['Specifier Resolution', '--experimental-specifier-resolution=node'], + ] +) { + const child = spawn(execPath, [ + arg, + '--input-type=module', + '--eval', + `const foo = 'a'`, + ]); + + 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); + ok(stderr.includes('ExperimentalWarning:')); + ok(stderr.includes(experiment), new Error(`Expected warning to mention ${experiment}`)); + })); +} diff --git a/test/es-module/test-esm-specifiers-legacy-flag-warning.mjs b/test/es-module/test-esm-specifiers-legacy-flag-warning.mjs deleted file mode 100644 index 244499a3e02093..00000000000000 --- a/test/es-module/test-esm-specifiers-legacy-flag-warning.mjs +++ /dev/null @@ -1,24 +0,0 @@ -import { mustCall } from '../common/index.mjs'; -import { fileURL } from '../common/fixtures.mjs'; -import { match, strictEqual } from 'assert'; -import { spawn } from 'child_process'; -import { execPath } from 'process'; - -// Verify experimental warning is printed -const child = spawn(execPath, [ - '--experimental-specifier-resolution=node', - '--input-type=module', - '--eval', - `import ${JSON.stringify(fileURL('es-module-specifiers', 'package-type-module'))}`, -]); - -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); - match(stderr, /ExperimentalWarning: The Node\.js specifier resolution flag is experimental/); -})); From 61c0135737d1d3369571c097fc6e8e946e4af631 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sat, 12 Mar 2022 22:30:31 +0100 Subject: [PATCH 2/8] fixup: switch test from substring to regex --- test/es-module/test-esm-experimental-warnings.mjs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/es-module/test-esm-experimental-warnings.mjs b/test/es-module/test-esm-experimental-warnings.mjs index 2b8cf9216d4017..30e481f4b30e7f 100644 --- a/test/es-module/test-esm-experimental-warnings.mjs +++ b/test/es-module/test-esm-experimental-warnings.mjs @@ -1,15 +1,15 @@ import { mustCall } from '../common/index.mjs'; import { fileURL } from '../common/fixtures.mjs'; -import { ok, strictEqual } from 'assert'; +import { match, strictEqual } from 'assert'; import { spawn } from 'child_process'; import { execPath } from 'process'; // Verify experimental warnings are printed for ( const [experiment, arg] of [ - ['Custom ESM Loaders', `--experimental-loader=${fileURL('es-module-loaders', 'hooks-custom.mjs')}`], - ['Network Imports', '--experimental-network-imports'], - ['Specifier Resolution', '--experimental-specifier-resolution=node'], + [/Custom ESM Loaders/, `--experimental-loader=${fileURL('es-module-loaders', 'hooks-custom.mjs')}`], + [/Network Imports/, '--experimental-network-imports'], + [/Specifier Resolution/, '--experimental-specifier-resolution=node'], ] ) { const child = spawn(execPath, [ @@ -25,7 +25,7 @@ for ( child.on('close', mustCall((code, signal) => { strictEqual(code, 0); strictEqual(signal, null); - ok(stderr.includes('ExperimentalWarning:')); - ok(stderr.includes(experiment), new Error(`Expected warning to mention ${experiment}`)); + match(stderr, /ExperimentalWarning:/); + match(stderr, experiment); })); } From 463ae28954ed66b010953ad6cd6496e68fe9c170 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sat, 12 Mar 2022 22:34:54 +0100 Subject: [PATCH 3/8] fixup: delint --- test/es-module/test-esm-experimental-warnings.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/es-module/test-esm-experimental-warnings.mjs b/test/es-module/test-esm-experimental-warnings.mjs index 30e481f4b30e7f..3d849b4810968c 100644 --- a/test/es-module/test-esm-experimental-warnings.mjs +++ b/test/es-module/test-esm-experimental-warnings.mjs @@ -16,12 +16,12 @@ for ( arg, '--input-type=module', '--eval', - `const foo = 'a'`, + 'const foo = "a"', ]); let stderr = ''; child.stderr.setEncoding('utf8'); - child.stderr.on('data', (data) => { stderr += data }); + child.stderr.on('data', (data) => { stderr += data; }); child.on('close', mustCall((code, signal) => { strictEqual(code, 0); strictEqual(signal, null); From 59eff1ff8763a3279e9f0ef8ea0d944e4af603d0 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 20 Mar 2022 19:13:47 +0100 Subject: [PATCH 4/8] WIP: switch specifier resolution warning to custom message IRL, both internal and external instances of ESMLoader happen. in the test, only 1 external happens and then the test fails. --- lib/internal/modules/esm/loader.js | 10 ++++++++-- lib/internal/process/esm_loader.js | 2 +- test/es-module/test-esm-experimental-warnings.mjs | 13 ++++++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index f05d7ba5d432fa..608c5b1875b7b4 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -108,7 +108,10 @@ class ESMLoader { */ translators = translators; - constructor() { + constructor(isInternal = false) { +console.log({ isInternal }) + if (isInternal) return; + if (getOptionValue('--experimental-loader')) { emitExperimentalWarning('Custom ESM Loaders'); } @@ -116,7 +119,10 @@ class ESMLoader { emitExperimentalWarning('Network Imports'); } if (getOptionValue('--experimental-specifier-resolution') === 'node') { - emitExperimentalWarning('Specifier Resolution'); + process.emitWarning( + 'The Node.js specifier resolution flag is experimental. It could change or be removed at any time.', + 'ExperimentalWarning' + ); } } diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index 20021134ce40f3..ee20f2badf6621 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -39,7 +39,7 @@ async function importModuleDynamicallyCallback(wrap, specifier, assertions) { throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING(); }; -const esmLoader = new ESMLoader(); +const esmLoader = new ESMLoader(true); exports.esmLoader = esmLoader; diff --git a/test/es-module/test-esm-experimental-warnings.mjs b/test/es-module/test-esm-experimental-warnings.mjs index 3d849b4810968c..2d9a976c1896fd 100644 --- a/test/es-module/test-esm-experimental-warnings.mjs +++ b/test/es-module/test-esm-experimental-warnings.mjs @@ -9,23 +9,30 @@ for ( const [experiment, arg] of [ [/Custom ESM Loaders/, `--experimental-loader=${fileURL('es-module-loaders', 'hooks-custom.mjs')}`], [/Network Imports/, '--experimental-network-imports'], - [/Specifier Resolution/, '--experimental-specifier-resolution=node'], + [/specifier resolution/, '--experimental-specifier-resolution=node'], ] ) { + const input = `import ${JSON.stringify(fileURL('es-module-loaders','module-named-exports.mjs'))}`; +console.log({ input }) const child = spawn(execPath, [ arg, '--input-type=module', '--eval', - 'const foo = "a"', + input, ]); let stderr = ''; child.stderr.setEncoding('utf8'); child.stderr.on('data', (data) => { stderr += data; }); + + let stdout = ''; + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (data) => { stdout += data; }); child.on('close', mustCall((code, signal) => { +console.log({ experiment, code, signal, stderr, stdout }) strictEqual(code, 0); strictEqual(signal, null); - match(stderr, /ExperimentalWarning:/); + match(stderr, /ExperimentalWarning/); match(stderr, experiment); })); } From ad57f42f18f01e0249a2e5deac27513cc3310a2d Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 27 Mar 2022 19:11:44 -0700 Subject: [PATCH 5/8] fixup: remove log --- lib/internal/modules/esm/loader.js | 1 - test/es-module/test-esm-experimental-warnings.mjs | 2 -- 2 files changed, 3 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 608c5b1875b7b4..da7383300972fe 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -109,7 +109,6 @@ class ESMLoader { translators = translators; constructor(isInternal = false) { -console.log({ isInternal }) if (isInternal) return; if (getOptionValue('--experimental-loader')) { diff --git a/test/es-module/test-esm-experimental-warnings.mjs b/test/es-module/test-esm-experimental-warnings.mjs index 2d9a976c1896fd..adc6b645f88fbe 100644 --- a/test/es-module/test-esm-experimental-warnings.mjs +++ b/test/es-module/test-esm-experimental-warnings.mjs @@ -13,7 +13,6 @@ for ( ] ) { const input = `import ${JSON.stringify(fileURL('es-module-loaders','module-named-exports.mjs'))}`; -console.log({ input }) const child = spawn(execPath, [ arg, '--input-type=module', @@ -29,7 +28,6 @@ console.log({ input }) child.stdout.setEncoding('utf8'); child.stdout.on('data', (data) => { stdout += data; }); child.on('close', mustCall((code, signal) => { -console.log({ experiment, code, signal, stderr, stdout }) strictEqual(code, 0); strictEqual(signal, null); match(stderr, /ExperimentalWarning/); From 592bf88a40b5e642212ca1912bc7c22779f3430f Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 27 Mar 2022 19:23:44 -0700 Subject: [PATCH 6/8] fixup: remove isInternal flag for ESMLoader --- lib/internal/modules/esm/loader.js | 4 +--- lib/internal/process/esm_loader.js | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index da7383300972fe..70b6cc5fa24b6e 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -108,9 +108,7 @@ class ESMLoader { */ translators = translators; - constructor(isInternal = false) { - if (isInternal) return; - + constructor() { if (getOptionValue('--experimental-loader')) { emitExperimentalWarning('Custom ESM Loaders'); } diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js index ee20f2badf6621..20021134ce40f3 100644 --- a/lib/internal/process/esm_loader.js +++ b/lib/internal/process/esm_loader.js @@ -39,7 +39,7 @@ async function importModuleDynamicallyCallback(wrap, specifier, assertions) { throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING(); }; -const esmLoader = new ESMLoader(true); +const esmLoader = new ESMLoader(); exports.esmLoader = esmLoader; From 35bb8b00cb9c9b70070b9ca02df4ae7fa7a3aea4 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 27 Mar 2022 19:56:16 -0700 Subject: [PATCH 7/8] fixup: lint --- test/es-module/test-esm-experimental-warnings.mjs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/es-module/test-esm-experimental-warnings.mjs b/test/es-module/test-esm-experimental-warnings.mjs index adc6b645f88fbe..bf92b158e485eb 100644 --- a/test/es-module/test-esm-experimental-warnings.mjs +++ b/test/es-module/test-esm-experimental-warnings.mjs @@ -12,7 +12,7 @@ for ( [/specifier resolution/, '--experimental-specifier-resolution=node'], ] ) { - const input = `import ${JSON.stringify(fileURL('es-module-loaders','module-named-exports.mjs'))}`; + const input = `import ${JSON.stringify(fileURL('es-module-loaders', 'module-named-exports.mjs'))}`; const child = spawn(execPath, [ arg, '--input-type=module', @@ -23,10 +23,6 @@ for ( let stderr = ''; child.stderr.setEncoding('utf8'); child.stderr.on('data', (data) => { stderr += data; }); - - let stdout = ''; - child.stdout.setEncoding('utf8'); - child.stdout.on('data', (data) => { stdout += data; }); child.on('close', mustCall((code, signal) => { strictEqual(code, 0); strictEqual(signal, null); From 4c9ecfb95090309f46333f33444821ad526e6aa3 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Mon, 28 Mar 2022 11:06:51 -0700 Subject: [PATCH 8/8] fixup: prevent the specifier resolution warning from being printed twice --- lib/internal/modules/esm/loader.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 70b6cc5fa24b6e..0dccfebbd4f37f 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -54,6 +54,13 @@ const { fetchModule, } = require('internal/modules/esm/fetch_module'); + +/** + * Prevent the specifier resolution warning from being printed twice + */ +let emittedSpecifierResolutionWarning = false; + + /** * An ESMLoader instance is used as the main entry point for loading ES modules. * Currently, this is a singleton -- there is only one used for loading @@ -115,11 +122,12 @@ class ESMLoader { if (getOptionValue('--experimental-network-imports')) { emitExperimentalWarning('Network Imports'); } - if (getOptionValue('--experimental-specifier-resolution') === 'node') { + if (getOptionValue('--experimental-specifier-resolution') === 'node' && !emittedSpecifierResolutionWarning) { process.emitWarning( 'The Node.js specifier resolution flag is experimental. It could change or be removed at any time.', 'ExperimentalWarning' ); + emittedSpecifierResolutionWarning = true; } }