Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v12.x] Backport unflag --experimental-modules #33055

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ Specify the `module` of a custom [experimental ECMAScript Module loader][].
added: v8.5.0
-->

Enable experimental ES module support and caching modules.
Enable latest experimental modules features (deprecated).

### `--experimental-policy`
<!-- YAML
Expand Down Expand Up @@ -376,9 +376,8 @@ Specify ICU data load path. (Overrides `NODE_ICU_DATA`.)
added: v12.0.0
-->

Used with `--experimental-modules`, this configures Node.js to interpret string
input as CommonJS or as an ES module. String input is input via `--eval`,
`--print`, or `STDIN`.
This configures Node.js to interpret string input as CommonJS or as an ES
module. String input is input via `--eval`, `--print`, or `STDIN`.

Valid values are `"commonjs"` and `"module"`. The default is `"commonjs"`.

Expand Down
36 changes: 15 additions & 21 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,9 @@ specifier resolution, and default behavior.

<!-- type=misc -->

The `--experimental-modules` flag can be used to enable support for
ECMAScript modules (ES modules).

Once enabled, Node.js will treat the following as ES modules when passed to
`node` as the initial input, or when referenced by `import` statements within
ES module code:
Experimental support for ECMAScript modules is enabled by default.
Node.js will treat the following as ES modules when passed to `node` as the
initial input, or when referenced by `import` statements within ES module code:

* Files ending in `.mjs`.

Expand Down Expand Up @@ -78,7 +75,7 @@ until the root of the volume is reached.

```sh
# In same folder as above package.json
node --experimental-modules my-app.js # Runs as ES module
node my-app.js # Runs as ES module
```

If the nearest parent `package.json` lacks a `"type"` field, or contains
Expand Down Expand Up @@ -113,9 +110,8 @@ project’s `node_modules` folder contains its own `package.json` file, so each
project’s dependencies have their own package scopes. A `package.json` lacking a
`"type"` field is treated as if it contained `"type": "commonjs"`.

The package scope applies not only to initial entry points (`node
--experimental-modules my-app.js`) but also to files referenced by `import`
statements and `import()` expressions.
The package scope applies not only to initial entry points (`node my-app.js`)
but also to files referenced by `import` statements and `import()` expressions.

```js
// my-app.js, in an ES module package scope because there is a package.json
Expand Down Expand Up @@ -168,11 +164,9 @@ piped to `node` via `STDIN`, will be treated as ES modules when the
`--input-type=module` flag is set.

```sh
node --experimental-modules --input-type=module --eval \
"import { sep } from 'path'; console.log(sep);"
node --input-type=module --eval "import { sep } from 'path'; console.log(sep);"

