From 2c960a212e5f33debcb458aa91e138c9c62478e3 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 22 Oct 2024 18:51:45 +0200 Subject: [PATCH] module: include module information in require(esm) warning When emitting the experimental warning for `require(esm)`, include information about the parent module and the module being require()-d to help users locate and update them. PR-URL: https://github.com/nodejs/node/pull/55397 Reviewed-By: Matteo Collina Reviewed-By: Stephen Belanger Reviewed-By: Chemi Atlow Reviewed-By: James M Snell Reviewed-By: Chengzhong Wu --- lib/internal/modules/cjs/loader.js | 24 +++++++++++++++++-- lib/internal/util.js | 7 ++++-- test/es-module/test-require-module-preload.js | 18 +++++++------- test/es-module/test-require-module.js | 4 ++++ 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 7d8e6948e2552c..23366e973afa3b 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1369,11 +1369,31 @@ 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 parent = mod[kModuleParent]; + let messagePrefix; + if (parent) { + // In the case of the module calling `require()`, it's more useful to know its absolute path. + let from = parent.filename || parent.id; + // In the case of the module being require()d, it's more useful to know the id passed into require(). + const to = mod.id || mod.filename; + if (from === 'internal/preload') { + from = '--require'; + } else if (from === '') { + from = 'The REPL'; + } else if (from === '.') { + from = 'The entry point'; + } else { + from &&= `CommonJS module ${from}`; + } + if (from && to) { + messagePrefix = `${from} is loading ES Module ${to} using require().\n`; + } + } + emitExperimentalWarning('Support for loading ES Module in require()', messagePrefix); const { wrap, namespace, - } = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, mod[kModuleParent]); + } = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, parent); // Tooling in the ecosystem have been using the __esModule property to recognize // transpiled ESM in consuming code. For example, a 'log' package written in ESM: // diff --git a/lib/internal/util.js b/lib/internal/util.js index 6e048250c6a189..4d8cf99a3453d9 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -266,10 +266,13 @@ function slowCases(enc) { } } -function emitExperimentalWarning(feature) { +function emitExperimentalWarning(feature, messagePrefix) { if (experimentalWarnings.has(feature)) return; - const msg = `${feature} is an experimental feature and might change at any time`; experimentalWarnings.add(feature); + let msg = `${feature} is an experimental feature and might change at any time`; + if (messagePrefix) { + msg = messagePrefix + msg; + } process.emitWarning(msg, 'ExperimentalWarning'); } diff --git a/test/es-module/test-require-module-preload.js b/test/es-module/test-require-module-preload.js index 65ec1a93bc9d4b..7a8a09486b43d7 100644 --- a/test/es-module/test-require-module-preload.js +++ b/test/es-module/test-require-module-preload.js @@ -4,11 +4,9 @@ require('../common'); const { spawnSyncAndAssert } = require('../common/child_process'); const { fixturesDir } = require('../common/fixtures'); -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; - + const isRequire = preloadFlag === '--require'; // Test named exports. { spawnSyncAndAssert( @@ -24,7 +22,8 @@ function testPreload(preloadFlag) { }, { stdout: 'A', - stderr, + stderr: isRequire ? + /ExperimentalWarning: --require is loading ES Module .*module-named-exports\.mjs using require/ : undefined, trim: true, } ); @@ -44,7 +43,8 @@ function testPreload(preloadFlag) { cwd: fixturesDir }, { - stderr, + stderr: isRequire ? + /ExperimentalWarning: --require is loading ES Module .*import-esm\.mjs using require/ : undefined, stdout: /^world\s+A$/, trim: true, } @@ -66,7 +66,8 @@ function testPreload(preloadFlag) { }, { stdout: /^ok\s+A$/, - stderr, + stderr: isRequire ? + /ExperimentalWarning: --require is loading ES Module .*cjs-exports\.mjs using require/ : undefined, trim: true, } ); @@ -89,7 +90,8 @@ function testPreload(preloadFlag) { }, { stdout: /^world\s+A$/, - stderr, + stderr: isRequire ? + /ExperimentalWarning: --require is loading ES Module .*require-cjs\.mjs using require/ : undefined, trim: true, } ); @@ -115,7 +117,7 @@ testPreload('--import'); }, { stdout: /^package-type-module\s+A$/, - stderr: warningRE, + stderr: /ExperimentalWarning: --require is loading ES Module .*package-type-module[\\/]index\.js using require/, trim: true, } ); diff --git a/test/es-module/test-require-module.js b/test/es-module/test-require-module.js index 2cc7d0198837df..4987fbf7b07d43 100644 --- a/test/es-module/test-require-module.js +++ b/test/es-module/test-require-module.js @@ -3,9 +3,13 @@ const common = require('../common'); const assert = require('assert'); +const path = 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' );