From 6a95d804beebbeba6b6920f4477381f7c61796d4 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Tue, 17 Dec 2019 00:58:30 -0500 Subject: [PATCH] module: unflag conditional exports --- doc/api/cli.md | 14 +------ doc/api/esm.md | 59 +++++++++-------------------- doc/node.1 | 3 -- lib/internal/modules/cjs/loader.js | 7 ---- src/module_wrap.cc | 1 - src/node_options.cc | 6 +-- src/node_options.h | 1 - test/es-module/test-esm-exports.mjs | 27 ------------- 8 files changed, 20 insertions(+), 98 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 673a3d5a75fbd6..4b283ceb167557 100644 --- a/doc/api/cli.md +++ b/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` - - -Enable experimental support for the `"require"` and `"node"` conditional -package export resolutions. -See [Conditional Exports][] for more information. - ### `--experimental-json-modules` -Enable latest experimental modules features (currently -`--experimental-conditional-exports`). +Enable latest experimental modules features (deprecated). ### `--experimental-policy` * `--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 diff --git a/doc/api/esm.md b/doc/api/esm.md index d48eb2ea9f7e19..e5271b5b343a84 100644 --- a/doc/api/esm.md +++ b/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`. ```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): ```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: ```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: ```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 diff --git a/doc/node.1 b/doc/node.1 index 96a2df9151cf91..9a75cd073eecdd 100644 --- a/doc/node.1 +++ b/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. . diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index a2d96442b9c76a..01d96a4a784397 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -72,8 +72,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; @@ -436,9 +434,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; @@ -611,8 +606,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, diff --git a/src/module_wrap.cc b/src/module_wrap.cc index f06b9d40c33c37..2368279d8e640a 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -1020,7 +1020,6 @@ Maybe 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 resolved = ResolveExportsTarget(env, pjson_url, diff --git a/src/node_options.cc b/src/node_options.cc index 8c4ecc4ec44d66..d05d9a36bf6d42 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -325,11 +325,7 @@ 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, - kAllowedInEnvironment); + AddAlias("--experimental-modules", {}); AddOption("--experimental-wasm-modules", "experimental ES Module support for webassembly modules", &EnvironmentOptions::experimental_wasm_modules, diff --git a/src/node_options.h b/src/node_options.h index 1e156a7edc56d1..9c0d65e2a1ee89 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -101,7 +101,6 @@ 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; std::string experimental_specifier_resolution; std::string es_module_specifier_resolution; diff --git a/test/es-module/test-esm-exports.mjs b/test/es-module/test-esm-exports.mjs index 96f33978968612..bdd4a975cf748e 100644 --- a/test/es-module/test-esm-exports.mjs +++ b/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' - )); - }); -});