module: unflag conditional exports

PR-URL: #31001
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Myles Borins <[email protected]>

doc/api/cli.md

@@ -156,15 +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.
 
-### `--experimental-conditional-exports`
-<!-- YAML
-added: v13.2.0
--->
-
-Enable experimental support for the `"require"` and `"node"` conditional
-package export resolutions.
-See [Conditional Exports][] for more information.
-
 ### `--experimental-json-modules`
 <!-- YAML
 added: v12.9.0
@@ -177,8 +168,7 @@ Enable experimental JSON support for the ES Module loader.
 added: v8.5.0
 -->
 
-Enable latest experimental modules features (currently
-`--experimental-conditional-exports`).
+Enable latest experimental modules features (deprecated).
 
 ### `--experimental-policy`
 <!-- YAML
@@ -1076,7 +1066,6 @@ Node.js options that are allowed are:
 <!-- node-options-node start -->
 * `--enable-fips`
 * `--enable-source-maps`
-* `--experimental-conditional-exports`
 * `--experimental-json-modules`
 * `--experimental-loader`
 * `--experimental-modules`
@@ -1372,7 +1361,6 @@ greater than `4` (its current default value). For more information, see the
 [`tls.DEFAULT_MIN_VERSION`]: tls.html#tls_tls_default_min_version
 [`unhandledRejection`]: process.html#process_event_unhandledrejection
 [Chrome DevTools Protocol]: https://chromedevtools.github.io/devtools-protocol/
-[Conditional Exports]: esm.html#esm_conditional_exports
 [REPL]: repl.html
 [ScriptCoverage]: https://chromedevtools.github.io/devtools-protocol/tot/Profiler#type-ScriptCoverage
 [Source Map]: https://sourcemaps.info/spec.html

doc/api/esm.md

@@ -349,21 +349,14 @@ The conditions supported in Node.js condition matching:
    or ES module file.
 * `"import"` - matched when the package is loaded via `import` or
    `import()`. Can be any module format, this field does not set the type
-   interpretation. _This is currently only supported behind the
-   `--experimental-conditional-exports` flag._
+   interpretation.
 * `"node"` - matched for any Node.js environment. Can be a CommonJS or ES
-   module file. _This is currently only supported behind the
-   `--experimental-conditional-exports` flag._
+   module file.
 * `"require"` - matched when the package is loaded via `require()`.
-   _This is currently only supported behind the
-   `--experimental-conditional-exports` flag._
 
 Condition matching is applied in object order from first to last within the
 `"exports"` object.
 
-> Setting the above conditions for a published package is not recommended until
-> conditional exports have been unflagged to avoid breaking changes to packages.
-
 Using the `"require"` condition it is possible to define a package that will
 have a different exported value for CommonJS and ES modules, which can be a
 hazard in that it can result in having two separate instances of the same
@@ -456,10 +449,10 @@ ignores) the top-level `"module"` field.
 Node.js can now run ES module entry points, and a package can contain both
 CommonJS and ES module entry points (either via separate specifiers such as
 `'pkg'` and `'pkg/es-module'`, or both at the same specifier via [Conditional
-Exports][] with the `--experimental-conditional-exports` flag). Unlike in the
-scenario where `"module"` is only used by bundlers, or ES module files are
-transpiled into CommonJS on the fly before evaluation by Node.js, the files
-referenced by the ES module entry point are evaluated as ES modules.
+Exports][]). Unlike in the scenario where `"module"` is only used by bundlers,
+or ES module files are transpiled into CommonJS on the fly before evaluation by
+Node.js, the files referenced by the ES module entry point are evaluated as ES
+modules.
 
 #### Dual Package Hazard
 
@@ -518,13 +511,8 @@ following conditions:
 
 Write the package in CommonJS or transpile ES module sources into CommonJS, and
 create an ES module wrapper file that defines the named exports. Using
