From fa96f540282cad07d0090ff0455ab50e78ab2ea3 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Wed, 27 Nov 2019 01:54:23 -0500 Subject: [PATCH] esm: make specifier flag clearly experimental `--es-module-specifier-resolution` is the only flagged portion of the ESM implementation that does not have the word experimental in the flag name. This commit changes the flag to: `--experimental-specifier-resolution` `--es-module-specifier-resolution` remains as an alias for backwards compatibility but it is no longer documented. PR-URL: https://github.com/nodejs/node/pull/30678 Reviewed-By: Jan Krems Reviewed-By: Guy Bedford --- doc/api/cli.md | 30 +++++++++---------- doc/api/esm.md | 4 +-- doc/node.1 | 6 ++-- lib/internal/modules/esm/default_resolve.js | 12 +++++--- src/module_wrap.cc | 4 +-- src/node_options.cc | 30 ++++++++++++++++--- src/node_options.h | 1 + .../test-esm-specifiers-both-flags.mjs | 18 +++++++++++ .../test-esm-specifiers-legacy-flag.mjs | 18 +++++++++++ test/es-module/test-esm-specifiers.mjs | 2 +- ...rocess-env-allowed-flags-are-documented.js | 1 + 11 files changed, 95 insertions(+), 31 deletions(-) create mode 100644 test/es-module/test-esm-specifiers-both-flags.mjs create mode 100644 test/es-module/test-esm-specifiers-legacy-flag.mjs diff --git a/doc/api/cli.md b/doc/api/cli.md index 96287e46b4eb47..470c90eaac8267 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -156,20 +156,6 @@ Enable experimental Source Map V3 support for stack traces. Currently, overriding `Error.prepareStackTrace` is ignored when the `--enable-source-maps` flag is set. -### `--es-module-specifier-resolution=mode` - - -To be used in conjunction with `--experimental-modules`. Sets the resolution -algorithm for resolving specifiers. Valid options are `explicit` and `node`. - -The default is `explicit`, which requires providing the full path to a -module. The `node` mode will enable support for optional file extensions and -the ability to import a directory that has an index file. - -Please see [customizing ESM specifier resolution][] for example usage. - ### `--experimental-conditional-exports` + +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 will enable support for optional file extensions and +the ability to import a directory that has an index file. + +Please see [customizing ESM specifier resolution][] for example usage. + ### `--experimental-vm-modules` * `--enable-fips` * `--enable-source-maps` -* `--es-module-specifier-resolution` * `--experimental-conditional-exports` * `--experimental-json-modules` * `--experimental-loader` @@ -1091,6 +1090,7 @@ Node.js options that are allowed are: * `--experimental-repl-await` * `--experimental-report` * `--experimental-resolve-self` +* `--experimental-specifier-resolution` * `--experimental-vm-modules` * `--experimental-wasm-modules` * `--force-context-aware` diff --git a/doc/api/esm.md b/doc/api/esm.md index 7ff48909ec79cf..b256595c345941 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1375,7 +1375,7 @@ 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 `--es-module-specifier-resolution=[mode]` flag can be used to customize +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 @@ -1386,7 +1386,7 @@ $ node --experimental-modules index.mjs success! $ node --experimental-modules index #Failure! Error: Cannot find module -$ node --experimental-modules --es-module-specifier-resolution=node index +$ node --experimental-modules --experimental-specifier-resolution=node index success! ``` diff --git a/doc/node.1 b/doc/node.1 index 2b49a392a73b2c..8c483558213d78 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -110,9 +110,6 @@ Enable FIPS-compliant crypto at startup. Requires Node.js to be built with .Sy ./configure --openssl-fips . . -.It Fl -es-module-specifier-resolution -Select extension resolution algorithm for ES Modules; either 'explicit' (default) or 'node' -. .It Fl -experimental-conditional-exports Enable experimental support for "require" and "node" conditional export targets. . @@ -130,6 +127,9 @@ Enable experimental top-level .Sy 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-report Enable experimental .Sy diagnostic report diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index f436bc2a1a1d84..f02d01eb56abe1 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -9,8 +9,8 @@ const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); const experimentalJsonModules = getOptionValue('--experimental-json-modules'); -const esModuleSpecifierResolution = - getOptionValue('--es-module-specifier-resolution'); +const experimentalSpeciferResolution = + getOptionValue('--experimental-specifier-resolution'); const typeFlag = getOptionValue('--input-type'); const experimentalWasmModules = getOptionValue('--experimental-wasm-modules'); const { resolve: moduleWrapResolve, @@ -108,10 +108,14 @@ function resolve(specifier, parentURL) { if (ext === '.js' || (!format && isMain)) format = getPackageType(url.href) === TYPE_MODULE ? 'module' : 'commonjs'; if (!format) { - if (esModuleSpecifierResolution === 'node') + if (experimentalSpeciferResolution === 'node') { + process.emitWarning( + 'The Node.js specifier resolution in ESM is experimental.', + 'ExperimentalWarning'); format = legacyExtensionFormatMap[ext]; - else + } else { throw new ERR_UNKNOWN_FILE_EXTENSION(fileURLToPath(url)); + } } return { url: `${url}`, format }; } diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 5745cce9e099ab..3c3d568329221d 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -789,7 +789,7 @@ inline Maybe ResolveIndex(const URL& search) { Maybe FinalizeResolution(Environment* env, const URL& resolved, const URL& base) { - if (env->options()->es_module_specifier_resolution == "node") { + if (env->options()->experimental_specifier_resolution == "node") { Maybe file = ResolveExtensions(resolved); if (!file.IsNothing()) { return file; @@ -1053,7 +1053,7 @@ Maybe PackageMainResolve(Environment* env, return Just(resolved); } } - if (env->options()->es_module_specifier_resolution == "node") { + if (env->options()->experimental_specifier_resolution == "node") { if (pcfg.has_main == HasMain::Yes) { return FinalizeResolution(env, URL(pcfg.main, pjson_url), base); } else { diff --git a/src/node_options.cc b/src/node_options.cc index bb78a22f46c47a..247cec634a2297 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -155,9 +155,27 @@ void EnvironmentOptions::CheckOptions(std::vector* errors) { errors->push_back("--es-module-specifier-resolution requires " "--experimental-modules be enabled"); } - if (es_module_specifier_resolution != "node" && - es_module_specifier_resolution != "explicit") { - errors->push_back("invalid value for --es-module-specifier-resolution"); + if (!experimental_specifier_resolution.empty()) { + errors->push_back( + "bad option: cannot use --es-module-specifier-resolution" + " and --experimental-specifier-resolution at the same time"); + } else { + experimental_specifier_resolution = es_module_specifier_resolution; + if (experimental_specifier_resolution != "node" && + experimental_specifier_resolution != "explicit") { + errors->push_back( + "invalid value for --es-module-specifier-resolution"); + } + } + } else if (!experimental_specifier_resolution.empty()) { + if (!experimental_modules) { + errors->push_back("--experimental-specifier-resolution requires " + "--experimental-modules be enabled"); + } + if (experimental_specifier_resolution != "node" && + experimental_specifier_resolution != "explicit") { + errors->push_back( + "invalid value for --experimental-specifier-resolution"); } } @@ -408,9 +426,13 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "set module type for string input", &EnvironmentOptions::module_type, kAllowedInEnvironment); - AddOption("--es-module-specifier-resolution", + AddOption("--experimental-specifier-resolution", "Select extension resolution algorithm for es modules; " "either 'explicit' (default) or 'node'", + &EnvironmentOptions::experimental_specifier_resolution, + kAllowedInEnvironment); + AddOption("--es-module-specifier-resolution", + "", &EnvironmentOptions::es_module_specifier_resolution, kAllowedInEnvironment); AddOption("--no-deprecation", diff --git a/src/node_options.h b/src/node_options.h index cf7e20ca1c91ed..481077588bde99 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -105,6 +105,7 @@ class EnvironmentOptions : public Options { bool experimental_json_modules = false; bool experimental_modules = false; bool experimental_resolve_self = false; + std::string experimental_specifier_resolution; std::string es_module_specifier_resolution; bool experimental_wasm_modules = false; std::string module_type; diff --git a/test/es-module/test-esm-specifiers-both-flags.mjs b/test/es-module/test-esm-specifiers-both-flags.mjs new file mode 100644 index 00000000000000..42fb1cb68974a9 --- /dev/null +++ b/test/es-module/test-esm-specifiers-both-flags.mjs @@ -0,0 +1,18 @@ +// Flags: --experimental-modules +import { mustCall } from '../common/index.mjs'; +import { exec } from 'child_process'; +import assert from 'assert'; + +const expectedError = + 'cannot use --es-module-specifier-resolution ' + + 'and --experimental-specifier-resolution at the same time'; + +const flags = '--experimental-modules ' + + '--es-module-specifier-resolution=node ' + + '--experimental-specifier-resolution=node'; + +exec(`${process.execPath} ${flags}`, { + timeout: 300 +}, mustCall((error) => { + assert(error.message.includes(expectedError)); +})); diff --git a/test/es-module/test-esm-specifiers-legacy-flag.mjs b/test/es-module/test-esm-specifiers-legacy-flag.mjs new file mode 100644 index 00000000000000..03388493f0b8cb --- /dev/null +++ b/test/es-module/test-esm-specifiers-legacy-flag.mjs @@ -0,0 +1,18 @@ +// Flags: --experimental-modules --es-module-specifier-resolution=node +import '../common/index.mjs'; +import assert from 'assert'; + +// commonJS index.js +import commonjs from '../fixtures/es-module-specifiers/package-type-commonjs'; +// esm index.js +import module from '../fixtures/es-module-specifiers/package-type-module'; +// Notice the trailing slash +import success, { explicit, implicit, implicitModule } + from '../fixtures/es-module-specifiers/'; + +assert.strictEqual(commonjs, 'commonjs'); +assert.strictEqual(module, 'module'); +assert.strictEqual(success, 'success'); +assert.strictEqual(explicit, 'esm'); +assert.strictEqual(implicit, 'cjs'); +assert.strictEqual(implicitModule, 'cjs'); diff --git a/test/es-module/test-esm-specifiers.mjs b/test/es-module/test-esm-specifiers.mjs index 59d54cbf63dc79..499a14643859a4 100644 --- a/test/es-module/test-esm-specifiers.mjs +++ b/test/es-module/test-esm-specifiers.mjs @@ -1,4 +1,4 @@ -// Flags: --experimental-modules --es-module-specifier-resolution=node +// Flags: --experimental-modules --experimental-specifier-resolution=node import { mustNotCall } from '../common/index.mjs'; import assert from 'assert'; diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js index 2e0d67eefcb242..f356f88fe9c740 100644 --- a/test/parallel/test-process-env-allowed-flags-are-documented.js +++ b/test/parallel/test-process-env-allowed-flags-are-documented.js @@ -85,6 +85,7 @@ const undocumented = difference(process.allowedNodeEnvironmentFlags, documented); // Remove intentionally undocumented options. assert(undocumented.delete('--debug-arraybuffer-allocations')); +assert(undocumented.delete('--es-module-specifier-resolution')); assert(undocumented.delete('--experimental-worker')); assert(undocumented.delete('--no-node-snapshot')); assert(undocumented.delete('--loader'));