Skip to content

Commit 58d6871

Browse files
joyeecheungruyadorno
authored andcommitted
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: #55085 Backport-PR-URL: #55217 Refs: #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>
1 parent 9ffd2dd commit 58d6871

24 files changed

+131
-71
lines changed

doc/api/cli.md

+21-3
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,10 @@ following permissions are restricted:
10541054

10551055
<!-- YAML
10561056
added: v22.0.0
1057+
changes:
1058+
- version: REPLACEME
1059+
pr-url: https://github.com/nodejs/node/pull/55085
1060+
description: This is now true by default.
10571061
-->
10581062

10591063
> Stability: 1.1 - Active Development
@@ -1695,6 +1699,22 @@ added: v16.6.0
16951699

16961700
Use this flag to disable top-level await in REPL.
16971701

1702+
### `--no-experimental-require-module`
1703+
1704+
<!-- YAML
1705+
added: v22.0.0
1706+
changes:
1707+
- version: REPLACEME
1708+
pr-url: https://github.com/nodejs/node/pull/55085
1709+
description: This is now false by default.
1710+
-->
1711+
1712+
> Stability: 1.1 - Active Development
1713+
1714+
Disable support for loading a synchronous ES module graph in `require()`.
1715+
1716+
See [Loading ECMAScript modules using `require()`][].
1717+
16981718
### `--no-experimental-websocket`
16991719

17001720
<!-- YAML
@@ -1900,9 +1920,7 @@ Identical to `-e` but prints the result.
19001920
added: v22.0.0
19011921
-->
19021922

1903-
This flag is only useful when `--experimental-require-module` is enabled.
1904-
1905-
If the ES module being `require()`'d contains top-level await, this flag
1923+
If the ES module being `require()`'d contains top-level `await`, this flag
19061924
allows Node.js to evaluate the module, try to locate the
19071925
top-level awaits, and print their location to help users find them.
19081926

doc/api/errors.md

+16-7
Original file line numberDiff line numberDiff line change
@@ -2478,8 +2478,8 @@ Opening a QUIC stream failed.
24782478

24792479
> Stability: 1 - Experimental
24802480
2481-
When trying to `require()` a [ES Module][] under `--experimental-require-module`,
2482-
a CommonJS to ESM or ESM to CommonJS edge participates in an immediate cycle.
2481+
When trying to `require()` a [ES Module][], a CommonJS to ESM or ESM to CommonJS edge
2482+
participates in an immediate cycle.
24832483
This is not allowed because ES Modules cannot be evaluated while they are
24842484
already being evaluated.
24852485

@@ -2493,8 +2493,8 @@ module, and should be done lazily in an inner function.
24932493

24942494
> Stability: 1 - Experimental
24952495
2496-
When trying to `require()` a [ES Module][] under `--experimental-require-module`,
2497-
the module turns out to be asynchronous. That is, it contains top-level await.
2496+
When trying to `require()` a [ES Module][], the module turns out to be asynchronous.
2497+
That is, it contains top-level await.
24982498

24992499
To see where the top-level await is, use
25002500
`--experimental-print-required-tla` (this would execute the modules
@@ -2504,12 +2504,20 @@ before looking for the top-level awaits).
25042504

25052505
### `ERR_REQUIRE_ESM`
25062506

2507-
> Stability: 1 - Experimental
2507+
<!-- YAML
2508+
changes:
2509+
- version: REPLACEME
2510+
pr-url: https://github.com/nodejs/node/pull/55085
2511+
description: require() now supports loading synchronous ES modules by default.
2512+
-->
2513+
2514+
> Stability: 0 - Deprecated
25082515
25092516
An attempt was made to `require()` an [ES Module][].
25102517

2511-
To enable `require()` for synchronous module graphs (without
2512-
top-level `await`), use `--experimental-require-module`.
2518+
This error has been deprecated since `require()` now supports loading synchronous
2519+
ES modules. When `require()` encounters an ES module that contains top-level
2520+
`await`, it will throw [`ERR_REQUIRE_ASYNC_MODULE`][] instead.
25132521

25142522
<a id="ERR_SCRIPT_EXECUTION_INTERRUPTED"></a>
25152523

