Skip to content

Commit

Permalink
module: include module information in require(esm) warning
Browse files Browse the repository at this point in the history
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: nodejs#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>
  • Loading branch information
joyeecheung authored and tpoisseau committed Nov 21, 2024
1 parent 9b43d4c commit 00c6652
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 12 deletions.
24 changes: 22 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 === '<repl>') {
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:
//
Expand Down
7 changes: 5 additions & 2 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

Expand Down
18 changes: 10 additions & 8 deletions test/es-module/test-require-module-preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
}
);
Expand All @@ -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,
}
Expand All @@ -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,
}
);
Expand All @@ -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,
}
);
Expand All @@ -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,
}
);
Expand Down
4 changes: 4 additions & 0 deletions test/es-module/test-require-module.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
Expand Down

0 comments on commit 00c6652

Please sign in to comment.