-[Conditional Exports][] via the `--experimental-conditional-exports` flag, the
-ES module wrapper is used for `import` and the CommonJS entry point for
-`require`.
-
-> Note: While `--experimental-conditional-exports` is flagged, a package
-> using this pattern will throw when loaded unless package consumers use the
-> `--experimental-conditional-exports` flag.
+[Conditional Exports][], the ES module wrapper is used for `import` and the
+CommonJS entry point for `require`.
 
 <!-- eslint-skip -->
 ```js
@@ -580,13 +568,13 @@ This approach is appropriate for any of the following use cases:
 * The package stores internal state, and the package author would prefer not to
   refactor the package to isolate its state management. See the next section.
 
-A variant of this approach not requiring `--experimental-conditional-exports`
-for consumers could be to add an export, e.g. `"./module"`, to point to an
-all-ES module-syntax version of the package. This could be used via `import
-'pkg/module'` by users who are certain that the CommonJS version will not be
-loaded anywhere in the application, such as by dependencies; or if the CommonJS
-version can be loaded but doesn’t affect the ES module version (for example,
-because the package is stateless):
+A variant of this approach not requiring conditional exports for consumers could
+be to add an export, e.g. `"./module"`, to point to an all-ES module-syntax
+version of the package. This could be used via `import 'pkg/module'` by users
+who are certain that the CommonJS version will not be loaded anywhere in the
+application, such as by dependencies; or if the CommonJS version can be loaded
+but doesn’t affect the ES module version (for example, because the package is
+stateless):
 
 <!-- eslint-skip -->
 ```js
@@ -601,16 +589,10 @@ because the package is stateless):
 }
 ```
 
-If the `--experimental-conditional-exports` flag is dropped and therefore
-[Conditional Exports][] become available without a flag, this variant could be
-easily updated to use conditional exports by adding conditions to the `"."`
-path; while keeping `"./module"` for backward compatibility.
-
 ##### Approach #2: Isolate State
 
 The most straightforward `package.json` would be one that defines the separate
-CommonJS and ES module entry points directly (requires
-`--experimental-conditional-exports`):
+CommonJS and ES module entry points directly:
 
 <!-- eslint-skip -->
 ```js
@@ -695,8 +677,8 @@ Even with isolated state, there is still the cost of possible extra code
 execution between the CommonJS and ES module versions of a package.
 
 As with the previous approach, a variant of this approach not requiring
-`--experimental-conditional-exports` for consumers could be to add an export,
-e.g. `"./module"`, to point to an all-ES module-syntax version of the package:
+conditional exports for consumers could be to add an export, e.g.
+`"./module"`, to point to an all-ES module-syntax version of the package:
 
 <!-- eslint-skip -->
 ```js
@@ -711,11 +693,6 @@ e.g. `"./module"`, to point to an all-ES module-syntax version of the package:
 }
 ```
 
-If the `--experimental-conditional-exports` flag is dropped and therefore
-[Conditional Exports][] become available without a flag, this variant could be
-easily updated to use conditional exports by adding conditions to the `"."`
-path; while keeping `"./module"` for backward compatibility.
-
 ## `import` Specifiers
 
 ### Terminology

doc/node.1

@@ -113,9 +113,6 @@ Requires Node.js to be built with
 .It Fl -enable-source-maps
 Enable experimental Source Map V3 support for stack traces.
 .
-.It Fl -experimental-conditional-exports
-Enable experimental support for "require" and "node" conditional export targets.
-.
 .It Fl -experimental-json-modules
 Enable experimental JSON interop support for the ES Module loader.
 .

lib/internal/modules/cjs/loader.js

@@ -69,8 +69,6 @@ const { getOptionValue } = require('internal/options');
 const enableSourceMaps = getOptionValue('--enable-source-maps');
 const preserveSymlinks = getOptionValue('--preserve-symlinks');
 const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
-const experimentalConditionalExports =
-    getOptionValue('--experimental-conditional-exports');
 const manifest = getOptionValue('--experimental-policy') ?
   require('internal/process/policy').manifest :
   null;
