module: unflag --experimental-require-module

This unflags --experimental-require-module so require(esm) can be
used without the flag. For now, when require() actually encounters
an ESM, it will still emit an experimental warning. To opt out
of the feature, --no-experimental-require-module can be used.

There are some tests specifically testing ERR_REQUIRE_ESM. Some
of them are repurposed to test --no-experimental-require-module.
Some of them are modified to just expect loading require(esm) to
work, when it's appropriate.

PR-URL: https://github.com/nodejs/node/pull/55085
Refs: https://github.com/nodejs/node/issues/52697
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
This commit is contained in:
Joyee Cheung 2024-09-26 16:21:37 +02:00 committed by GitHub
parent 17fd32790a
commit 06206af181
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
23 changed files with 135 additions and 68 deletions

View File

@ -1034,6 +1034,10 @@ following permissions are restricted:
added:
- v22.0.0
- v20.17.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/55085
description: This is now true by default.
-->
> Stability: 1.1 - Active Development
@ -1659,6 +1663,24 @@ added: v16.6.0
Use this flag to disable top-level await in REPL.
### `--no-experimental-require-module`
<!-- YAML
added:
- v22.0.0
- v20.17.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/55085
description: This is now false by default.
-->
> Stability: 1.1 - Active Development
Disable support for loading a synchronous ES module graph in `require()`.
See [Loading ECMAScript modules using `require()`][].
### `--no-experimental-websocket`
<!-- YAML
@ -1875,9 +1897,7 @@ added:
- v20.17.0
-->
This flag is only useful when `--experimental-require-module` is enabled.
If the ES module being `require()`'d contains top-level await, this flag
If the ES module being `require()`'d contains top-level `await`, this flag
allows Node.js to evaluate the module, try to locate the
top-level awaits, and print their location to help users find them.

View File

