Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit de313b2

Browse filesBrowse files
joyeecheungmarco-ippolito
joyeecheung
authored and
marco-ippolito
committed
module: only emit require(esm) warning under --trace-require-module
require(esm) is relatively stable now and the experimental warning has run its course - it's now more troublesome than useful. This patch changes it to no longer emit a warning unless `--trace-require-module` is explicitly used. The flag supports two modes: - `--trace-require-module=all`: emit warnings for all usages - `--trace-require-module=no-node-modules`: emit warnings for usages that do not come from a `node_modules` folder. PR-URL: #56194 Backport-PR-URL: #56927 Fixes: #55417 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Refs: #52697
1 parent 3d89e6b commit de313b2
Copy full SHA for de313b2

File tree

Expand file treeCollapse file tree

9 files changed

+83
-46
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

9 files changed

+83
-46
lines changed
Open diff view settings
Collapse file

‎doc/api/cli.md‎

Copy file name to clipboardExpand all lines: doc/api/cli.md
+13Lines changed: 13 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -2387,6 +2387,18 @@ added:
23872387
Prints a stack trace whenever an environment is exited proactively,
23882388
i.e. invoking `process.exit()`.
23892389

2390+
### `--trace-require-module=mode`
2391+
2392+
<!-- YAML
2393+
added:
2394+
- REPLACEME
2395+
-->
2396+
2397+
Prints information about usage of [Loading ECMAScript modules using `require()`][].
2398+
2399+
When `mode` is `all`, all usage is printed. When `mode` is `no-node-modules`, usage
2400+
from the `node_modules` folder is excluded.
2401+
23902402
### `--trace-sigint`
23912403

23922404
<!-- YAML
@@ -2865,6 +2877,7 @@ one is included in the list below.
28652877
* `--trace-event-file-pattern`
28662878
* `--trace-events-enabled`
28672879
* `--trace-exit`
2880+
* `--trace-require-module`
28682881
* `--trace-sigint`
28692882
* `--trace-sync-io`
28702883
* `--trace-tls`
Collapse file

‎doc/api/modules.md‎

Copy file name to clipboardExpand all lines: doc/api/modules.md
+10-4Lines changed: 10 additions & 4 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,13 @@ added:
175175
- v22.0.0
176176
- v20.17.0
177177
changes:
178-
- version: REPLACEME
178+
- version:
179+
- REPLACEME
180+
pr-url: https://github.com/nodejs/node/pull/56194
181+
description: This feature no longer emits an experimental warning by default,
182+
though the warning can still be emitted by --trace-require-module.
183+
- version:
184+
- REPLACEME
179185
pr-url: https://github.com/nodejs/node/pull/55085
180186
description: This feature is no longer behind the `--experimental-require-module` CLI flag.
181187
- version: REPLACEME
@@ -315,9 +321,8 @@ help users fix them.
315321

316322
Support for loading ES modules using `require()` is currently
317323
experimental and can be disabled using `--no-experimental-require-module`.
318-
When `require()` actually encounters an ES module for the
319-
first time in the process, it will emit an experimental warning. The
320-
warning is expected to be removed when this feature stablizes.
324+
To print where this feature is used, use [`--trace-require-module`][].
325+
321326
This feature can be detected by checking if
322327
[`process.features.require_module`][] is `true`.
323328

@@ -1266,6 +1271,7 @@ This section was moved to
12661271
[GLOBAL_FOLDERS]: #loading-from-the-global-folders
12671272
[`"main"`]: packages.md#main
12681273
[`"type"`]: packages.md#type
1274+
[`--trace-require-module`]: cli.md#--trace-require-modulemode
12691275
[`ERR_REQUIRE_ASYNC_MODULE`]: errors.md#err_require_async_module
12701276
[`ERR_UNSUPPORTED_DIR_IMPORT`]: errors.md#err_unsupported_dir_import
12711277
[`MODULE_NOT_FOUND`]: errors.md#module_not_found
Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+17-12Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,7 +1316,7 @@ Module.prototype.require = function(id) {
13161316
}
13171317
};
13181318

