From 71c2d02be9e7d1b0a43113a8cd76b1aff705695f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 2 Oct 2024 20:07:30 +0200 Subject: [PATCH] module: do not warn when require(esm) comes from node_modules 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. PR-URL: https://github.com/nodejs/node/pull/55217 Refs: https://github.com/nodejs/node/issues/52697 Reviewed-By: Matteo Collina Reviewed-By: Antoine du Hamel --- lib/internal/modules/cjs/loader.js | 22 ++++++- .../test-require-node-modules-warning.js | 59 +++++++++++++++++++ .../import-import-require-esm.mjs | 2 + .../test_node_modules/import-require-esm.mjs | 2 + .../node_modules/esm/index.js | 1 + .../node_modules/esm/package.json | 4 ++ .../node_modules/import-require-esm/index.js | 2 + .../import-require-esm/package.json | 4 ++ .../node_modules/require-esm/index.js | 2 + .../test_node_modules/require-esm.js | 2 + .../test_node_modules/require-require-esm.js | 2 + 11 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 test/es-module/test-require-node-modules-warning.js create mode 100644 test/fixtures/es-modules/test_node_modules/import-import-require-esm.mjs create mode 100644 test/fixtures/es-modules/test_node_modules/import-require-esm.mjs create mode 100644 test/fixtures/es-modules/test_node_modules/node_modules/esm/index.js create mode 100644 test/fixtures/es-modules/test_node_modules/node_modules/esm/package.json create mode 100644 test/fixtures/es-modules/test_node_modules/node_modules/import-require-esm/index.js create mode 100644 test/fixtures/es-modules/test_node_modules/node_modules/import-require-esm/package.json create mode 100644 test/fixtures/es-modules/test_node_modules/node_modules/require-esm/index.js create mode 100644 test/fixtures/es-modules/test_node_modules/require-esm.js create mode 100644 test/fixtures/es-modules/test_node_modules/require-require-esm.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index e4b3ce57b88b1e..67c2d0088f06ea 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -124,6 +124,7 @@ const { pathToFileURL, fileURLToPath, isURL } = require('internal/url'); const { pendingDeprecate, emitExperimentalWarning, + isInsideNodeModules, isUnderNodeModules, kEmptyObject, setOwnProperty, @@ -1351,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 @@ -1374,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); + } 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: // diff --git a/test/es-module/test-require-node-modules-warning.js b/test/es-module/test-require-node-modules-warning.js new file mode 100644 index 00000000000000..048802ae03442c --- /dev/null +++ b/test/es-module/test-require-node-modules-warning.js @@ -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 +// 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', + } +); diff --git a/test/fixtures/es-modules/test_node_modules/import-import-require-esm.mjs b/test/fixtures/es-modules/test_node_modules/import-import-require-esm.mjs new file mode 100644 index 00000000000000..d470088c38caee --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/import-import-require-esm.mjs @@ -0,0 +1,2 @@ +import mod from 'import-require-esm'; +console.log(mod.hello); diff --git a/test/fixtures/es-modules/test_node_modules/import-require-esm.mjs b/test/fixtures/es-modules/test_node_modules/import-require-esm.mjs new file mode 100644 index 00000000000000..2ad346de9e841c --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/import-require-esm.mjs @@ -0,0 +1,2 @@ +import mod from 'require-esm'; +console.log(mod.hello); diff --git a/test/fixtures/es-modules/test_node_modules/node_modules/esm/index.js b/test/fixtures/es-modules/test_node_modules/node_modules/esm/index.js new file mode 100644 index 00000000000000..35f468bf4856d8 --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/node_modules/esm/index.js @@ -0,0 +1 @@ +export const hello = 'world'; diff --git a/test/fixtures/es-modules/test_node_modules/node_modules/esm/package.json b/test/fixtures/es-modules/test_node_modules/node_modules/esm/package.json new file mode 100644 index 00000000000000..07aec65d5a4f31 --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/node_modules/esm/package.json @@ -0,0 +1,4 @@ +{ + "type": "module", + "main": "index.js" +} diff --git a/test/fixtures/es-modules/test_node_modules/node_modules/import-require-esm/index.js b/test/fixtures/es-modules/test_node_modules/node_modules/import-require-esm/index.js new file mode 100644 index 00000000000000..918bb5d5597e34 --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/node_modules/import-require-esm/index.js @@ -0,0 +1,2 @@ +import mod from 'require-esm'; +export default mod; diff --git a/test/fixtures/es-modules/test_node_modules/node_modules/import-require-esm/package.json b/test/fixtures/es-modules/test_node_modules/node_modules/import-require-esm/package.json new file mode 100644 index 00000000000000..07aec65d5a4f31 --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/node_modules/import-require-esm/package.json @@ -0,0 +1,4 @@ +{ + "type": "module", + "main": "index.js" +} diff --git a/test/fixtures/es-modules/test_node_modules/node_modules/require-esm/index.js b/test/fixtures/es-modules/test_node_modules/node_modules/require-esm/index.js new file mode 100644 index 00000000000000..ca6f0c264ad1fc --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/node_modules/require-esm/index.js @@ -0,0 +1,2 @@ +module.exports = require('esm'); + diff --git a/test/fixtures/es-modules/test_node_modules/require-esm.js b/test/fixtures/es-modules/test_node_modules/require-esm.js new file mode 100644 index 00000000000000..60ad3f7fff60c6 --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/require-esm.js @@ -0,0 +1,2 @@ +const { hello } = require('esm'); +console.log(hello); diff --git a/test/fixtures/es-modules/test_node_modules/require-require-esm.js b/test/fixtures/es-modules/test_node_modules/require-require-esm.js new file mode 100644 index 00000000000000..9fe255dce258a6 --- /dev/null +++ b/test/fixtures/es-modules/test_node_modules/require-require-esm.js @@ -0,0 +1,2 @@ +const { hello } = require('require-esm'); +console.log(hello);