@ -2426,8 +2426,8 @@ object.
> Stability: 1 - Experimental
When trying to `require()` a [ES Module][] under `--experimental-require-module`,
a CommonJS to ESM or ESM to CommonJS edge participates in an immediate cycle.
When trying to `require()` a [ES Module][], a CommonJS to ESM or ESM to CommonJS edge
participates in an immediate cycle.
This is not allowed because ES Modules cannot be evaluated while they are
already being evaluated.
@ -2441,8 +2441,8 @@ module, and should be done lazily in an inner function.
> Stability: 1 - Experimental
When trying to `require()` a [ES Module][] under `--experimental-require-module`,
the module turns out to be asynchronous. That is, it contains top-level await.
When trying to `require()` a [ES Module][], the module turns out to be asynchronous.
That is, it contains top-level await.
To see where the top-level await is, use
`--experimental-print-required-tla` (this would execute the modules
@ -2452,12 +2452,20 @@ before looking for the top-level awaits).
### `ERR_REQUIRE_ESM`
> Stability: 1 - Experimental
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/55085
description: require() now supports loading synchronous ES modules by default.
-->
> Stability: 0 - Deprecated
An attempt was made to `require()` an [ES Module][].
To enable `require()` for synchronous module graphs (without
top-level `await`), use `--experimental-require-module`.
This error has been deprecated since `require()` now supports loading synchronous
ES modules. When `require()` encounters an ES module that contains top-level
`await`, it will throw [`ERR_REQUIRE_ASYNC_MODULE`][] instead.
<a id="ERR_SCRIPT_EXECUTION_INTERRUPTED"></a>
@ -4061,6 +4069,7 @@ Type stripping is not supported for files descendent of a `node_modules` directo
[`ERR_INVALID_ARG_TYPE`]: #err_invalid_arg_type
[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: #err_missing_message_port_in_transfer_list
[`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`]: #err_missing_transferable_in_transfer_list
[`ERR_REQUIRE_ASYNC_MODULE`]: #err_require_async_module
[`EventEmitter`]: events.md#class-eventemitter
[`MessagePort`]: worker_threads.md#class-messageport
[`Object.getPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getPrototypeOf

View File

@ -468,7 +468,7 @@ compatibility.
### `require`
The CommonJS module `require` currently only supports loading synchronous ES
modules when `--experimental-require-module` is enabled.
modules (that is, ES modules that do not use top-level `await`).
See [Loading ECMAScript modules using `require()`][] for details.

View File

@ -170,15 +170,18 @@ relative, and based on the real path of the files making the calls to
## Loading ECMAScript modules using `require()`
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/55085
description: require() now supports loading synchronous ES modules by default.
-->
The `.mjs` extension is reserved for [ECMAScript Modules][].
Currently, if the flag `--experimental-require-module` is not used, loading
an ECMAScript module using `require()` will throw a [`ERR_REQUIRE_ESM`][]
error, and users need to use [`import()`][] instead. See
[Determining module system][] section for more info
See [Determining module system][] section for more info
regarding which files are parsed as ECMAScript modules.
If `--experimental-require-module` is enabled, and the ECMAScript module being
loaded by `require()` meets the following requirements:
`require()` only supports loading ECMAScript modules that meet the following requirements:
* The module is fully synchronous (contains no top-level `await`); and
* One of these conditions are met:
@ -187,8 +190,8 @@ loaded by `require()` meets the following requirements:
3. The file has a `.js` extension, the closest `package.json` does not contain
`"type": "commonjs"`, and the module contains ES module syntax.
`require()` will load the requested module as an ES Module, and return
the module namespace object. In this case it is similar to dynamic
If the ES Module being loaded meet the requirements, `require()` can load it and
return the module namespace object. In this case it is similar to dynamic
`import()` but is run synchronously and returns the name space object
directly.
@ -207,7 +210,7 @@ class Point {
export default Point;
```
A CommonJS module can load them with `require()` under `--experimental-detect-module`:
A CommonJS module can load them with `require()`:
```cjs
const distance = require('./distance.mjs');
@ -236,13 +239,19 @@ conventions. Code authored directly in CommonJS should avoid depending on it.
If the module being `require()`'d contains top-level `await`, or the module
graph it `import`s contains top-level `await`,
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
load the asynchronous module using `import()`.
load the asynchronous module using [`import()`][].
If `--experimental-print-required-tla` is enabled, instead of throwing
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
module, try to locate the top-level awaits, and print their location to
help users fix them.
Support for loading ES modules using `require()` is currently
experimental and can be disabled using `--no-experimental-require-module`.
When `require()` actually encounters an ES module for the
first time in the process, it will emit an experimental warning. The
warning is expected to be removed when this feature stablizes.
## All together
<!-- type=misc -->
@ -272,8 +281,7 @@ require(X) from module at path Y
MAYBE_DETECT_AND_LOAD(X)
1. If X parses as a CommonJS module, load X as a CommonJS module. STOP.
2. Else, if `--experimental-require-module` is
enabled, and the source code of X can be parsed as ECMAScript module using
2. Else, if the source code of X can be parsed as ECMAScript module using
<a href="esm.md#resolver-algorithm-specification">DETECT_MODULE_SYNTAX defined in
the ESM resolver</a>,
a. Load X as an ECMAScript module. STOP.
@ -1190,7 +1198,6 @@ This section was moved to
[`"main"`]: packages.md#main
[`"type"`]: packages.md#type
[`ERR_REQUIRE_ASYNC_MODULE`]: errors.md#err_require_async_module
[`ERR_REQUIRE_ESM`]: errors.md#err_require_esm
[`ERR_UNSUPPORTED_DIR_IMPORT`]: errors.md#err_unsupported_dir_import
[`MODULE_NOT_FOUND`]: errors.md#module_not_found
[`__dirname`]: #__dirname

View File

@ -172,8 +172,7 @@ There is the CommonJS module loader:
* It treats all files that lack `.json` or `.node` extensions as JavaScript
text files.
* It can only be used to [load ECMASCript modules from CommonJS modules][] if
the module graph is synchronous (that contains no top-level `await`) when
`--experimental-require-module` is enabled.
the module graph is synchronous (that contains no top-level `await`).
When used to load a JavaScript text file that is not an ECMAScript module,
the file will be loaded as a CommonJS module.
@ -662,8 +661,7 @@ specific to least specific as conditions should be defined:
* `"require"` - matches when the package is loaded via `require()`. The
referenced file should be loadable with `require()` although the condition
matches regardless of the module format of the target file. Expected
formats include CommonJS, JSON, native addons, and ES modules
if `--experimental-require-module` is enabled. _Always mutually
formats include CommonJS, JSON, native addons, and ES modules. _Always mutually
exclusive with `"import"`._
* `"module-sync"` - matches no matter the package is loaded via `import`,
`import()` or `require()`. The format is expected to be ES modules that does

View File

@ -438,7 +438,6 @@ function initializeCJS() {
Module._extensions['.ts'] = loadTS;
}
if (getOptionValue('--experimental-require-module')) {
emitExperimentalWarning('Support for loading ES Module in require()');
Module._extensions['.mjs'] = loadESMFromCJS;
if (tsEnabled) {
Module._extensions['.mts'] = loadESMFromCJS;
@ -1386,6 +1385,7 @@ function loadESMFromCJS(mod, filename) {
// ESM won't be accessible via process.mainModule.
setOwnProperty(process, 'mainModule', undefined);
} else {
emitExperimentalWarning('Support for loading ES Module in require()');
const {
wrap,
namespace,

View File

@ -131,11 +131,6 @@ async function defaultLoad(url, context = kEmptyObject) {
validateAttributes(url, format, importAttributes);
// Use the synchronous commonjs translator which can deal with cycles.
if (format === 'commonjs' && getOptionValue('--experimental-require-module')) {
format = 'commonjs-sync';
}
if (getOptionValue('--experimental-strip-types') &&
(format === 'module-typescript' || format === 'commonjs-typescript') &&
isUnderNodeModules(url)) {
@ -191,11 +186,6 @@ function defaultLoadSync(url, context = kEmptyObject) {
validateAttributes(url, format, importAttributes);
// Use the synchronous commonjs translator which can deal with cycles.
if (format === 'commonjs' && getOptionValue('--experimental-require-module')) {
format = 'commonjs-sync';
}
return {
__proto__: null,
format,

View File

@ -373,15 +373,15 @@ class ModuleLoader {
defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync;
const loadResult = defaultLoadSync(url, { format, importAttributes });
const {
format: finalFormat,
source,
} = loadResult;
// Use the synchronous commonjs translator which can deal with cycles.
const finalFormat = loadResult.format === 'commonjs' ? 'commonjs-sync' : loadResult.format;
if (finalFormat === 'wasm') {
assert.fail('WASM is currently unsupported by require(esm)');
}
const { source } = loadResult;
const isMain = (parentURL === undefined);
const wrap = this.#translate(url, finalFormat, source, isMain);
assert(wrap instanceof ModuleWrap, `Translator used for require(${url}) should not be async`);

View File

@ -361,6 +361,7 @@ class ModuleJobSync extends ModuleJobBase {
}
runSync() {
// TODO(joyeecheung): add the error decoration logic from the async instantiate.
this.module.instantiateSync();
setHasStartedUserESMExecution();
const namespace = this.module.evaluateSync();

View File

@ -75,17 +75,15 @@ function initializeDefaultConditions() {
const userConditions = getOptionValue('--conditions');
const noAddons = getOptionValue('--no-addons');
const addonConditions = noAddons ? [] : ['node-addons'];
const moduleConditions = getOptionValue('--experimental-require-module') ? ['module-sync'] : [];
defaultConditions = ObjectFreeze([
'node',
'import',
...moduleConditions,
...addonConditions,
...userConditions,
]);
defaultConditionsSet = new SafeSet(defaultConditions);
if (getOptionValue('--experimental-require-module')) {
defaultConditionsSet.add('module-sync');
}
}
/**

View File

@ -374,9 +374,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
&EnvironmentOptions::print_required_tla,
kAllowedInEnvvar);
AddOption("--experimental-require-module",
"Allow loading explicit ES Modules in require().",
"Allow loading synchronous ES Modules in require().",
&EnvironmentOptions::require_module,
kAllowedInEnvvar);
kAllowedInEnvvar,
true);
AddOption("--diagnostic-dir",
"set dir for all output files"
" (default: current working directory)",

View File

@ -117,7 +117,7 @@ class EnvironmentOptions : public Options {
std::vector<std::string> conditions;
bool detect_module = true;
bool print_required_tla = false;
bool require_module = false;
bool require_module = true;
std::string dns_result_order;
bool enable_source_maps = false;
bool experimental_eventsource = false;

View File

@ -1,3 +1,6 @@
// Previously, this tested that require(esm) throws ERR_REQUIRE_ESM, which is no longer applicable
// since require(esm) is now supported. The test has been repurposed to ensure that the old behavior
// is preserved when the --no-experimental-require-module flag is used.
'use strict';
const { spawnPromisified } = require('../common');
@ -22,7 +25,9 @@ describe('CJS ↔︎ ESM interop warnings', { concurrency: !process.env.TEST_PAR
fixtures.path('/es-modules/package-type-module/cjs.js')
);
const basename = 'cjs.js';
const { code, signal, stderr } = await spawnPromisified(execPath, [requiringCjsAsEsm]);
const { code, signal, stderr } = await spawnPromisified(execPath, [
'--no-experimental-require-module', requiringCjsAsEsm,
]);
assert.ok(
stderr.replaceAll('\r', '').includes(
@ -48,7 +53,9 @@ describe('CJS ↔︎ ESM interop warnings', { concurrency: !process.env.TEST_PAR
fixtures.path('/es-modules/package-type-module/esm.js')
);
const basename = 'esm.js';
const { code, signal, stderr } = await spawnPromisified(execPath, [requiringEsm]);
const { code, signal, stderr } = await spawnPromisified(execPath, [
'--no-experimental-require-module', requiringEsm,
]);
assert.ok(
stderr.replace(/\r/g, '').includes(

View File

@ -402,15 +402,16 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL },
});
});
// Validate temporarily disabling `--abort-on-uncaught-exception`
// while running `containsModuleSyntax`.
// Checks the error caught during module detection does not trigger abort when
// `--abort-on-uncaught-exception` is passed in (as that's a caught internal error).
// Ref: https://github.com/nodejs/node/issues/50878
describe('Wrapping a `require` of an ES module while using `--abort-on-uncaught-exception`', () => {
it('should work', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--abort-on-uncaught-exception',
'--no-warnings',
'--eval',
'assert.throws(() => require("./package-type-module/esm.js"), { code: "ERR_REQUIRE_ESM" })',
'require("./package-type-module/esm.js")',
], {
cwd: fixtures.path('es-modules'),
});

View File

@ -774,6 +774,7 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => {
describe('should use hooks', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--no-experimental-require-module',
'--import',
fixtures.fileURL('es-module-loaders/builtin-named-exports.mjs'),
fixtures.path('es-modules/require-esm-throws-with-loaders.js'),

View File

@ -0,0 +1,17 @@
// Flags: --no-experimental-require-module
// Previously, this tested that require(esm) throws ERR_REQUIRE_ESM, which is no longer applicable
// since require(esm) is now supported. The test has been repurposed to ensure that the old behavior
// is preserved when the --no-experimental-require-module flag is used.
'use strict';
require('../common');
const assert = require('assert');
const { describe, it } = require('node:test');
describe('Errors related to ESM type field', () => {
it('Should throw an error when loading CJS from a `type: "module"` package.', () => {
assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), {
code: 'ERR_REQUIRE_ESM'
});
});
});

View File

@ -50,12 +50,6 @@ describe('ESM type field errors', { concurrency: true }, () => {
true,
);
});
it('--input-type=module disallowed for directories', () => {
assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), {
code: 'ERR_REQUIRE_ESM'
});
});
});
function expect(opt = '', inputFile, want, wantsError = false) {

View File

@ -3,9 +3,12 @@
require('../common');
const { spawnSyncAndAssert } = require('../common/child_process');
const { fixturesDir } = require('../common/fixtures');
const stderr = /ExperimentalWarning: Support for loading ES Module in require/;
const warningRE = /ExperimentalWarning: Support for loading ES Module in require/;
function testPreload(preloadFlag) {
// The warning is only emitted when ESM is loaded by --require.
const stderr = preloadFlag !== '--import' ? warningRE : undefined;
// Test named exports.
{
spawnSyncAndAssert(
@ -112,7 +115,7 @@ testPreload('--import');
},
{
stdout: /^package-type-module\s+A$/,
stderr,
stderr: warningRE,
trim: true,
}
);

View File

@ -100,6 +100,7 @@ test('execute a .cts file importing a .ts file export', async () => {
test('execute a .cts file importing a .mts file export', async () => {
const result = await spawnPromisified(process.execPath, [
'--experimental-strip-types',
'--no-experimental-require-module',
fixtures.path('typescript/cts/test-require-mts-module.cts'),
]);
@ -158,6 +159,7 @@ test('expect failure of a .ts file in node_modules', async () => {
test('expect failure of a .cts requiring esm without default type module', async () => {
const result = await spawnPromisified(process.execPath, [
'--experimental-strip-types',
'--no-experimental-require-module',
fixtures.path('typescript/cts/test-mts-node_modules.cts'),
]);

View File

@ -67,6 +67,7 @@ test('execute an .mts file importing a .cts file', async () => {
test('execute an .mts file with wrong default module', async () => {
const result = await spawnPromisified(process.execPath, [
'--experimental-strip-types',
'--no-experimental-require-module',
'--experimental-default-type=commonjs',
fixtures.path('typescript/mts/test-import-module.mts'),
]);
@ -76,6 +77,18 @@ test('execute an .mts file with wrong default module', async () => {
strictEqual(result.code, 1);
});
test('execute an .mts file with wrong default module', async () => {
const result = await spawnPromisified(process.execPath, [
'--experimental-strip-types',
'--experimental-require-module',
'--experimental-default-type=commonjs',
fixtures.path('typescript/mts/test-import-module.mts'),
]);
match(result.stdout, /Hello, TypeScript!/);
strictEqual(result.code, 0);
});
test('execute an .mts file from node_modules', async () => {
const result = await spawnPromisified(process.execPath, [
'--experimental-strip-types',

View File

@ -32,11 +32,11 @@ test('execute a TypeScript file with imports', async () => {
const result = await spawnPromisified(process.execPath, [
'--no-warnings',
'--eval',
`assert.throws(() => require(${JSON.stringify(fixtures.path('typescript/ts/test-import-fs.ts'))}), {code: 'ERR_REQUIRE_ESM'})`,
`require(${JSON.stringify(fixtures.path('typescript/ts/test-import-fs.ts'))})`,
]);
strictEqual(result.stderr, '');
strictEqual(result.stdout, '');
match(result.stdout, /Hello, TypeScript!/);
strictEqual(result.code, 0);
});
@ -283,9 +283,8 @@ test('execute a TypeScript file with CommonJS syntax requiring .mts', async () =
fixtures.path('typescript/ts/test-require-mts.ts'),
]);
strictEqual(result.stdout, '');
match(result.stderr, /Error \[ERR_REQUIRE_ESM\]: require\(\) of ES Module/);
strictEqual(result.code, 1);
match(result.stdout, /Hello, TypeScript!/);
strictEqual(result.code, 0);
});
test('execute a TypeScript file with CommonJS syntax requiring .mts with require-module', async () => {

View File

@ -5,6 +5,7 @@ export async function resolve(specifier, context, defaultResolve) {
deepStrictEqual([...context.conditions].sort(), [
'import',
'module-sync',
'node',
'node-addons',
]);

View File

@ -1,3 +1,8 @@
// Flags: --no-experimental-require-module
// Previously, this tested that require(esm) throws ERR_REQUIRE_ESM, which is no longer applicable
// since require(esm) is now supported. The test has been repurposed to ensure that the old behavior
// is preserved when the --no-experimental-require-module flag is used.
'use strict';
require('../common');
const assert = require('assert');