1319-
let emittedRequireModuleWarning = false;
1319+
let requireModuleWarningMode;
13201320
/**
13211321
* Resolved path to `process.argv[1]` will be lazily placed here
13221322
* (needed for setting breakpoint when called with `--inspect-brk`).
@@ -1345,17 +1345,22 @@ function loadESMFromCJS(mod, filename) {
13451345
} else {
13461346
const parent = mod[kModuleParent];
13471347

1348-
if (!emittedRequireModuleWarning) {
1348+
requireModuleWarningMode ??= getOptionValue('--trace-require-module');
1349+
if (requireModuleWarningMode) {
13491350
let shouldEmitWarning = false;
1350-
// Check if the require() comes from node_modules.
1351-
if (parent) {
1352-
shouldEmitWarning = !isUnderNodeModules(parent.filename);
1353-
} else if (mod[kIsCachedByESMLoader]) {
1354-
// It comes from the require() built for `import cjs` and doesn't have a parent recorded
1355-
// in the CJS module instance. Inspect the stack trace to see if the require()
1356-
// comes from node_modules and reduce the noise. If there are more than 100 frames,
1357-
// just give up and assume it is under node_modules.
1358-
shouldEmitWarning = !isInsideNodeModules(100, true);
1351+
if (requireModuleWarningMode === 'no-node-modules') {
1352+
// Check if the require() comes from node_modules.
1353+
if (parent) {
1354+
shouldEmitWarning = !isUnderNodeModules(parent.filename);
1355+
} else if (mod[kIsCachedByESMLoader]) {
1356+
// It comes from the require() built for `import cjs` and doesn't have a parent recorded
1357+
// in the CJS module instance. Inspect the stack trace to see if the require()
1358+
// comes from node_modules and reduce the noise. If there are more than 100 frames,
1359+
// just give up and assume it is under node_modules.
1360+
shouldEmitWarning = !isInsideNodeModules(100, true);
1361+
}
1362+
} else {
1363+
shouldEmitWarning = true;
13591364
}
13601365
if (shouldEmitWarning) {
13611366
let messagePrefix;
@@ -1381,7 +1386,7 @@ function loadESMFromCJS(mod, filename) {
13811386
messagePrefix,
13821387
undefined,
13831388
parent?.require);
1384-
emittedRequireModuleWarning = true;
1389+
requireModuleWarningMode = true;
13851390
}
13861391
}
13871392
const {
Collapse file

‎src/node_options.cc‎

Copy file name to clipboardExpand all lines: src/node_options.cc
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,11 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
150150
errors->push_back("--heapsnapshot-near-heap-limit must not be negative");
151151
}
152152

153+
if (!trace_require_module.empty() && trace_require_module != "all" &&
154+
trace_require_module != "no-node-modules") {
155+
errors->push_back("invalid value for --trace-require-module");
156+
}
157+
153158
if (test_runner) {
154159
if (syntax_check_only) {
155160
errors->push_back("either --test or --check can be used, not both");
@@ -734,6 +739,14 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
734739
"set module system to use by default",
735740
&EnvironmentOptions::type,
736741
kAllowedInEnvvar);
742+
743+
AddOption(
744+
"--trace-require-module",
745+
"Print access to require(esm). Options are 'all' (print all usage) and "
746+
"'no-node-modules' (excluding usage from the node_modules folder)",
747+
&EnvironmentOptions::trace_require_module,
748+
kAllowedInEnvvar);
749+
737750
AddOption("--extra-info-on-fatal-exception",
738751
"hide extra information on fatal exception that causes exit",
739752
&EnvironmentOptions::extra_info_on_fatal_exception,
Collapse file

‎src/node_options.h‎

Copy file name to clipboardExpand all lines: src/node_options.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ class EnvironmentOptions : public Options {
199199
bool trace_uncaught = false;
200200
bool trace_warnings = false;
201201
bool trace_promises = false;
202+
std::string trace_require_module;
202203
bool extra_info_on_fatal_exception = true;
203204
std::string unhandled_rejections;
204205
std::vector<std::string> userland_loaders;
Collapse file

‎test/es-module/test-require-module-preload.js‎

Copy file name to clipboardExpand all lines: test/es-module/test-require-module-preload.js
-11Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ const { spawnSyncAndAssert } = require('../common/child_process');
55
const { fixturesDir } = require('../common/fixtures');
66

77
function testPreload(preloadFlag) {
8-
// The warning is only emitted when ESM is loaded by --require.
9-
const isRequire = preloadFlag === '--require';
108
// Test named exports.
119
{
1210
spawnSyncAndAssert(
@@ -22,8 +20,6 @@ function testPreload(preloadFlag) {
2220
},
2321
{
2422
stdout: 'A',
25-
stderr: isRequire ?
26-
/ExperimentalWarning: --require is loading ES Module .*module-named-exports\.mjs using require/ : undefined,
2723
trim: true,
2824
}
2925
);
@@ -43,8 +39,6 @@ function testPreload(preloadFlag) {
4339
cwd: fixturesDir
4440
},
4541
{
46-
stderr: isRequire ?
47-
/ExperimentalWarning: --require is loading ES Module .*import-esm\.mjs using require/ : undefined,
4842
stdout: /^world\s+A$/,
4943
trim: true,
5044
}
@@ -66,8 +60,6 @@ function testPreload(preloadFlag) {
6660
},
6761
{
6862
stdout: /^ok\s+A$/,
69-
stderr: isRequire ?
70-
/ExperimentalWarning: --require is loading ES Module .*cjs-exports\.mjs using require/ : undefined,
7163
trim: true,
7264
}
7365
);
@@ -90,8 +82,6 @@ function testPreload(preloadFlag) {
9082
},
9183
{
9284
stdout: /^world\s+A$/,
93-
stderr: isRequire ?
94-
/ExperimentalWarning: --require is loading ES Module .*require-cjs\.mjs using require/ : undefined,
9585
trim: true,
9686
}
9787
);
@@ -117,7 +107,6 @@ testPreload('--import');
117107
},
118108
{
119109
stdout: /^package-type-module\s+A$/,
120-
stderr: /ExperimentalWarning: --require is loading ES Module .*package-type-module[\\/]index\.js using require/,
121110
trim: true,
122111
}
123112
);
Collapse file

‎test/es-module/test-require-module-warning.js‎

Copy file name to clipboardExpand all lines: test/es-module/test-require-module-warning.js
+11-3Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
'use strict';
22

3-
// This checks the warning and the stack trace emitted by the require(esm)
4-
// experimental warning. It can get removed when `require(esm)` becomes stable.
5-
3+
// This checks the warning and the stack trace emitted by --trace-require-module=all.
64
require('../common');
75
const { spawnSyncAndAssert } = require('../common/child_process');
86
const fixtures = require('../common/fixtures');
97
const assert = require('assert');
108

119
spawnSyncAndAssert(process.execPath, [
1210
'--trace-warnings',
11+
'--trace-require-module=all',
1312
fixtures.path('es-modules', 'require-module.js'),
1413
], {
1514
trim: true,
@@ -33,3 +32,12 @@ spawnSyncAndAssert(process.execPath, [
3332
);
3433
}
3534
});
35+
36+
spawnSyncAndAssert(process.execPath, [
37+
'--trace-require-module=1',
38+
fixtures.path('es-modules', 'require-module.js'),
39+
], {
40+
status: 9,
41+
trim: true,
42+
stderr: /invalid value for --trace-require-module/
43+
});
Collapse file

‎test/es-module/test-require-module.js‎

Copy file name to clipboardExpand all lines: test/es-module/test-require-module.js
-10Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,6 @@
33

44
const common = require('../common');
55
const assert = require('assert');
6-
const path = require('path');
7-
8-
// Only the first load will trigger the warning.
9-
common.expectWarning(
10-
'ExperimentalWarning',
11-
`CommonJS module ${__filename} is loading ES Module ` +
12-
`${path.resolve(__dirname, '../fixtures/es-module-loaders/module-named-exports.mjs')} using require().\n` +
13-
'Support for loading ES Module in require() is an experimental feature ' +
14-
'and might change at any time'
15-
);
166

177
// Test named exports.
188
{
Collapse file

‎test/es-module/test-require-node-modules-warning.js‎

Copy file name to clipboardExpand all lines: test/es-module/test-require-node-modules-warning.js
+18-6Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

3-
// This checks the experimental warning for require(esm) is disabled when the
4-
// require() comes from node_modules.
3+
// This checks the warning and the stack trace emitted by
4+
// --trace-require-module=no-node-modules.
55
require('../common');
66
const { spawnSyncAndAssert } = require('../common/child_process');
77
const fixtures = require('../common/fixtures');
@@ -14,7 +14,10 @@ const warningRE = /Support for loading ES Module in require\(\)/;
1414
// require() in non-node_modules -> esm in node_modules should warn.
1515
spawnSyncAndAssert(
1616
process.execPath,
17-
[fixtures.path('es-modules', 'test_node_modules', 'require-esm.js')],
17+
[
18+
'--trace-require-module=no-node-modules',
19+
fixtures.path('es-modules', 'test_node_modules', 'require-esm.js'),
20+
],
1821
{
1922
trim: true,
2023
stderr: warningRE,
@@ -26,7 +29,10 @@ spawnSyncAndAssert(
2629
// should not warn.
2730
spawnSyncAndAssert(
2831
process.execPath,
29-
[fixtures.path('es-modules', 'test_node_modules', 'require-require-esm.js')],
32+
[
33+
'--trace-require-module=no-node-modules',
34+
fixtures.path('es-modules', 'test_node_modules', 'require-require-esm.js'),
35+
],
3036
{
3137
trim: true,
3238
stderr: '',
@@ -38,7 +44,10 @@ spawnSyncAndAssert(
3844
// should not warn.
3945
spawnSyncAndAssert(
4046
process.execPath,
41-
[fixtures.path('es-modules', 'test_node_modules', 'import-require-esm.mjs')],
47+
[
48+
'--trace-require-module=no-node-modules',
49+
fixtures.path('es-modules', 'test_node_modules', 'import-require-esm.mjs'),
50+
],
4251
{
4352
trim: true,
4453
stderr: '',
@@ -50,7 +59,10 @@ spawnSyncAndAssert(
5059
// require() in node_modules -> esm in node_modules should not warn.
5160
spawnSyncAndAssert(
5261
process.execPath,
53-
[fixtures.path('es-modules', 'test_node_modules', 'import-import-require-esm.mjs')],
62+
[
63+
'--trace-require-module=no-node-modules',
64+
fixtures.path('es-modules', 'test_node_modules', 'import-import-require-esm.mjs'),
65+
],
5466
{
5567
trim: true,
5668
stderr: '',

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.