@@ -4123,6 +4131,7 @@ An error occurred trying to allocate memory. This should never happen.
41234131
[`ERR_INVALID_ARG_TYPE`]: #err_invalid_arg_type
41244132
[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: #err_missing_message_port_in_transfer_list
41254133
[`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`]: #err_missing_transferable_in_transfer_list
4134+
[`ERR_REQUIRE_ASYNC_MODULE`]: #err_require_async_module
41264135
[`EventEmitter`]: events.md#class-eventemitter
41274136
[`MessagePort`]: worker_threads.md#class-messageport
41284137
[`Object.getPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getPrototypeOf

doc/api/esm.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ compatibility.
468468
### `require`
469469
470470
The CommonJS module `require` currently only supports loading synchronous ES
471-
modules when `--experimental-require-module` is enabled.
471+
modules (that is, ES modules that do not use top-level `await`).
472472
473473
See [Loading ECMAScript modules using `require()`][] for details.
474474

doc/api/modules.md

+16-13
Original file line numberDiff line numberDiff line change
@@ -172,20 +172,20 @@ relative, and based on the real path of the files making the calls to
172172

173173
<!-- YAML
174174
added: v22.0.0
175+
changes:
176+
- version: REPLACEME
177+
pr-url: https://github.com/nodejs/node/pull/55085
178+
description: require() now supports loading synchronous ES modules by default.
175179
-->
176180

177181
> Stability: 1.1 - Active Development. Enable this API with the
178182
> [`--experimental-require-module`][] CLI flag.
179183
180184
The `.mjs` extension is reserved for [ECMAScript Modules][].
181-
Currently, if the flag `--experimental-require-module` is not used, loading
182-
an ECMAScript module using `require()` will throw a [`ERR_REQUIRE_ESM`][]
183-
error, and users need to use [`import()`][] instead. See
184-
[Determining module system][] section for more info
185+
See [Determining module system][] section for more info
185186
regarding which files are parsed as ECMAScript modules.
186187

187-
If `--experimental-require-module` is enabled, and the ECMAScript module being
188-
loaded by `require()` meets the following requirements:
188+
`require()` only supports loading ECMAScript modules that meet the following requirements:
189189

190190
* The module is fully synchronous (contains no top-level `await`); and
191191
* One of these conditions are met:
@@ -194,8 +194,8 @@ loaded by `require()` meets the following requirements:
194194
3. The file has a `.js` extension, the closest `package.json` does not contain
195195
`"type": "commonjs"`, and the module contains ES module syntax.
196196

197-
`require()` will load the requested module as an ES Module, and return
198-
the module namespace object. In this case it is similar to dynamic
197+
If the ES Module being loaded meet the requirements, `require()` can load it and
198+
return the module namespace object. In this case it is similar to dynamic
199199
`import()` but is run synchronously and returns the name space object
200200
directly.
201201

@@ -214,7 +214,7 @@ class Point {
214214
export default Point;
215215
```
216216

217-
A CommonJS module can load them with `require()` under `--experimental-detect-module`:
217+
A CommonJS module can load them with `require()`:
218218

219219
```cjs
220220
const distance = require('./distance.mjs');
@@ -243,13 +243,18 @@ conventions. Code authored directly in CommonJS should avoid depending on it.
243243
If the module being `require()`'d contains top-level `await`, or the module
244244
graph it `import`s contains top-level `await`,
245245
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
246-
load the asynchronous module using `import()`.
246+
load the asynchronous module using [`import()`][].
247247

248248
If `--experimental-print-required-tla` is enabled, instead of throwing
249249
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
250250
module, try to locate the top-level awaits, and print their location to
251251
help users fix them.
252252

253+
Support for loading ES modules using `require()` is currently
254+
experimental and can be disabled using `--no-experimental-require-module`.
255+
When `require()` actually encounters an ES module for the
256+
first time in the process, it will emit an experimental warning. The
257+
warning is expected to be removed when this feature stablizes.
253258
This feature can be detected by checking if
254259
[`process.features.require_module`][] is `true`.
255260

@@ -282,8 +287,7 @@ require(X) from module at path Y
282287
283288
MAYBE_DETECT_AND_LOAD(X)
284289
1. If X parses as a CommonJS module, load X as a CommonJS module. STOP.
285-
2. Else, if `--experimental-require-module` is
286-
enabled, and the source code of X can be parsed as ECMAScript module using
290+
2. Else, if the source code of X can be parsed as ECMAScript module using
287291
<a href="esm.md#resolver-algorithm-specification">DETECT_MODULE_SYNTAX defined in
288292
the ESM resolver</a>,
289293
a. Load X as an ECMAScript module. STOP.
@@ -1201,7 +1205,6 @@ This section was moved to
12011205
[`"type"`]: packages.md#type
12021206
[`--experimental-require-module`]: cli.md#--experimental-require-module
12031207
[`ERR_REQUIRE_ASYNC_MODULE`]: errors.md#err_require_async_module
1204-
[`ERR_REQUIRE_ESM`]: errors.md#err_require_esm
12051208
[`ERR_UNSUPPORTED_DIR_IMPORT`]: errors.md#err_unsupported_dir_import
12061209
[`MODULE_NOT_FOUND`]: errors.md#module_not_found
12071210
[`__dirname`]: #__dirname

doc/api/packages.md

+2-4
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,7 @@ There is the CommonJS module loader:
172172
* It treats all files that lack `.json` or `.node` extensions as JavaScript
173173
text files.
174174
* It can only be used to [load ECMASCript modules from CommonJS modules][] if
175-
the module graph is synchronous (that contains no top-level `await`) when
176-
`--experimental-require-module` is enabled.
175+
the module graph is synchronous (that contains no top-level `await`).
177176
When used to load a JavaScript text file that is not an ECMAScript module,
178177
the file will be loaded as a CommonJS module.
179178

@@ -662,8 +661,7 @@ specific to least specific as conditions should be defined:
662661
* `"require"` - matches when the package is loaded via `require()`. The
663662
referenced file should be loadable with `require()` although the condition
664663
matches regardless of the module format of the target file. Expected
665-
formats include CommonJS, JSON, native addons, and ES modules
666-
if `--experimental-require-module` is enabled. _Always mutually
664+
formats include CommonJS, JSON, native addons, and ES modules. _Always mutually
667665
exclusive with `"import"`._
668666
* `"module-sync"` - matches no matter the package is loaded via `import`,
669667
`import()` or `require()`. The format is expected to be ES modules that does

lib/internal/modules/cjs/loader.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,6 @@ function initializeCJS() {
438438
Module._extensions['.ts'] = loadTS;
439439
}
440440
if (getOptionValue('--experimental-require-module')) {
441-
emitExperimentalWarning('Support for loading ES Module in require()');
442441
Module._extensions['.mjs'] = loadESMFromCJS;
443442
if (tsEnabled) {
444443
Module._extensions['.mts'] = loadESMFromCJS;
@@ -1375,6 +1374,7 @@ function loadESMFromCJS(mod, filename) {
13751374
// ESM won't be accessible via process.mainModule.
13761375
setOwnProperty(process, 'mainModule', undefined);
13771376
} else {
1377+
emitExperimentalWarning('Support for loading ES Module in require()');
13781378
const {
13791379
wrap,
13801380
namespace,

lib/internal/modules/esm/load.js

-10
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,6 @@ async function defaultLoad(url, context = kEmptyObject) {
131131

132132
validateAttributes(url, format, importAttributes);
133133

134-
// Use the synchronous commonjs translator which can deal with cycles.
135-
if (format === 'commonjs' && getOptionValue('--experimental-require-module')) {
136-
format = 'commonjs-sync';
137-
}
138-
139134
if (getOptionValue('--experimental-strip-types') &&
140135
(format === 'module-typescript' || format === 'commonjs-typescript') &&
141136
isUnderNodeModules(url)) {
@@ -191,11 +186,6 @@ function defaultLoadSync(url, context = kEmptyObject) {
191186

192187
validateAttributes(url, format, importAttributes);
193188

194-
// Use the synchronous commonjs translator which can deal with cycles.
195-
if (format === 'commonjs' && getOptionValue('--experimental-require-module')) {
196-
format = 'commonjs-sync';
197-
}
198-
199189
return {
200190
__proto__: null,
201191
format,

lib/internal/modules/esm/loader.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -373,15 +373,15 @@ class ModuleLoader {
373373

374374
defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync;
375375
const loadResult = defaultLoadSync(url, { format, importAttributes });
376-
const {
377-
format: finalFormat,
378-
source,
379-
} = loadResult;
376+
377+
// Use the synchronous commonjs translator which can deal with cycles.
378+
const finalFormat = loadResult.format === 'commonjs' ? 'commonjs-sync' : loadResult.format;
380379

381380
if (finalFormat === 'wasm') {
382381
assert.fail('WASM is currently unsupported by require(esm)');
383382
}
384383

384+
const { source } = loadResult;
385385
const isMain = (parentURL === undefined);
386386
const wrap = this.#translate(url, finalFormat, source, isMain);
387387
assert(wrap instanceof ModuleWrap, `Translator used for require(${url}) should not be async`);

lib/internal/modules/esm/module_job.js

+1
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ class ModuleJobSync extends ModuleJobBase {
361361
}
362362

363363
runSync() {
364+
// TODO(joyeecheung): add the error decoration logic from the async instantiate.
364365
this.module.instantiateSync();
365366
setHasStartedUserESMExecution();
366367
const namespace = this.module.evaluateSync();

lib/internal/modules/esm/utils.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,15 @@ function initializeDefaultConditions() {
7575
const userConditions = getOptionValue('--conditions');
7676
const noAddons = getOptionValue('--no-addons');
7777
const addonConditions = noAddons ? [] : ['node-addons'];
78-
78+
const moduleConditions = getOptionValue('--experimental-require-module') ? ['module-sync'] : [];
7979
defaultConditions = ObjectFreeze([
8080
'node',
8181
'import',
82+
...moduleConditions,
8283
...addonConditions,
8384
...userConditions,
8485
]);
8586
defaultConditionsSet = new SafeSet(defaultConditions);
86-
if (getOptionValue('--experimental-require-module')) {
87-
defaultConditionsSet.add('module-sync');
88-
}
8987
}
9088

9189
/**

src/node_options.cc

+3-2
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
374374
&EnvironmentOptions::print_required_tla,
375375
kAllowedInEnvvar);
376376
AddOption("--experimental-require-module",
377-
"Allow loading explicit ES Modules in require().",
377+
"Allow loading synchronous ES Modules in require().",
378378
&EnvironmentOptions::require_module,
379-
kAllowedInEnvvar);
379+
kAllowedInEnvvar,
380+
true);
380381
AddOption("--diagnostic-dir",
381382
"set dir for all output files"
382383
" (default: current working directory)",

src/node_options.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class EnvironmentOptions : public Options {
117117
std::vector<std::string> conditions;
118118
bool detect_module = true;
119119
bool print_required_tla = false;
120-
bool require_module = false;
120+
bool require_module = true;
121121
std::string dns_result_order;
122122
bool enable_source_maps = false;
123123
bool experimental_eventsource = false;

test/es-module/test-cjs-esm-warn.js

+9-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Previously, this tested that require(esm) throws ERR_REQUIRE_ESM, which is no longer applicable
2+
// since require(esm) is now supported. The test has been repurposed to ensure that the old behavior
3+
// is preserved when the --no-experimental-require-module flag is used.
14
'use strict';
25

36
const { spawnPromisified } = require('../common');
@@ -22,7 +25,9 @@ describe('CJS ↔︎ ESM interop warnings', { concurrency: !process.env.TEST_PAR
2225
fixtures.path('/es-modules/package-type-module/cjs.js')
2326
);
2427
const basename = 'cjs.js';
25-
const { code, signal, stderr } = await spawnPromisified(execPath, [requiringCjsAsEsm]);
28+
const { code, signal, stderr } = await spawnPromisified(execPath, [
29+
'--no-experimental-require-module', requiringCjsAsEsm,
30+
]);
2631

2732
assert.ok(
2833
stderr.replaceAll('\r', '').includes(
@@ -48,7 +53,9 @@ describe('CJS ↔︎ ESM interop warnings', { concurrency: !process.env.TEST_PAR
4853
fixtures.path('/es-modules/package-type-module/esm.js')
4954
);
5055
const basename = 'esm.js';
51-
const { code, signal, stderr } = await spawnPromisified(execPath, [requiringEsm]);
56+
const { code, signal, stderr } = await spawnPromisified(execPath, [
57+
'--no-experimental-require-module', requiringEsm,
58+
]);
5259

5360
assert.ok(
5461
stderr.replace(/\r/g, '').includes(

test/es-module/test-esm-detect-ambiguous.mjs

+4-3
Original file line numberDiff line numberDiff line change
@@ -402,15 +402,16 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL },
402402
});
403403
});
404404

405-
// Validate temporarily disabling `--abort-on-uncaught-exception`
406-
// while running `containsModuleSyntax`.
405+
// Checks the error caught during module detection does not trigger abort when
406+
// `--abort-on-uncaught-exception` is passed in (as that's a caught internal error).
407407
// Ref: https://github.com/nodejs/node/issues/50878
408408
describe('Wrapping a `require` of an ES module while using `--abort-on-uncaught-exception`', () => {
409409
it('should work', async () => {
410410
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
411411
'--abort-on-uncaught-exception',
412+
'--no-warnings',
412413
'--eval',
413-
'assert.throws(() => require("./package-type-module/esm.js"), { code: "ERR_REQUIRE_ESM" })',
414+
'require("./package-type-module/esm.js")',
414415
], {
415416
cwd: fixtures.path('es-modules'),
416417
});

test/es-module/test-esm-loader-hooks.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,7 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => {
774774

775775
describe('should use hooks', async () => {
776776
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
777+
'--no-experimental-require-module',
777778
'--import',
778779
fixtures.fileURL('es-module-loaders/builtin-named-exports.mjs'),
779780
fixtures.path('es-modules/require-esm-throws-with-loaders.js'),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Flags: --no-experimental-require-module
2+
// Previously, this tested that require(esm) throws ERR_REQUIRE_ESM, which is no longer applicable
3+
// since require(esm) is now supported. The test has been repurposed to ensure that the old behavior
4+
// is preserved when the --no-experimental-require-module flag is used.
5+
6+
'use strict';
7+
require('../common');
8+
const assert = require('assert');
9+
const { describe, it } = require('node:test');
10+
11+
describe('Errors related to ESM type field', () => {
12+
it('Should throw an error when loading CJS from a `type: "module"` package.', () => {
13+
assert.throws(() => require('../fixtures/es-modules/package-type-module/index.js'), {
14+
code: 'ERR_REQUIRE_ESM'
15+
});
16+
});
17+
});

0 commit comments

Comments
 (0)