echo "import { sep } from 'path'; console.log(sep);" | \
node --experimental-modules --input-type=module
echo "import { sep } from 'path'; console.log(sep);" | node --input-type=module
```

For completeness there is also `--input-type=commonjs`, for explicitly running
Expand Down Expand Up @@ -1004,8 +998,8 @@ The `--experimental-json-modules` flag is needed for the module
to work.

```bash
node --experimental-modules index.mjs # fails
node --experimental-modules --experimental-json-modules index.mjs # works
node index.mjs # fails
node --experimental-json-modules index.mjs # works
```

## Experimental Wasm Modules
Expand All @@ -1027,7 +1021,7 @@ console.log(M);
executed under:

```bash
node --experimental-modules --experimental-wasm-modules index.mjs
node --experimental-wasm-modules index.mjs
```

would provide the exports interface for the instantiation of `module.wasm`.
Expand Down Expand Up @@ -1173,7 +1167,7 @@ export async function getSource(url, context, defaultGetSource) {
#### <code>transformSource</code> hook

```console
NODE_OPTIONS='--experimental-modules --experimental-loader ./custom-loader.mjs' node x.js
NODE_OPTIONS='--experimental-loader ./custom-loader.mjs' node x.js
```

> Note: The loaders API is being redesigned. This hook may disappear or its
Expand Down Expand Up @@ -1733,11 +1727,11 @@ automatic extension resolution and importing from directories that include an
index file use the `node` mode.

```bash
$ node --experimental-modules index.mjs
$ node index.mjs
success!
$ node --experimental-modules index #Failure!
$ node index # Failure!
Error: Cannot find module
$ node --experimental-modules --experimental-specifier-resolution=node index
$ node --experimental-specifier-resolution=node index
success!
```

Expand Down
16 changes: 8 additions & 8 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ changes:
* `importModuleDynamically` {Function} Called during evaluation of this module
when `import()` is called. If this option is not specified, calls to
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
This option is part of the experimental API for the `--experimental-modules`
flag, and should not be considered stable.
This option is part of the experimental modules API, and should not be
considered stable.
* `specifier` {string} specifier passed to `import()`
* `module` {vm.Module}
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
Expand Down Expand Up @@ -876,8 +876,8 @@ changes:
* `importModuleDynamically` {Function} Called during evaluation of this module
when `import()` is called. If this option is not specified, calls to
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
This option is part of the experimental API for the `--experimental-modules`
flag, and should not be considered stable.
This option is part of the experimental modules API, and should not be
considered stable.
* `specifier` {string} specifier passed to `import()`
* `module` {vm.Module}
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
Expand Down Expand Up @@ -972,8 +972,8 @@ changes:
* `importModuleDynamically` {Function} Called during evaluation of this module
when `import()` is called. If this option is not specified, calls to
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
This option is part of the experimental API for the `--experimental-modules`
flag, and should not be considered stable.
This option is part of the experimental modules API, and should not be
considered stable.
* `specifier` {string} specifier passed to `import()`
* `module` {vm.Module}
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
Expand Down Expand Up @@ -1048,8 +1048,8 @@ changes:
* `importModuleDynamically` {Function} Called during evaluation of this module
when `import()` is called. If this option is not specified, calls to
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
This option is part of the experimental API for the `--experimental-modules`
flag, and should not be considered stable.
This option is part of the experimental modules API, and should not be
considered stable.
* `specifier` {string} specifier passed to `import()`
* `module` {vm.Module}
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
Expand Down
2 changes: 1 addition & 1 deletion doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ Specify the
to use as a custom module loader.
.
.It Fl -experimental-modules
Enable experimental ES module support and caching modules.
Enable experimental latest experimental modules features.
.
.It Fl -experimental-policy
Use the specified file as a security policy.
Expand Down
18 changes: 8 additions & 10 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,23 +210,21 @@ class NativeModule {
}

// Used by user-land module loaders to compile and load builtins.
compileForPublicLoader(needToSyncExports) {
compileForPublicLoader() {
if (!this.canBeRequiredByUsers) {
// No code because this is an assertion against bugs
// eslint-disable-next-line no-restricted-syntax
throw new Error(`Should not compile ${this.id} for public use`);
}
this.compileForInternalLoader();
if (needToSyncExports) {
if (!this.exportKeys) {
// When using --expose-internals, we do not want to reflect the named
// exports from core modules as this can trigger unnecessary getters.
const internal = this.id.startsWith('internal/');
this.exportKeys = internal ? [] : ObjectKeys(this.exports);
}
this.getESMFacade();
this.syncExports();
if (!this.exportKeys) {
// When using --expose-internals, we do not want to reflect the named
// exports from core modules as this can trigger unnecessary getters.
const internal = this.id.startsWith('internal/');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const internal = this.id.startsWith('internal/');
const internal = StringPrototypeStartsWith(this.id, 'internal/');

i realize this is a backport PR, so maybe it should land in master with this change first, but it seems important to avoid brittleness in LTS specifically.

this.exportKeys = internal ? [] : ObjectKeys(this.exports);
}
this.getESMFacade();
this.syncExports();
return this.exports;
}

Expand Down
27 changes: 9 additions & 18 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -408,24 +408,15 @@ function initializeESMLoader() {
// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
internalBinding('module_wrap').callbackMap = new SafeWeakMap();

const experimentalModules = getOptionValue('--experimental-modules');
const experimentalVMModules = getOptionValue('--experimental-vm-modules');
if (experimentalModules || experimentalVMModules) {
if (experimentalModules) {
process.emitWarning(
'The ESM module loader is experimental.',
'ExperimentalWarning', undefined);
}
const {
setImportModuleDynamicallyCallback,
setInitializeImportMetaObjectCallback
} = internalBinding('module_wrap');
const esm = require('internal/process/esm_loader');
// Setup per-isolate callbacks that locate data or callbacks that we keep
// track of for different ESM modules.
setInitializeImportMetaObjectCallback(esm.initializeImportMetaObject);
setImportModuleDynamicallyCallback(esm.importModuleDynamicallyCallback);
}
const {
setImportModuleDynamicallyCallback,
setInitializeImportMetaObjectCallback
} = internalBinding('module_wrap');
const esm = require('internal/process/esm_loader');
// Setup per-isolate callbacks that locate data or callbacks that we keep
// track of for different ESM modules.
setInitializeImportMetaObjectCallback(esm.initializeImportMetaObject);
setImportModuleDynamicallyCallback(esm.importModuleDynamicallyCallback);
}

function initializeFrozenIntrinsics() {
Expand Down
31 changes: 14 additions & 17 deletions lib/internal/main/check_syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,20 @@ if (process.argv[1] && process.argv[1] !== '-') {

function checkSyntax(source, filename) {
const { getOptionValue } = require('internal/options');
const experimentalModules = getOptionValue('--experimental-modules');
if (experimentalModules) {
let isModule = false;
if (filename === '[stdin]' || filename === '[eval]') {
isModule = getOptionValue('--input-type') === 'module';
} else {
const { defaultResolve } = require('internal/modules/esm/resolve');
const { defaultGetFormat } = require('internal/modules/esm/get_format');
const { url } = defaultResolve(pathToFileURL(filename).toString());
const { format } = defaultGetFormat(url);
isModule = format === 'module';
}
if (isModule) {
const { ModuleWrap } = internalBinding('module_wrap');
new ModuleWrap(filename, undefined, source, 0, 0);
return;
}
let isModule = false;
if (filename === '[stdin]' || filename === '[eval]') {
isModule = getOptionValue('--input-type') === 'module';
} else {
const { defaultResolve } = require('internal/modules/esm/resolve');
const { defaultGetFormat } = require('internal/modules/esm/get_format');
const { url } = defaultResolve(pathToFileURL(filename).toString());
const { format } = defaultGetFormat(url);
isModule = format === 'module';
}
if (isModule) {
const { ModuleWrap } = internalBinding('module_wrap');
new ModuleWrap(filename, undefined, source, 0, 0);
return;
}

wrapSafe(filename, source);
Expand Down
9 changes: 4 additions & 5 deletions lib/internal/main/run_main_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ prepareMainThreadExecution(true);

markBootstrapComplete();

// Note: this loads the module through the ESM loader if
// --experimental-loader is provided or --experimental-modules is on
// and the module is determined to be an ES module. This hangs from the CJS
// module loader because we currently allow monkey-patching of the module
// loaders in the preloaded scripts through require('module').
// Note: this loads the module through the ESM loader if the module is
// determined to be an ES module. This hangs from the CJS module loader
// because we currently allow monkey-patching of the module loaders
// in the preloaded scripts through require('module').
// runMain here might be monkey-patched by users in --require.
// XXX: the monkey-patchability here should probably be deprecated.
require('internal/modules/cjs/loader').Module.runMain(process.argv[1]);
11 changes: 3 additions & 8 deletions lib/internal/modules/cjs/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ const {
ERR_UNKNOWN_BUILTIN_MODULE
} = require('internal/errors').codes;
const { NativeModule } = require('internal/bootstrap/loaders');
const { getOptionValue } = require('internal/options');
const experimentalModules = getOptionValue('--experimental-modules');

const { validateString } = require('internal/validators');
const path = require('path');
Expand All @@ -19,11 +17,11 @@ const { URL } = require('url');

const debug = require('internal/util/debuglog').debuglog('module');

function loadNativeModule(filename, request, experimentalModules) {
function loadNativeModule(filename, request) {
const mod = NativeModule.map.get(filename);
if (mod) {
debug('load native module %s', request);
mod.compileForPublicLoader(experimentalModules);
mod.compileForPublicLoader();
return mod;
}
}
Expand All @@ -48,10 +46,7 @@ function makeRequireFunction(mod, redirects) {
const href = destination.href;
if (destination.protocol === 'node:') {
const specifier = destination.pathname;
const mod = loadNativeModule(
specifier,
href,
experimentalModules);
const mod = loadNativeModule(specifier, href);
if (mod && mod.canBeRequiredByUsers) {
return mod.exports;
}
Expand Down
Loading