Skip to content

Commit 2c960a2

Browse files
joyeecheungaduh95
authored andcommitted
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: #55397 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 6764273 commit 2c960a2

File tree

4 files changed

+41
-12
lines changed

4 files changed

+41
-12
lines changed

lib/internal/modules/cjs/loader.js

+22-2
Original file line numberDiff line numberDiff line change
@@ -1369,11 +1369,31 @@ function loadESMFromCJS(mod, filename) {
13691369
// ESM won't be accessible via process.mainModule.
13701370
setOwnProperty(process, 'mainModule', undefined);
13711371
} else {
1372-
emitExperimentalWarning('Support for loading ES Module in require()');
1372+
const parent = mod[kModuleParent];
1373+
let messagePrefix;
1374+
if (parent) {
1375+
// In the case of the module calling `require()`, it's more useful to know its absolute path.
1376+
let from = parent.filename || parent.id;
1377+
// In the case of the module being require()d, it's more useful to know the id passed into require().
1378+
const to = mod.id || mod.filename;
1379+
if (from === 'internal/preload') {
1380+
from = '--require';
1381+
} else if (from === '<repl>') {
1382+
from = 'The REPL';
1383+
} else if (from === '.') {
1384+
from = 'The entry point';
1385+
} else {
1386+
from &&= `CommonJS module ${from}`;
1387+
}
1388+
if (from && to) {
1389+
messagePrefix = `${from} is loading ES Module ${to} using require().\n`;
1390+
}
1391+
}
1392+
emitExperimentalWarning('Support for loading ES Module in require()', messagePrefix);
13731393
const {
13741394
wrap,
13751395
namespace,
1376-
} = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, mod[kModuleParent]);
1396+
} = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, parent);
13771397
// Tooling in the ecosystem have been using the __esModule property to recognize
13781398
// transpiled ESM in consuming code. For example, a 'log' package written in ESM:
13791399
//

lib/internal/util.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -266,10 +266,13 @@ function slowCases(enc) {
266266
}
267267
}
268268

269-
function emitExperimentalWarning(feature) {
269+
function emitExperimentalWarning(feature, messagePrefix) {
270270
if (experimentalWarnings.has(feature)) return;
271-
const msg = `${feature} is an experimental feature and might change at any time`;
272271
experimentalWarnings.add(feature);
272+
let msg = `${feature} is an experimental feature and might change at any time`;
273+
if (messagePrefix) {
274+
msg = messagePrefix + msg;
275+
}
273276
process.emitWarning(msg, 'ExperimentalWarning');
274277
}
275278

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

+10-8
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@ require('../common');
44
const { spawnSyncAndAssert } = require('../common/child_process');
55
const { fixturesDir } = require('../common/fixtures');
66

7-
const warningRE = /ExperimentalWarning: Support for loading ES Module in require/;
87
function testPreload(preloadFlag) {
98
// The warning is only emitted when ESM is loaded by --require.
10-
const stderr = preloadFlag !== '--import' ? warningRE : undefined;
11-
9+
const isRequire = preloadFlag === '--require';
1210
// Test named exports.
1311
{
1412
spawnSyncAndAssert(
@@ -24,7 +22,8 @@ function testPreload(preloadFlag) {
2422
},
2523
{
2624
stdout: 'A',
27-
stderr,
25+
stderr: isRequire ?
26+
/ExperimentalWarning: --require is loading ES Module .*module-named-exports\.mjs using require/ : undefined,
2827
trim: true,
2928
}
3029
);
@@ -44,7 +43,8 @@ function testPreload(preloadFlag) {
4443
cwd: fixturesDir
4544
},
4645
{
47-
stderr,
46+
stderr: isRequire ?
47+
/ExperimentalWarning: --require is loading ES Module .*import-esm\.mjs using require/ : undefined,
4848
stdout: /^world\s+A$/,
4949
trim: true,
5050
}
@@ -66,7 +66,8 @@ function testPreload(preloadFlag) {
6666
},
6767
{
6868
stdout: /^ok\s+A$/,
69-
stderr,
69+
stderr: isRequire ?
70+
/ExperimentalWarning: --require is loading ES Module .*cjs-exports\.mjs using require/ : undefined,
7071
trim: true,
7172
}
7273
);
@@ -89,7 +90,8 @@ function testPreload(preloadFlag) {
8990
},
9091
{
9192
stdout: /^world\s+A$/,
92-
stderr,
93+
stderr: isRequire ?
94+
/ExperimentalWarning: --require is loading ES Module .*require-cjs\.mjs using require/ : undefined,
9395
trim: true,
9496
}
9597
);
@@ -115,7 +117,7 @@ testPreload('--import');
115117
},
116118
{
117119
stdout: /^package-type-module\s+A$/,
118-
stderr: warningRE,
120+
stderr: /ExperimentalWarning: --require is loading ES Module .*package-type-module[\\/]index\.js using require/,
119121
trim: true,
120122
}
121123
);

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

+4
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,13 @@
33

44
const common = require('../common');
55
const assert = require('assert');
6+
const path = require('path');
67

8+
// Only the first load will trigger the warning.
79
common.expectWarning(
810
'ExperimentalWarning',
11+
`CommonJS module ${__filename} is loading ES Module ` +
12+
`${path.resolve(__dirname, '../fixtures/es-module-loaders/module-named-exports.mjs')} using require().\n` +
913
'Support for loading ES Module in require() is an experimental feature ' +
1014
'and might change at any time'
1115
);

0 commit comments

Comments
 (0)