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 <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Joyee Cheung 2024-07-29 19:46:41 +02:00 committed by GitHub
parent 269175b2b4
commit be7a0c5780
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 19 additions and 6 deletions

View File

@ -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`;
}
@ -164,7 +165,7 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
const parsedSource = tsParse(source);
const detectedFormat = detectModuleFormat(parsedSource, url);
const format = detectedFormat ? `${detectedFormat}-typescript` : 'commonjs-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);

View File

@ -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'),