Skip to content

Commit

Permalink
module: unflag conditional exports
Browse files Browse the repository at this point in the history
  • Loading branch information
guybedford authored and MylesBorins committed Jan 15, 2020
1 parent b4f5c9e commit 6a95d80
Show file tree
Hide file tree
Showing 8 changed files with 20 additions and 98 deletions.
14 changes: 1 addition & 13 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand Down
59 changes: 18 additions & 41 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 0 additions & 3 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -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.
.
Expand Down
7 changes: 0 additions & 7 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 1 addition & 5 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
27 changes: 0 additions & 27 deletions test/es-module/test-esm-exports.mjs
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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'
));
});
});

0 comments on commit 6a95d80

Please sign in to comment.