From 296b4b15af150d3ac1c5fa883a0234a00eaf8617 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 29 Jul 2024 19:46:41 +0200 Subject: [PATCH] module: do not warn for typeless package.json when there isn't one It was intended that warnings should only be emitted for an existing package.json without a type. This fixes a confusing warning telling users to update /package.json when there are no package.json on the lookup path at all, like this: [MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module because module syntax was detected; to avoid the performance penalty of syntax detection, add "type": "module" to /package.json Drive-by: update the warning message to be clear about reparsing and make it clear what's actionable. PR-URL: https://github.com/nodejs/node/pull/54045 Reviewed-By: Geoffrey Booth Reviewed-By: Antoine du Hamel Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/internal/modules/esm/get_format.js | 13 +++++++------ test/es-module/test-esm-detect-ambiguous.mjs | 12 ++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index afbee7fdf5cd8e..a89446df710a94 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -92,8 +92,9 @@ let typelessPackageJsonFilesWarnedAbout; function warnTypelessPackageJsonFile(pjsonPath, url) { typelessPackageJsonFilesWarnedAbout ??= new SafeSet(); if (!typelessPackageJsonFilesWarnedAbout.has(pjsonPath)) { - const warning = `${url} parsed as an ES module because module syntax was detected;` + - ` to avoid the performance penalty of syntax detection, add "type": "module" to ${pjsonPath}`; + const warning = `Module type of ${url} is not specified and it doesn't parse as CommonJS.\n` + + 'Reparsing as ES module because module syntax was detected. This incurs a performance overhead.\n' + + `To eliminate this warning, add "type": "module" to ${pjsonPath}.`; process.emitWarning(warning, { code: 'MODULE_TYPELESS_PACKAGE_JSON', }); @@ -112,7 +113,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE const ext = extname(url); if (ext === '.js') { - const { type: packageType, pjsonPath } = getPackageScopeConfig(url); + const { type: packageType, pjsonPath, exists: foundPackageJson } = getPackageScopeConfig(url); if (packageType !== 'none') { return packageType; } @@ -133,7 +134,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE // For ambiguous files (no type field, .js extension) we return // undefined from `resolve` and re-run the check in `load`. const format = detectModuleFormat(source, url); - if (format === 'module') { + if (format === 'module' && foundPackageJson) { // This module has a .js extension, a package.json with no `type` field, and ESM syntax. // Warn about the missing `type` field so that the user can avoid the performance penalty of detection. warnTypelessPackageJsonFile(pjsonPath, url); @@ -143,7 +144,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE } } if (ext === '.ts' && getOptionValue('--experimental-strip-types')) { - const { type: packageType, pjsonPath } = getPackageScopeConfig(url); + const { type: packageType, pjsonPath, exists: foundPackageJson } = getPackageScopeConfig(url); if (packageType !== 'none') { return `${packageType}-typescript`; } @@ -168,7 +169,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE const detectedFormat = detectModuleFormat(parsedSource, url); // When source is undefined, default to module-typescript. const format = detectedFormat ? `${detectedFormat}-typescript` : 'module-typescript'; - if (format === 'module-typescript') { + if (format === 'module-typescript' && foundPackageJson) { // This module has a .js extension, a package.json with no `type` field, and ESM syntax. // Warn about the missing `type` field so that the user can avoid the performance penalty of detection. warnTypelessPackageJsonFile(pjsonPath, url); diff --git a/test/es-module/test-esm-detect-ambiguous.mjs b/test/es-module/test-esm-detect-ambiguous.mjs index eb061473de66a6..fce455cf18bb60 100644 --- a/test/es-module/test-esm-detect-ambiguous.mjs +++ b/test/es-module/test-esm-detect-ambiguous.mjs @@ -352,6 +352,18 @@ describe('Module syntax detection', { concurrency: !process.env.TEST_PARALLEL }, }); } + it('does not warn when there are no package.json', async () => { + const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ + fixtures.path('es-modules/loose.js'), + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, 'executed\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('warns only once for a package.json that affects multiple files', async () => { const { stdout, stderr, code, signal } = await spawnPromisified(process.execPath, [ fixtures.path('es-modules/package-without-type/detected-as-esm.js'),