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: do not warn when require(esm) comes from node_modules #55960

Merged
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
60 changes: 40 additions & 20 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const {
module_export_private_symbol,
module_parent_private_symbol,
},
isInsideNodeModules,
} = internalBinding('util');

const { kEvaluated, createRequiredModuleFacade } = internalBinding('module_wrap');
Expand Down Expand Up @@ -131,6 +132,7 @@ const {
setOwnProperty,
getLazy,
isWindows,
isUnderNodeModules,
} = require('internal/util');
const {
makeContextifyScript,
Expand Down Expand Up @@ -1328,6 +1330,7 @@ Module.prototype.require = function(id) {
}
};

let emittedRequireModuleWarning = false;
/**
* Resolve and evaluate it synchronously as ESM if it's ESM.
* @param {Module} mod CJS module instance
Expand All @@ -1347,29 +1350,46 @@ function loadESMFromCJS(mod, filename, format, source) {
setOwnProperty(process, 'mainModule', undefined);
} else {
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 (!emittedRequireModuleWarning) {
let shouldEmitWarning = false;
// Check if the require() comes from node_modules.
if (parent) {
shouldEmitWarning = !isUnderNodeModules(parent.filename);
} else if (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 (from && to) {
messagePrefix = `${from} is loading ES Module ${to} using require().\n`;
if (shouldEmitWarning) {
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,
undefined,
parent?.require);
emittedRequireModuleWarning = true;
}
}
emitExperimentalWarning('Support for loading ES Module in require()',
messagePrefix,
undefined,
parent?.require);
const {
wrap,
namespace,
Expand Down
59 changes: 59 additions & 0 deletions test/es-module/test-require-node-modules-warning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
'use strict';

// This checks the experimental warning for require(esm) is disabled when the
// require() comes from node_modules.
require('../common');
const { spawnSyncAndAssert } = require('../common/child_process');
const fixtures = require('../common/fixtures');

const warningRE = /Support for loading ES Module in require\(\)/;

// The fixtures are placed in a directory that includes "node_modules" in its name
// to check false negatives.

// require() in non-node_modules -> esm in node_modules should warn.
spawnSyncAndAssert(
process.execPath,
[fixtures.path('es-modules', 'test_node_modules', 'require-esm.js')],
{
trim: true,
stderr: warningRE,
stdout: 'world',
}
);

// require() in non-node_modules -> require() in node_modules -> esm in node_modules
// should not warn.
spawnSyncAndAssert(
process.execPath,
[fixtures.path('es-modules', 'test_node_modules', 'require-require-esm.js')],
{
trim: true,
stderr: '',
stdout: 'world',
}
);

// Import in non-node_modules -> require() in node_modules -> esm in node_modules
// should not warn.
spawnSyncAndAssert(
process.execPath,
[fixtures.path('es-modules', 'test_node_modules', 'import-require-esm.mjs')],
{
trim: true,
stderr: '',
stdout: 'world',
}
);

// Import in non-node_modules -> import in node_modules ->
// 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')],
{
trim: true,
stderr: '',
stdout: 'world',
}
);

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions test/fixtures/es-modules/test_node_modules/require-esm.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading