Skip to content

Commit acea43e

Browse files
joyeecheungjoyeecheung
committed
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: nodejs#55085 Refs: nodejs#52697 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Richard Lau <[email protected]>
1 parent 2f65b3f commit acea43e

23 files changed

+138
-69
lines changed

‎doc/api/cli.md

+26-4
Original file line numberDiff line numberDiff line change
@@ -1053,7 +1053,13 @@ following permissions are restricted:
10531053
### `--experimental-require-module`
10541054

10551055
<!-- YAML
1056-
added: v22.0.0
1056+
added:
1057+
- v22.0.0
1058+
- v20.17.0
1059+
changes:
1060+
- version: REPLACEME
1061+
pr-url: https://github.com/nodejs/node/pull/55085
1062+
description: This is now true by default.
10571063
-->
10581064

10591065
> Stability: 1.1 - Active Development
@@ -1695,6 +1701,24 @@ added: v16.6.0
16951701

16961702
Use this flag to disable top-level await in REPL.
16971703

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

17001724
<!-- YAML
@@ -1900,9 +1924,7 @@ Identical to `-e` but prints the result.
19001924
added: v22.0.0
19011925
-->
19021926

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
1927+
If the ES module being `require()`'d contains top-level `await`, this flag
19061928
allows Node.js to evaluate the module, try to locate the
19071929
top-level awaits, and print their location to help users find them.
19081930

‎doc/api/errors.md

+16-7
Original file line numberDiff line numberDiff line change
@@ -2426,8 +2426,8 @@ object.
24262426

24272427
> Stability: 1 - Experimental
24282428
2429-
When trying to `require()` a [ES Module][] under `--experimental-require-module`,
2430-
a CommonJS to ESM or ESM to CommonJS edge participates in an immediate cycle.
2429+
When trying to `require()` a [ES Module][], a CommonJS to ESM or ESM to CommonJS edge
2430+
participates in an immediate cycle.
24312431
This is not allowed because ES Modules cannot be evaluated while they are
24322432
already being evaluated.
24332433

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

24422442
> Stability: 1 - Experimental
24432443
2444-
When trying to `require()` a [ES Module][] under `--experimental-require-module`,
2445-
the module turns out to be asynchronous. That is, it contains top-level await.
2444+
When trying to `require()` a [ES Module][], the module turns out to be asynchronous.
2445+
That is, it contains top-level await.
24462446

24472447
To see where the top-level await is, use
24482448
`--experimental-print-required-tla` (this would execute the modules
@@ -2452,12 +2452,20 @@ before looking for the top-level awaits).
24522452

24532453
### `ERR_REQUIRE_ESM`
24542454

2455-
> Stability: 1 - Experimental
2455+
<!-- YAML
2456+
changes:
2457+
- version: REPLACEME
2458+
pr-url: https://github.com/nodejs/node/pull/55085
2459+
description: require() now supports loading synchronous ES modules by default.
2460+
-->
2461+
2462+
> Stability: 0 - Deprecated
24562463
24572464
An attempt was made to `require()` an [ES Module][].
24582465

2459-
To enable `require()` for synchronous module graphs (without
2460-
top-level `await`), use `--experimental-require-module`.
2466+
This error has been deprecated since `require()` now supports loading synchronous
2467+
ES modules. When `require()` encounters an ES module that contains top-level
2468+
`await`, it will throw [`ERR_REQUIRE_ASYNC_MODULE`][] instead.
24612469

24622470
<a id="ERR_SCRIPT_EXECUTION_INTERRUPTED"></a>
24632471

