Skip to content

Commit

Permalink
module: do not warn when require(esm) comes from node_modules
Browse files Browse the repository at this point in the history
Some packages have been using try-catch to load require(esm)
in environments that are available. In 23, where require(esm)
is unflagged, we emit an experimental warning when require()
is used to load ESM. To backport require(esm) to older LTS
releases, however, this could break existing CLI output.
To reduce the disruption for LTS, on older release lines,
this commit is applied to skip the warning if require(esm)
comes from node_modules. This warning will be eventually
removed when require(esm) becomes stable.
  • Loading branch information
joyeecheung committed Oct 8, 2024
1 parent 76a2fa8 commit 898396a
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 2 deletions.
23 changes: 21 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ const { pathToFileURL, fileURLToPath, isURL } = require('internal/url');
const {
pendingDeprecate,
emitExperimentalWarning,
isInsideNodeModules,
isUnderNodeModules,
kEmptyObject,
setOwnProperty,
getLazy,
Expand Down Expand Up @@ -1350,6 +1352,7 @@ let resolvedArgv;
let hasPausedEntry = false;
/** @type {import('vm').Script} */

let emittedRequireModuleWarning = false;
/**
* Resolve and evaluate it synchronously as ESM if it's ESM.
* @param {Module} mod CJS module instance
Expand All @@ -1373,11 +1376,27 @@ 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];
if (!emittedRequireModuleWarning) {
let shouldEmitWarning = false;
// Check if the require() comes from node_modules.
if (parent) {
shouldEmitWarning = !isUnderNodeModules(parent.filename)

Check failure on line 1384 in lib/internal/modules/cjs/loader.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Missing semicolon
} 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.
shouldEmitWarning = !isInsideNodeModules();
}
if (shouldEmitWarning) {
emitExperimentalWarning('Support for loading ES Module in require()');
emittedRequireModuleWarning = true;
}
}
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
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 = /ExperimentalWarning: 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

Check failure on line 37 in test/es-module/test-require-node-modules-warning.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Comments should not begin with a lowercase character
// 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 ->

Check failure on line 49 in test/es-module/test-require-node-modules-warning.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Comments should not begin with a lowercase character
// 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.

0 comments on commit 898396a

Please sign in to comment.