@@ -433,9 +431,6 @@ function resolveBasePath(basePath, exts, isMain, trailingSlash, request) {
 }
 
 function trySelf(parentPath, isMain, request) {
-  if (!experimentalConditionalExports) {
-    return false;
-  }
   const { data: pkg, path: basePath } = readPackageScope(parentPath) || {};
   if (!pkg || 'exports' in pkg === false) return false;
   if (typeof pkg.name !== 'string') return false;
@@ -608,8 +603,6 @@ function resolveExportsTarget(pkgPath, target, subpath, basePath, mappingKey) {
       switch (p) {
         case 'node':
         case 'require':
-          if (!experimentalConditionalExports)
-            continue;
           try {
             emitExperimentalWarning('Conditional exports');
             const result = resolveExportsTarget(pkgPath, target[p], subpath,

src/module_wrap.cc

@@ -1020,7 +1020,6 @@ Maybe<URL> ResolveExportsTarget(Environment* env,
                          key->ToString(context).ToLocalChecked());
       std::string key_str(*key_utf8, key_utf8.length());
       if (key_str == "node" || key_str == "import") {
-        if (!env->options()->experimental_conditional_exports) continue;
         matched = true;
         conditionalTarget = target_obj->Get(context, key).ToLocalChecked();
         Maybe<URL> resolved = ResolveExportsTarget(env, pjson_url,

src/node_options.cc

@@ -325,10 +325,9 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::userland_loader,
             kAllowedInEnvironment);
   AddAlias("--loader", "--experimental-loader");
-  AddAlias("--experimental-modules", "--experimental-conditional-exports");
-  AddOption("--experimental-conditional-exports",
-            "experimental support for conditional exports targets",
-            &EnvironmentOptions::experimental_conditional_exports,
+  AddOption("--experimental-modules",
+            "experimental modules features",
+            &EnvironmentOptions::experimental_modules,
             kAllowedInEnvironment);
   AddOption("--experimental-wasm-modules",
             "experimental ES Module support for webassembly modules",

src/node_options.h

@@ -101,8 +101,8 @@ class EnvironmentOptions : public Options {
  public:
   bool abort_on_uncaught_exception = false;
   bool enable_source_maps = false;
-  bool experimental_conditional_exports = false;
   bool experimental_json_modules = false;
+  bool experimental_modules = false;
   std::string experimental_specifier_resolution;
   std::string es_module_specifier_resolution;
   bool experimental_wasm_modules = false;

test/es-module/test-esm-exports.mjs

@@ -1,8 +1,6 @@
 // Flags: --experimental-modules
 import { mustCall } from '../common/index.mjs';
-import { path } from '../common/fixtures.mjs';
 import { ok, deepStrictEqual, strictEqual } from 'assert';
-import { spawn } from 'child_process';
 
 import { requireFixture, importFixture } from '../fixtures/pkgexports.mjs';
 import fromInside from '../fixtures/node_modules/pkgexports/lib/hole.js';
@@ -168,28 +166,3 @@ function assertIncludes(actual, expected) {
   ok(actual.toString().indexOf(expected) !== -1,
      `${JSON.stringify(actual)} includes ${JSON.stringify(expected)}`);
 }
-
-// Test warning message
-[
-  [
-    '--experimental-conditional-exports',
-    '/es-modules/conditional-exports.js',
-    'Conditional exports',
-  ]
-].forEach(([flag, file, message]) => {
-  const child = spawn(process.execPath, [flag, path(file)]);
-
-  let stderr = '';
-  child.stderr.setEncoding('utf8');
-  child.stderr.on('data', (data) => {
-    stderr += data;
-  });
-  child.on('close', (code, signal) => {
-    strictEqual(code, 0);
-    strictEqual(signal, null);
-    ok(stderr.toString().includes(
-      `ExperimentalWarning: ${message} is an experimental feature. ` +
-      'This feature could change at any time'
-    ));
-  });
-});