esm: remove specifier resolution flag

PR-URL: #44859 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Guy Bedford <[email protected]>
nodejs · Oct 4, 2022 · f594cc8 · f594cc8
1 parent 417458d
commit f594cc8
Show file tree

Hide file tree

Showing 26 changed files with 24 additions and 374 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -427,23 +427,6 @@ added: REPLACEME
 
 Use this flag to enable [ShadowRealm][] support.
 
-### `--experimental-specifier-resolution=mode`
-
-<!-- YAML
-added:
- - v13.4.0
- - v12.16.0
--->
-
-Sets the resolution algorithm for resolving ES module specifiers. Valid options
-are `explicit` and `node`.
-
-The default is `explicit`, which requires providing the full path to a
-module. The `node` mode enables support for optional file extensions and
-the ability to import a directory that has an index file.
-
-See [customizing ESM specifier resolution][] for example usage.
-
 ### `--experimental-vm-modules`
 
 <!-- YAML
@@ -2312,7 +2295,6 @@ done
 [`worker_threads.threadId`]: worker_threads.md#workerthreadid
 [conditional exports]: packages.md#conditional-exports
 [context-aware]: addons.md#context-aware-addons
-[customizing ESM specifier resolution]: esm.md#customizing-esm-specifier-resolution-algorithm
 [debugger]: debugger.md
 [debugging security implications]: https://nodejs.org/en/docs/guides/debugging-getting-started/#security-implications
 [emit_warning]: process.md#processemitwarningwarning-options

diff --git a/doc/api/esm.md b/doc/api/esm.md
@@ -1518,31 +1518,9 @@ _isImports_, _conditions_)
 
 ### Customizing ESM specifier resolution algorithm
 
-> 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
-file.
-
-The `--experimental-specifier-resolution=[mode]` flag can be used to customize
-the extension resolution algorithm. The default mode is `explicit`, which
-requires the full path to a module be provided to the loader. To enable the
-automatic extension resolution and importing from directories that include an
-index file use the `node` mode.
-
-```console
-$ node index.mjs
-success!
-$ node index # Failure!
-Error: Cannot find module
-$ node --experimental-specifier-resolution=node index
-success!
-```
+The [Loaders API][] provides a mechanism for customizing the ESM specifier
+resolution algorithm. An example loader that provides CommonJS-style resolution
+for ESM specifiers is [commonjs-extension-resolution-loader][].
 
 <!-- Note: The cjs-module-lexer link should be kept in-sync with the deps version -->
 
