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: #56194Fixes: #55417 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>

Author: joyeecheung

doc/api/cli.md

@@ -2669,6 +2669,18 @@ added:
 Prints a stack trace whenever an environment is exited proactively,
 i.e. invoking `process.exit()`.
 
+### `--trace-require-module=mode`
+
+<!-- YAML
+added:
+ - REPLACEME
+-->
+
+Prints information about usage of [Loading ECMAScript modules using `require()`][].
+
+When `mode` is `all`, all usage is printed. When `mode` is `no-node-modules`, usage
+from the `node_modules` folder is excluded.
+
 ### `--trace-sigint`
 
 <!-- YAML
@@ -3180,6 +3192,7 @@ one is included in the list below.
 *`--trace-event-file-pattern`
 *`--trace-events-enabled`
 *`--trace-exit`
+*`--trace-require-module`
 *`--trace-sigint`
 *`--trace-sync-io`
 *`--trace-tls`

doc/api/modules.md

@@ -174,9 +174,15 @@ relative, and based on the real path of the files making the calls to
 added:
  - v22.0.0
 changes:
- - version: v22.12.0
+ - version:
+ - v23.0.0
+ - v22.12.0
  pr-url: https://github.com/nodejs/node/pull/55085
  description: This feature is no longer behind the `--experimental-require-module` CLI flag.
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs/node/pull/56194
+ description: This feature no longer emits an experimental warning by default,
+ though the warning can still be emitted by --trace-require-module.
  - version: v22.12.0
  pr-url: https://github.com/nodejs/node/pull/54563
  description: Support `'module.exports'` interop export in `require(esm)`.
@@ -314,9 +320,8 @@ 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.
+To print where this feature is used, use [`--trace-require-module`][].
+
 This feature can be detected by checking if
 [`process.features.require_module`][] is `true`.
 
@@ -1266,6 +1271,7 @@ This section was moved to
 [GLOBAL_FOLDERS]: #loading-from-the-global-folders
 [`"main"`]: packages.md#main
 [`"type"`]: packages.md#type
+[`--trace-require-module`]: cli.md#--trace-require-modulemode
 [`ERR_REQUIRE_ASYNC_MODULE`]: errors.md#err_require_async_module
 [`ERR_UNSUPPORTED_DIR_IMPORT`]: errors.md#err_unsupported_dir_import
 [`MODULE_NOT_FOUND`]: errors.md#module_not_found

lib/internal/modules/cjs/loader.js

@@ -1338,7 +1338,7 @@ Module.prototype.require = function(id){
 }
 };
 