@@ -4100,6 +4108,7 @@ Type stripping is not supported for files descendent of a `node_modules` directo
41004108
[`ERR_INVALID_ARG_TYPE`]: #err_invalid_arg_type
41014109
[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: #err_missing_message_port_in_transfer_list
41024110
[`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`]: #err_missing_transferable_in_transfer_list
4111+
[`ERR_REQUIRE_ASYNC_MODULE`]: #err_require_async_module
41034112
[`EventEmitter`]: events.md#class-eventemitter
41044113
[`MessagePort`]: worker_threads.md#class-messageport
41054114
[`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

+20-13
Original file line numberDiff line numberDiff line change
@@ -170,15 +170,18 @@ relative, and based on the real path of the files making the calls to
170170

171171
## Loading ECMAScript modules using `require()`
172172

173+
<!-- YAML
174+
changes:
175+
- version: REPLACEME
176+
pr-url: https://github.com/nodejs/node/pull/55085
177+
description: require() now supports loading synchronous ES modules by default.
178+
-->
179+
173180
The `.mjs` extension is reserved for [ECMAScript Modules][].
174-
Currently, if the flag `--experimental-require-module` is not used, loading
175-
an ECMAScript module using `require()` will throw a [`ERR_REQUIRE_ESM`][]
176-
error, and users need to use [`import()`][] instead. See
177-
[Determining module system][] section for more info
181+
See [Determining module system][] section for more info
178182
regarding which files are parsed as ECMAScript modules.
179183

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

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

190-
`require()` will load the requested module as an ES Module, and return
191-
the module namespace object. In this case it is similar to dynamic
193+
If the ES Module being loaded meet the requirements, `require()` can load it and
194+
return the module namespace object. In this case it is similar to dynamic
192195
`import()` but is run synchronously and returns the name space object
193196
directly.
194197

@@ -207,7 +210,7 @@ class Point {
207210
export default Point;
208211
```
209212

210-
A CommonJS module can load them with `require()` under `--experimental-detect-module`:
213+
A CommonJS module can load them with `require()`:
211214

212215
```cjs
213216
const distance = require('./distance.mjs');
@@ -236,13 +239,19 @@ conventions. Code authored directly in CommonJS should avoid depending on it.
236239
If the module being `require()`'d contains top-level `await`, or the module
237240
graph it `import`s contains top-level `await`,
238241
[`ERR_REQUIRE_ASYNC_MODULE`][] will be thrown. In this case, users should
239-
load the asynchronous module using `import()`.
242+
load the asynchronous module using [`import()`][].
240243

241244
If `--experimental-print-required-tla` is enabled, instead of throwing
242245
`ERR_REQUIRE_ASYNC_MODULE` before evaluation, Node.js will evaluate the
243246
module, try to locate the top-level awaits, and print their location to
244247
help users fix them.
245248

249+
Support for loading ES modules using `require()` is currently
250+
experimental and can be disabled using `--no-experimental-require-module`.
251+
When `require()` actually encounters an ES module for the
252+
first time in the process, it will emit an experimental warning. The
253+
warning is expected to be removed when this feature stablizes.
254+
246255
## All together
247256

248257
<!-- type=misc -->
@@ -272,8 +281,7 @@ require(X) from module at path Y
272281
273282
MAYBE_DETECT_AND_LOAD(X)
274283
1. If X parses as a CommonJS module, load X as a CommonJS module. STOP.
275-
2. Else, if `--experimental-require-module` is
276-
enabled, and the source code of X can be parsed as ECMAScript module using
284+
2. Else, if the source code of X can be parsed as ECMAScript module using
277285
<a href="esm.md#resolver-algorithm-specification">DETECT_MODULE_SYNTAX defined in
278286
the ESM resolver</a>,
279287
a. Load X as an ECMAScript module. STOP.
@@ -1190,7 +1198,6 @@ This section was moved to
11901198
[`"main"`]: packages.md#main
11911199
[`"type"`]: packages.md#type
11921200
[`ERR_REQUIRE_ASYNC_MODULE`]: errors.md#err_require_async_module
1193-
[`ERR_REQUIRE_ESM`]: errors.md#err_require_esm
11941201
[`ERR_UNSUPPORTED_DIR_IMPORT`]: errors.md#err_unsupported_dir_import
11951202
[`MODULE_NOT_FOUND`]: errors.md#module_not_found
11961203
[`__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'),

0 commit comments

Comments
 (0)