From 25017ca10765744ed9ccd9fb1c09d89b64587409 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth <456802+GeoffreyBooth@users.noreply.github.com> Date: Thu, 10 Mar 2022 03:43:47 -0800 Subject: [PATCH] esm: add runtime warning for specifier resolution flag PR-URL: https://github.com/nodejs/node/pull/42252 Reviewed-By: Antoine du Hamel Reviewed-By: Jacob Smith Reviewed-By: Rich Trott --- doc/api/esm.md | 19 +++++++++------ lib/internal/modules/esm/formats.js | 12 ---------- lib/internal/modules/esm/resolve.js | 8 +++++++ ...est-esm-specifiers-legacy-flag-warning.mjs | 24 +++++++++++++++++++ 4 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 test/es-module/test-esm-specifiers-legacy-flag-warning.mjs diff --git a/doc/api/esm.md b/doc/api/esm.md index 16ec0ab39790ee..4886123f8632e6 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -667,7 +667,7 @@ of Node.js applications. > Stability: 1 - Experimental -**Note: This API is currently being redesigned and will still change.** +> This API is currently being redesigned and will still change. @@ -688,7 +688,7 @@ changes: description: Add support for import assertions. --> -> Note: The loaders API is being redesigned. This hook may disappear or its +> The loaders API is being redesigned. This hook may disappear or its > signature may change. Do not rely on the API described below. * `specifier` {string} @@ -761,10 +761,10 @@ export async function resolve(specifier, context, defaultResolve) { #### `load(url, context, defaultLoad)` -> Note: The loaders API is being redesigned. This hook may disappear or its +> The loaders API is being redesigned. This hook may disappear or its > signature may change. Do not rely on the API described below. -> Note: In a previous version of this API, this was split across 3 separate, now +> In a previous version of this API, this was split across 3 separate, now > deprecated, hooks (`getFormat`, `getSource`, and `transformSource`). * `url` {string} @@ -802,7 +802,7 @@ overcome in the future. > are incompatible. Attempting to use them together will result in an empty > object from the import. This may be addressed in the future. -> Note: These types all correspond to classes defined in ECMAScript. +> These types all correspond to classes defined in ECMAScript. * The specific [`ArrayBuffer`][] object is a [`SharedArrayBuffer`][]. * The specific [`TypedArray`][] object is a [`Uint8Array`][]. @@ -852,10 +852,10 @@ source to a supported one (see [Examples](#examples) below). #### `globalPreload()` -> Note: The loaders API is being redesigned. This hook may disappear or its +> The loaders API is being redesigned. This hook may disappear or its > signature may change. Do not rely on the API described below. -> Note: In a previous version of this API, this hook was named +> In a previous version of this API, this hook was named > `getGlobalPreloadCode`. * Returns: {string} @@ -1465,6 +1465,10 @@ _internal_, _conditions_) > Stability: 1 - Experimental +> Do not rely on this flag. We plan to remove it once the +> [Loaders API][] has advanced to the point that equivalent functionality can +> be achieved via custom loaders. + The current specifier resolution does not support all default behavior of the CommonJS loader. One of the behavior differences is automatic resolution of file extensions and the ability to import directories that have an index @@ -1497,6 +1501,7 @@ success! [Import Assertions]: #import-assertions [Import Assertions proposal]: https://github.com/tc39/proposal-import-assertions [JSON modules]: #json-modules +[Loaders API]: #loaders [Node.js Module Resolution Algorithm]: #resolver-algorithm-specification [Terminology]: #terminology [URL]: https://url.spec.whatwg.org/ diff --git a/lib/internal/modules/esm/formats.js b/lib/internal/modules/esm/formats.js index 8fbe0f38446862..6f145931a1eed4 100644 --- a/lib/internal/modules/esm/formats.js +++ b/lib/internal/modules/esm/formats.js @@ -7,8 +7,6 @@ const { getOptionValue } = require('internal/options'); const experimentalWasmModules = getOptionValue('--experimental-wasm-modules'); -const experimentalSpecifierResolution = - getOptionValue('--experimental-specifier-resolution'); const extensionFormatMap = { '__proto__': null, @@ -43,17 +41,7 @@ function mimeToFormat(mime) { return null; } -let experimentalSpecifierResolutionWarned = false; function getLegacyExtensionFormat(ext) { - if ( - experimentalSpecifierResolution === 'node' && - !experimentalSpecifierResolutionWarned - ) { - process.emitWarning( - 'The Node.js specifier resolution in ESM is experimental.', - 'ExperimentalWarning'); - experimentalSpecifierResolutionWarned = true; - } return legacyExtensionFormatMap[ext]; } diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 2c4644b7a2cd4e..d870907d6e0f6c 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -386,6 +386,7 @@ function resolveDirectoryEntry(search) { } const encodedSepRegEx = /%2F|%5C/i; +let experimentalSpecifierResolutionWarned = false; /** * @param {URL} resolved * @param {string | URL | undefined} base @@ -400,6 +401,13 @@ 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/test/es-module/test-esm-specifiers-legacy-flag-warning.mjs b/test/es-module/test-esm-specifiers-legacy-flag-warning.mjs new file mode 100644 index 00000000000000..244499a3e02093 --- /dev/null +++ b/test/es-module/test-esm-specifiers-legacy-flag-warning.mjs @@ -0,0 +1,24 @@ +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/); +}));