@@ -1583,6 +1561,7 @@ success!
 [`string`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String
 [`util.TextDecoder`]: util.md#class-utiltextdecoder
 [cjs-module-lexer]: https://github.com/nodejs/cjs-module-lexer/tree/1.2.2
+[commonjs-extension-resolution-loader]: https://github.com/nodejs/loaders-test/tree/main/commonjs-extension-resolution-loader
 [custom https loader]: #https-loader
 [load hook]: #loadurl-context-nextload
 [percent-encoded]: url.md#percent-encoding-in-urls

diff --git a/doc/node.1 b/doc/node.1
@@ -171,9 +171,6 @@ Disable exposition of the Web Crypto API on the global scope.
 .It Fl -no-experimental-repl-await
 Disable top-level await keyword support in REPL.
 .
-.It Fl -experimental-specifier-resolution
-Select extension resolution algorithm for ES Modules; either 'explicit' (default) or 'node'.
-.
 .It Fl -experimental-vm-modules
 Enable experimental ES module support in VM module.
 .

diff --git a/lib/internal/modules/esm/formats.js b/lib/internal/modules/esm/formats.js
@@ -5,7 +5,6 @@ const {
 } = primordials;
 const { getOptionValue } = require('internal/options');
 
-
 const experimentalWasmModules = getOptionValue('--experimental-wasm-modules');
 
 const extensionFormatMap = {
@@ -16,17 +15,8 @@ const extensionFormatMap = {
   '.mjs': 'module',
 };
 
-const legacyExtensionFormatMap = {
-  '__proto__': null,
-  '.cjs': 'commonjs',
-  '.js': 'commonjs',
-  '.json': 'commonjs',
-  '.mjs': 'module',
-  '.node': 'commonjs',
-};
-
 if (experimentalWasmModules) {
-  extensionFormatMap['.wasm'] = legacyExtensionFormatMap['.wasm'] = 'wasm';
+  extensionFormatMap['.wasm'] = 'wasm';
 }
 
 /**
@@ -45,13 +35,7 @@ function mimeToFormat(mime) {
   return null;
 }
 
-function getLegacyExtensionFormat(ext) {
-  return legacyExtensionFormatMap[ext];
-}
-
 module.exports = {
   extensionFormatMap,
-  getLegacyExtensionFormat,
-  legacyExtensionFormatMap,
   mimeToFormat,
 };
diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js
@@ -11,14 +11,11 @@ const { getOptionValue } = require('internal/options');
 const { fetchModule } = require('internal/modules/esm/fetch_module');
 const {
   extensionFormatMap,
-  getLegacyExtensionFormat,
   mimeToFormat,
 } = require('internal/modules/esm/formats');
 
 const experimentalNetworkImports =
   getOptionValue('--experimental-network-imports');
-const experimentalSpecifierResolution =
-  getOptionValue('--experimental-specifier-resolution');
 const { getPackageType, getPackageScopeConfig } = require('internal/modules/esm/resolve');
 const { URL, fileURLToPath } = require('internal/url');
 const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
@@ -61,25 +58,21 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) {
   const format = extensionFormatMap[ext];
   if (format) return format;
 
-  if (experimentalSpecifierResolution !== 'node') {
-    // Explicit undefined return indicates load hook should rerun format check
-    if (ignoreErrors) return undefined;
-    let suggestion = '';
-    if (getPackageType(url) === 'module' && ext === '') {
-      const config = getPackageScopeConfig(url);
-      const fileBasename = basename(filepath);
-      const relativePath = StringPrototypeSlice(relative(config.pjsonPath, filepath), 1);
-      suggestion = 'Loading extensionless files is not supported inside of ' +
-        '"type":"module" package.json contexts. The package.json file ' +
-        `${config.pjsonPath} caused this "type":"module" context. Try ` +
-        `changing ${filepath} to have a file extension. Note the "bin" ` +
-        'field of package.json can point to a file with an extension, for example ' +
-        `{"type":"module","bin":{"${fileBasename}":"${relativePath}.js"}}`;
-    }
-    throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion);
+  // Explicit undefined return indicates load hook should rerun format check
+  if (ignoreErrors) { return undefined; }
+  let suggestion = '';
+  if (getPackageType(url) === 'module' && ext === '') {
+    const config = getPackageScopeConfig(url);
+    const fileBasename = basename(filepath);
+    const relativePath = StringPrototypeSlice(relative(config.pjsonPath, filepath), 1);
+    suggestion = 'Loading extensionless files is not supported inside of ' +
+      '"type":"module" package.json contexts. The package.json file ' +
+      `${config.pjsonPath} caused this "type":"module" context. Try ` +
+      `changing ${filepath} to have a file extension. Note the "bin" ` +
+      'field of package.json can point to a file with an extension, for example ' +
+      `{"type":"module","bin":{"${fileBasename}":"${relativePath}.js"}}`;
   }
-
-  return getLegacyExtensionFormat(ext) ?? null;
+  throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion);
 }
 
 /**

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -89,8 +89,6 @@ const { getOptionValue } = require('internal/options');
 
 // [2] `validate...()`s throw the wrong error
 
-let emittedSpecifierResolutionWarning = false;
-
 /**
  * A utility function to iterate through a hook chain, track advancement in the
  * chain, and generate and supply the `next<HookName>` argument to the custom
@@ -241,16 +239,6 @@ class ESMLoader {
     if (getOptionValue('--experimental-network-imports')) {
       emitExperimentalWarning('Network Imports');
     }
-    if (
-      !emittedSpecifierResolutionWarning &&
-      getOptionValue('--experimental-specifier-resolution') === 'node'
-    ) {
-      process.emitWarning(
-        'The Node.js specifier resolution flag is experimental. It could change or be removed at any time.',
-        'ExperimentalWarning'
-      );
-      emittedSpecifierResolutionWarning = true;
-    }
   }
 
   /**

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -5,7 +5,6 @@ const {
   ArrayPrototypeConcat,
   ArrayPrototypeJoin,
   ArrayPrototypeShift,
-  JSONParse,
   JSONStringify,
   ObjectFreeze,
   ObjectGetOwnPropertyNames,
@@ -37,7 +36,7 @@ const { getOptionValue } = require('internal/options');
 const policy = getOptionValue('--experimental-policy') ?
   require('internal/process/policy') :
   null;
-const { sep, relative, resolve } = require('path');
+const { sep, relative } = require('path');
 const preserveSymlinks = getOptionValue('--preserve-symlinks');
 const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
 const experimentalNetworkImports =
@@ -60,7 +59,6 @@ const {
 } = require('internal/errors').codes;
 
 const { Module: CJSModule } = require('internal/modules/cjs/loader');
-const packageJsonReader = require('internal/modules/package_json_reader');
 const { getPackageConfig, getPackageScopeConfig } = require('internal/modules/esm/package_config');
 
 /**
@@ -234,50 +232,6 @@ function legacyMainResolve(packageJSONUrl, packageConfig, base) {
     fileURLToPath(new URL('.', packageJSONUrl)), fileURLToPath(base));
 }
 
-/**
- * @param {URL} search
- * @returns {URL | undefined}
- */
-function resolveExtensionsWithTryExactName(search) {
-  if (fileExists(search)) return search;
-  return resolveExtensions(search);
-}
-
-const extensions = ['.js', '.json', '.node', '.mjs'];
-
-/**
- * @param {URL} search
- * @returns {URL | undefined}
- */
-function resolveExtensions(search) {
-  for (let i = 0; i < extensions.length; i++) {
-    const extension = extensions[i];
-    const guess = new URL(`${search.pathname}${extension}`, search);
-    if (fileExists(guess)) return guess;
-  }
-  return undefined;
-}
-
-/**
- * @param {URL} search
- * @returns {URL | undefined}
- */
-function resolveDirectoryEntry(search) {
-  const dirPath = fileURLToPath(search);
-  const pkgJsonPath = resolve(dirPath, 'package.json');
-  if (fileExists(pkgJsonPath)) {
-    const pkgJson = packageJsonReader.read(pkgJsonPath);
-    if (pkgJson.containsKeys) {
-      const { main } = JSONParse(pkgJson.string);
-      if (main != null) {
-        const mainUrl = pathToFileURL(resolve(dirPath, main));
-        return resolveExtensionsWithTryExactName(mainUrl);
-      }
-    }
-  }
-  return resolveExtensions(new URL('index', search));
-}
-
 const encodedSepRegEx = /%2F|%5C/i;
 /**
  * @param {URL} resolved
@@ -291,25 +245,7 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
       resolved.pathname, 'must not include encoded "/" or "\\" characters',
       fileURLToPath(base));
 
-  let path = fileURLToPath(resolved);
-  if (getOptionValue('--experimental-specifier-resolution') === 'node') {
-    let file = resolveExtensionsWithTryExactName(resolved);
-
-    // Directory
-    if (file === undefined) {
-      file = StringPrototypeEndsWith(path, '/') ?
-        (resolveDirectoryEntry(resolved) || resolved) : resolveDirectoryEntry(new URL(`${resolved}/`));
-
-      if (file === resolved) return file;
-
-      if (file === undefined) {
-        throw new ERR_MODULE_NOT_FOUND(
-          resolved.pathname, fileURLToPath(base), 'module');
-      }
-    }
-
-    path = file;
-  }
+  const path = fileURLToPath(resolved);
 
   const stats = tryStatSync(StringPrototypeEndsWith(path, '/') ?
     StringPrototypeSlice(path, -1) : path);

diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js
@@ -40,10 +40,6 @@ function shouldUseESMLoader(mainPath) {
   const userImports = getOptionValue('--import');
   if (userLoaders.length > 0 || userImports.length > 0)
     return true;
-  const esModuleSpecifierResolution =
-    getOptionValue('--experimental-specifier-resolution');
-  if (esModuleSpecifierResolution === 'node')
-    return true;
   // Determine the module format of the main
   if (mainPath && StringPrototypeEndsWith(mainPath, '.mjs'))
     return true;

diff --git a/lib/repl.js b/lib/repl.js
@@ -180,7 +180,6 @@ const {
 const history = require('internal/repl/history');
 const {
   extensionFormatMap,
-  legacyExtensionFormatMap,
 } = require('internal/modules/esm/formats');
 
 let nextREPLResourceNumber = 1;
@@ -1377,10 +1376,7 @@ function complete(line, callback) {
     if (this.allowBlockingCompletions) {
       const subdir = match[2] || '';
       // File extensions that can be imported:
-      const extensions = ObjectKeys(
-        getOptionValue('--experimental-specifier-resolution') === 'node' ?
-          legacyExtensionFormatMap :
-          extensionFormatMap);
+      const extensions = ObjectKeys(extensionFormatMap);
 
       // Only used when loading bare module specifiers from `node_modules`:
       const indexes = ArrayPrototypeMap(extensions, (ext) => `index${ext}`);

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -113,14 +113,6 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
     }
   }
 
-  if (!experimental_specifier_resolution.empty()) {
-    if (experimental_specifier_resolution != "node" &&
-        experimental_specifier_resolution != "explicit") {
-      errors->push_back(
-        "invalid value for --experimental-specifier-resolution");
-    }
-  }
-
   if (syntax_check_only && has_eval_string) {
     errors->push_back("either --check or --eval can be used, not both");
   }
@@ -444,11 +436,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "set module type for string input",
             &EnvironmentOptions::module_type,
             kAllowedInEnvironment);
-  AddOption("--experimental-specifier-resolution",
-            "Select extension resolution algorithm for es modules; "
-            "either 'explicit' (default) or 'node'",
-            &EnvironmentOptions::experimental_specifier_resolution,
-            kAllowedInEnvironment);
+  AddOption(
+      "--experimental-specifier-resolution", "", NoOp{}, kAllowedInEnvironment);
   AddAlias("--es-module-specifier-resolution",
            "--experimental-specifier-resolution");
   AddOption("--deprecation",

diff --git a/src/node_options.h b/src/node_options.h
@@ -112,7 +112,6 @@ class EnvironmentOptions : public Options {
   bool experimental_global_customevent = false;
   bool experimental_global_web_crypto = true;
   bool experimental_https_modules = false;
-  std::string experimental_specifier_resolution;
   bool experimental_wasm_modules = false;
   bool experimental_import_meta_resolve = false;
   std::string module_type;

diff --git a/test/es-module/test-esm-experimental-warnings.mjs b/test/es-module/test-esm-experimental-warnings.mjs
@@ -27,7 +27,6 @@ describe('ESM: warn for obsolete hooks provided', { concurrency: true }, () => {
       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'],
       ]
     ) {
       it(`should print for ${experiment.toString().replaceAll('/', '')}`, async () => {