Skip to content

Commit

Permalink
restore warning to be printed only on error
Browse files Browse the repository at this point in the history
  • Loading branch information
@GeoffreyBooth
GeoffreyBooth committed Mar 16, 2024
1 parent f4cd945 commit d301bbb
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 20 deletions.
8 changes: 5 additions & 3 deletions lib/internal/modules/cjs/loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1305,10 +1305,12 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
if (getOptionValue('--experimental-detect-module')) {
// For the main entry point, cache the source to potentially retry as ESM.
cjsRetryAsESMCache.set(filename, content);
} else {
// We only enrich the error (print a warning) if we're sure we're going to for-sure throw it; so if we're
// retrying as ESM, wait until we know whether we're going to retry before calling `enrichCJSError`.
const { enrichCJSError } = require('internal/modules/esm/translators');
enrichCJSError(err, content, filename);
}

const { enrichCJSError } = require('internal/modules/esm/translators');
enrichCJSError(err, content, filename);
}
throw err;
}
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/modules/run_main.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ function executeUserEntryPoint(main = process.argv[1]) {
cjsRetryAsESMCache.delete(resolvedMain);
const { shouldRetryAsESM } = require('internal/modules/helpers');
retryAsESM = shouldRetryAsESM(error.message, source);
if (!retryAsESM) {
const { enrichCJSError } = require('internal/modules/esm/translators');
enrichCJSError(error, source, resolvedMain);
}
}
if (!retryAsESM) {
throw error;
Expand Down
17 changes: 0 additions & 17 deletions test/es-module/test-esm-detect-ambiguous.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
describe('string input', { concurrency: true }, () => {
it('permits ESM syntax in --eval input without requiring --input-type=module', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--eval',
'import { version } from "node:process"; console.log(version);',
Expand All @@ -24,7 +23,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('permits ESM syntax in STDIN input without requiring --input-type=module', async () => {
const child = spawn(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
]);
child.stdin.end('console.log(typeof import.meta.resolve)');
Expand All @@ -34,7 +32,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('should be overridden by --input-type', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--input-type=commonjs',
'--eval',
Expand All @@ -49,7 +46,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('should be overridden by --experimental-default-type', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--experimental-default-type=commonjs',
'--eval',
Expand All @@ -64,7 +60,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('does not trigger detection via source code `eval()`', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--eval',
'eval("import \'nonexistent\';");',
Expand Down Expand Up @@ -106,7 +101,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
]) {
it(testName, async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
entryPath,
]);
Expand Down Expand Up @@ -148,7 +142,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
]) {
it(testName, async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
entryPath,
]);
Expand All @@ -163,7 +156,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
it('should not hint wrong format in resolve hook', async () => {
let writeSync;
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--no-warnings',
'--loader',
Expand Down Expand Up @@ -202,7 +194,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
]) {
it(testName, async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
entryPath,
]);
Expand Down Expand Up @@ -232,7 +223,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
]) {
it(testName, async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
entryPath,
]);
Expand All @@ -249,7 +239,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {
describe('syntax that errors in CommonJS but works in ESM', { concurrency: true }, () => {
it('permits top-level `await`', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--eval',
'await Promise.resolve(); console.log("executed");',
Expand All @@ -263,7 +252,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('permits top-level `await` above import/export syntax', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--eval',
'await Promise.resolve(); import "node:os"; console.log("executed");',
Expand All @@ -277,7 +265,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('still throws on `await` in an ordinary sync function', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--eval',
'function fn() { await Promise.resolve(); } fn();',
Expand All @@ -291,7 +278,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('throws on undefined `require` when top-level `await` triggers ESM parsing', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--eval',
'const fs = require("node:fs"); await Promise.resolve();',
Expand All @@ -305,7 +291,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('permits declaration of CommonJS module variables', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
fixtures.path('es-modules/package-without-type/commonjs-wrapper-variables.js'),
]);
Expand All @@ -318,7 +303,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('permits declaration of CommonJS module variables above import/export', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--eval',
'const module = 3; import "node:os"; console.log("executed");',
Expand All @@ -332,7 +316,6 @@ describe('--experimental-detect-module', { concurrency: true }, () => {

it('still throws on double `const` declaration not at the top level', async () => {
const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [
'--no-warnings',
'--experimental-detect-module',
'--eval',
'function fn() { const require = 1; const require = 2; } fn();',
Expand Down

0 comments on commit d301bbb

Please sign in to comment.