-letemittedRequireModuleWarning=false;
+letrequireModuleWarningMode;
 /**
  * Resolve and evaluate it synchronously as ESM if it's ESM.
  * @param{Module} mod CJS module instance
@@ -1361,17 +1361,22 @@ function loadESMFromCJS(mod, filename){
 }else{
 constparent=mod[kModuleParent];
 
-if(!emittedRequireModuleWarning){
+requireModuleWarningMode??=getOptionValue('--trace-require-module');
+if(requireModuleWarningMode){
 letshouldEmitWarning=false;
-// Check if the require() comes from node_modules.
-if(parent){
-shouldEmitWarning=!isUnderNodeModules(parent.filename);
-}elseif(mod[kIsCachedByESMLoader]){
-// It comes from the require() built for `import cjs` and doesn't have a parent recorded
-// in the CJS module instance. Inspect the stack trace to see if the require()
-// comes from node_modules and reduce the noise. If there are more than 100 frames,
-// just give up and assume it is under node_modules.
-shouldEmitWarning=!isInsideNodeModules(100,true);
+if(requireModuleWarningMode==='no-node-modules'){
+// Check if the require() comes from node_modules.
+if(parent){
+shouldEmitWarning=!isUnderNodeModules(parent.filename);
+}elseif(mod[kIsCachedByESMLoader]){
+// It comes from the require() built for `import cjs` and doesn't have a parent recorded
+// in the CJS module instance. Inspect the stack trace to see if the require()
+// comes from node_modules and reduce the noise. If there are more than 100 frames,
+// just give up and assume it is under node_modules.
+shouldEmitWarning=!isInsideNodeModules(100,true);
+}
+}else{
+shouldEmitWarning=true;
 }
 if(shouldEmitWarning){
 letmessagePrefix;
@@ -1397,7 +1402,7 @@ function loadESMFromCJS(mod, filename){
 messagePrefix,
 undefined,
 parent?.require);
-emittedRequireModuleWarning=true;
+requireModuleWarningMode=true;
 }
 }
 const{

src/node_options.cc

@@ -142,6 +142,11 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
  errors->push_back("--heapsnapshot-near-heap-limit must not be negative");
  }
 
+if (!trace_require_module.empty() && trace_require_module != "all" &&
+ trace_require_module != "no-node-modules"){
+ errors->push_back("invalid value for --trace-require-module");
+ }
+
 if (test_runner){
 if (test_isolation == "none"){
  debug_options_.allow_attaching_debugger = true;
@@ -797,6 +802,14 @@ EnvironmentOptionsParser::EnvironmentOptionsParser(){
 "set module system to use by default",
  &EnvironmentOptions::type,
 kAllowedInEnvvar);
+
+AddOption(
+"--trace-require-module",
+"Print access to require(esm). Options are 'all' (print all usage) and "
+"'no-node-modules' (excluding usage from the node_modules folder)",
+ &EnvironmentOptions::trace_require_module,
+kAllowedInEnvvar);
+
 AddOption("--extra-info-on-fatal-exception",
 "hide extra information on fatal exception that causes exit",
  &EnvironmentOptions::extra_info_on_fatal_exception,

src/node_options.h

@@ -213,6 +213,7 @@ class EnvironmentOptions : public Options{
 bool trace_env = false;
 bool trace_env_js_stack = false;
 bool trace_env_native_stack = false;
+ std::string trace_require_module;
 bool extra_info_on_fatal_exception = true;
  std::string unhandled_rejections;
  std::vector<std::string> userland_loaders;

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

@@ -5,8 +5,6 @@ const{spawnSyncAndAssert } = require('../common/child_process');
 const{ fixturesDir }=require('../common/fixtures');
 
 functiontestPreload(preloadFlag){
-// The warning is only emitted when ESM is loaded by --require.
-constisRequire=preloadFlag==='--require';
 // Test named exports.
 {
 spawnSyncAndAssert(
@@ -22,8 +20,6 @@ function testPreload(preloadFlag){
 },
 {
 stdout: 'A',
-stderr: isRequire ?
-/ExperimentalWarning:--requireisloadingESModule.*module-named-exports\.mjsusingrequire/ : undefined,
 trim: true,
 }
 );
@@ -43,8 +39,6 @@ function testPreload(preloadFlag){
 cwd: fixturesDir
 },
 {
-stderr: isRequire ?
-/ExperimentalWarning:--requireisloadingESModule.*import-esm\.mjsusingrequire/ : undefined,
 stdout: /^world\s+A$/,
 trim: true,
 }
@@ -66,8 +60,6 @@ function testPreload(preloadFlag){
 },
 {
 stdout: /^ok\s+A$/,
-stderr: isRequire ?
-/ExperimentalWarning:--requireisloadingESModule.*cjs-exports\.mjsusingrequire/ : undefined,
 trim: true,
 }
 );
@@ -90,8 +82,6 @@ function testPreload(preloadFlag){
 },
 {
 stdout: /^world\s+A$/,
-stderr: isRequire ?
-/ExperimentalWarning:--requireisloadingESModule.*require-cjs\.mjsusingrequire/ : undefined,
 trim: true,
 }
 );
@@ -117,7 +107,6 @@ testPreload('--import');
 },
 {
 stdout: /^package-type-module\s+A$/,
-stderr: /ExperimentalWarning:--requireisloadingESModule.*package-type-module[\\/]index\.jsusingrequire/,
 trim: true,
 }
 );

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

@@ -1,15 +1,14 @@
 'use strict';
 
-// This checks the warning and the stack trace emitted by the require(esm)
-// experimental warning. It can get removed when `require(esm)` becomes stable.
-
+// This checks the warning and the stack trace emitted by --trace-require-module=all.
 require('../common');
 const{ spawnSyncAndAssert }=require('../common/child_process');
 constfixtures=require('../common/fixtures');
 constassert=require('assert');
 
 spawnSyncAndAssert(process.execPath,[
 '--trace-warnings',
+'--trace-require-module=all',
 fixtures.path('es-modules','require-module.js'),
 ],{
 trim: true,
@@ -33,3 +32,12 @@ spawnSyncAndAssert(process.execPath, [
 );
 }
 });
+
+spawnSyncAndAssert(process.execPath,[
+'--trace-require-module=1',
+fixtures.path('es-modules','require-module.js'),
+],{
+status: 9,
+trim: true,
+stderr: /invalidvaluefor--trace-require-module/
+});

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

@@ -3,16 +3,6 @@
 
 constcommon=require('../common');
 constassert=require('assert');
-constpath=require('path');
-
-// Only the first load will trigger the warning.
-common.expectWarning(
-'ExperimentalWarning',
-`CommonJS module ${__filename} is loading ES Module `+
-`${path.resolve(__dirname,'../fixtures/es-module-loaders/module-named-exports.mjs')} using require().\n`+
-'Support for loading ES Module in require() is an experimental feature '+
-'and might change at any time'
-);
 
 // Test named exports.
 {

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

@@ -1,7 +1,7 @@
 'use strict';
 
-// This checks the experimental warning for require(esm) is disabled when the
-// require() comes from node_modules.
+// This checks the warning and the stack trace emitted by
+// --trace-require-module=no-node-modules.
 require('../common');
 const{ spawnSyncAndAssert }=require('../common/child_process');
 constfixtures=require('../common/fixtures');
@@ -14,7 +14,10 @@ const warningRE = /Support for loading ES Module in require\(\)/;
 // require() in non-node_modules -> esm in node_modules should warn.
 spawnSyncAndAssert(
 process.execPath,
-[fixtures.path('es-modules','test_node_modules','require-esm.js')],
+[
+'--trace-require-module=no-node-modules',
+fixtures.path('es-modules','test_node_modules','require-esm.js'),
+],
 {
 trim: true,
 stderr: warningRE,
@@ -26,7 +29,10 @@ spawnSyncAndAssert(
 // should not warn.
 spawnSyncAndAssert(
 process.execPath,
-[fixtures.path('es-modules','test_node_modules','require-require-esm.js')],
+[
+'--trace-require-module=no-node-modules',
+fixtures.path('es-modules','test_node_modules','require-require-esm.js'),
+],
 {
 trim: true,
 stderr: '',
@@ -38,7 +44,10 @@ spawnSyncAndAssert(
 // should not warn.
 spawnSyncAndAssert(
 process.execPath,
-[fixtures.path('es-modules','test_node_modules','import-require-esm.mjs')],
+[
+'--trace-require-module=no-node-modules',
+fixtures.path('es-modules','test_node_modules','import-require-esm.mjs'),
+],
 {
 trim: true,
 stderr: '',
@@ -50,7 +59,10 @@ spawnSyncAndAssert(
 // require() in node_modules -> esm in node_modules should not warn.
 spawnSyncAndAssert(
 process.execPath,
-[fixtures.path('es-modules','test_node_modules','import-import-require-esm.mjs')],
+[
+'--trace-require-module=no-node-modules',
+fixtures.path('es-modules','test_node_modules','import-import-require-esm.mjs'),
+],
 {
 trim: true,
 stderr: '',

test/es-module/test-typescript-commonjs.mjs

@@ -116,7 +116,6 @@ test('execute a .cts file importing a .mts file export', async () =>{
 fixtures.path('typescript/cts/test-require-mts-module.cts'),
 ]);
 
-match(result.stderr,/SupportforloadingESModuleinrequire\(\)isanexperimentalfeatureandmightchangeatanytime/);
 match(result.stdout,/Hello,TypeScript!/);
 strictEqual(result.code,0);
 });