Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: include module information in require(esm) warning #55397

Merged
merged 4 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -1385,11 +1385,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 @@ -262,10 +262,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
Loading