From e6223eecd4069145f854d8edcbb86c8995435079 Mon Sep 17 00:00:00 2001 From: Shuklax Date: Fri, 10 Oct 2025 00:23:25 +0530 Subject: [PATCH 1/2] module: warn on invalid package.json type field Add validation to warn users when the "type" field in package.json contains an invalid value. Previously, values like "CommonJS" (wrong case) would silently fall back to typeless behavior. Now a clear warning message is displayed indicating the expected values are "commonjs" or "module". Fixes: https://github.com/nodejs/node/issues/60085 --- src/node_modules.cc | 11 ++++- test/parallel/test-find-package-json.js | 64 +++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/src/node_modules.cc b/src/node_modules.cc index 7e615a691a5a14..259f80772d7a0f 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -212,6 +212,15 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON( if (value.get_string().get(field_value)) { return throw_invalid_package_config(); } + + if (field_value != "commonjs" && field_value != "module") { + fprintf(stderr, + "node: [MODULE_INVALID_TYPE] Invalid \"type\" field in package.json: \"%.*s\". " + "Expected \"commonjs\" or \"module\".\n", + static_cast(field_value.size()), + field_value.data()); + } + // Only update type if it is "commonjs" or "module" // The default value is "none" for backward compatibility. if (field_value == "commonjs" || field_value == "module") { @@ -275,7 +284,7 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo& args) { if (package_json == nullptr) { return; - } + } args.GetReturnValue().Set(package_json->Serialize(realm)); } diff --git a/test/parallel/test-find-package-json.js b/test/parallel/test-find-package-json.js index fd9a1a85e77441..1e27f29256d55a 100644 --- a/test/parallel/test-find-package-json.js +++ b/test/parallel/test-find-package-json.js @@ -204,4 +204,68 @@ describe('findPackageJSON', () => { // Throws when no arguments are provided assert.ok(stdout.includes(foundPjsonPath), stdout); assert.strictEqual(code, 0); }); + + +}); + +describe('package.json type field validation', () => { + it('should warn for invalid type field values', async () => { + tmpdir.refresh(); + + fs.mkdirSync(tmpdir.resolve('invalid-type-pkg'), { recursive: true }); + fs.writeFileSync( + tmpdir.resolve('invalid-type-pkg/package.json'), + JSON.stringify({ type: 'CommonJS' }) + ); + fs.writeFileSync( + tmpdir.resolve('invalid-type-pkg/index.js'), + 'module.exports = 42;' + ); + + const { stderr } = await common.spawnPromisified(process.execPath, [ + tmpdir.resolve('invalid-type-pkg/index.js'), + ]); + + assert.ok(stderr.includes('MODULE_INVALID_TYPE')); + assert.ok(stderr.includes('CommonJS')); + }); + + it('should not warn for valid type values', async () => { + tmpdir.refresh(); + + fs.mkdirSync(tmpdir.resolve('valid-type-pkg'), { recursive: true }); + fs.writeFileSync( + tmpdir.resolve('valid-type-pkg/package.json'), + JSON.stringify({ type: 'commonjs' }) + ); + fs.writeFileSync( + tmpdir.resolve('valid-type-pkg/index.js'), + 'module.exports = 42;' + ); + + const { stderr } = await common.spawnPromisified(process.execPath, [ + tmpdir.resolve('valid-type-pkg/index.js'), + ]); + + assert.ok(!stderr.includes('MODULE_INVALID_TYPE')); + }); + + it('should not warn when type field is missing', async () => { + tmpdir.refresh(); + fs.mkdirSync(tmpdir.resolve('no-type-pkg'), { recursive: true }); + fs.writeFileSync( + tmpdir.resolve('no-type-pkg/package.json'), + JSON.stringify({ name: 'test' }) + ); + fs.writeFileSync( + tmpdir.resolve('no-type-pkg/index.js'), + 'module.exports = 42;' + ); + + const { stderr } = await common.spawnPromisified(process.execPath, [ + tmpdir.resolve('no-type-pkg/index.js'), + ]); + + assert.ok(!stderr.includes('MODULE_INVALID_TYPE')); + }); }); From 5b3fde4932fffdf3e02ec0605205795feaad803d Mon Sep 17 00:00:00 2001 From: Shuklax Date: Sun, 12 Oct 2025 10:10:47 +0530 Subject: [PATCH 2/2] address review feedback: use ProcessEmitWarning and separate test file --- src/node_modules.cc | 13 ++-- test/parallel/test-find-package-json.js | 62 ---------------- .../test-package-json-type-validation.js | 70 +++++++++++++++++++ 3 files changed, 77 insertions(+), 68 deletions(-) create mode 100644 test/parallel/test-package-json-type-validation.js diff --git a/src/node_modules.cc b/src/node_modules.cc index 259f80772d7a0f..4c767011282b93 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -214,11 +214,12 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON( } if (field_value != "commonjs" && field_value != "module") { - fprintf(stderr, - "node: [MODULE_INVALID_TYPE] Invalid \"type\" field in package.json: \"%.*s\". " - "Expected \"commonjs\" or \"module\".\n", - static_cast(field_value.size()), - field_value.data()); + ProcessEmitWarning(realm->env(), + "Invalid \"type\" field in package.json: \"%.*s\". " + "Expected \"commonjs\" or \"module\".", + "InvalidPackageType", + static_cast(field_value.size()), + field_value.data()); } // Only update type if it is "commonjs" or "module" @@ -284,7 +285,7 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo& args) { if (package_json == nullptr) { return; - } + } args.GetReturnValue().Set(package_json->Serialize(realm)); } diff --git a/test/parallel/test-find-package-json.js b/test/parallel/test-find-package-json.js index 1e27f29256d55a..07404d0c650015 100644 --- a/test/parallel/test-find-package-json.js +++ b/test/parallel/test-find-package-json.js @@ -207,65 +207,3 @@ describe('findPackageJSON', () => { // Throws when no arguments are provided }); - -describe('package.json type field validation', () => { - it('should warn for invalid type field values', async () => { - tmpdir.refresh(); - - fs.mkdirSync(tmpdir.resolve('invalid-type-pkg'), { recursive: true }); - fs.writeFileSync( - tmpdir.resolve('invalid-type-pkg/package.json'), - JSON.stringify({ type: 'CommonJS' }) - ); - fs.writeFileSync( - tmpdir.resolve('invalid-type-pkg/index.js'), - 'module.exports = 42;' - ); - - const { stderr } = await common.spawnPromisified(process.execPath, [ - tmpdir.resolve('invalid-type-pkg/index.js'), - ]); - - assert.ok(stderr.includes('MODULE_INVALID_TYPE')); - assert.ok(stderr.includes('CommonJS')); - }); - - it('should not warn for valid type values', async () => { - tmpdir.refresh(); - - fs.mkdirSync(tmpdir.resolve('valid-type-pkg'), { recursive: true }); - fs.writeFileSync( - tmpdir.resolve('valid-type-pkg/package.json'), - JSON.stringify({ type: 'commonjs' }) - ); - fs.writeFileSync( - tmpdir.resolve('valid-type-pkg/index.js'), - 'module.exports = 42;' - ); - - const { stderr } = await common.spawnPromisified(process.execPath, [ - tmpdir.resolve('valid-type-pkg/index.js'), - ]); - - assert.ok(!stderr.includes('MODULE_INVALID_TYPE')); - }); - - it('should not warn when type field is missing', async () => { - tmpdir.refresh(); - fs.mkdirSync(tmpdir.resolve('no-type-pkg'), { recursive: true }); - fs.writeFileSync( - tmpdir.resolve('no-type-pkg/package.json'), - JSON.stringify({ name: 'test' }) - ); - fs.writeFileSync( - tmpdir.resolve('no-type-pkg/index.js'), - 'module.exports = 42;' - ); - - const { stderr } = await common.spawnPromisified(process.execPath, [ - tmpdir.resolve('no-type-pkg/index.js'), - ]); - - assert.ok(!stderr.includes('MODULE_INVALID_TYPE')); - }); -}); diff --git a/test/parallel/test-package-json-type-validation.js b/test/parallel/test-package-json-type-validation.js new file mode 100644 index 00000000000000..ac62e170bc6953 --- /dev/null +++ b/test/parallel/test-package-json-type-validation.js @@ -0,0 +1,70 @@ +'use strict'; + +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const { describe, it } = require('node:test'); + + +describe('package.json type field validation', () => { + it('should warn for invalid type field values', async () => { + tmpdir.refresh(); + + fs.mkdirSync(tmpdir.resolve('invalid-type-pkg'), { recursive: true }); + fs.writeFileSync( + tmpdir.resolve('invalid-type-pkg/package.json'), + JSON.stringify({ type: 'CommonJS' }) + ); + fs.writeFileSync( + tmpdir.resolve('invalid-type-pkg/index.js'), + 'module.exports = 42;' + ); + + const { stderr } = await common.spawnPromisified(process.execPath, [ + tmpdir.resolve('invalid-type-pkg/index.js'), + ]); + + assert.ok(stderr.includes('MODULE_INVALID_TYPE')); + assert.ok(stderr.includes('CommonJS')); + }); + + it('should not warn for valid type values', async () => { + tmpdir.refresh(); + + fs.mkdirSync(tmpdir.resolve('valid-type-pkg'), { recursive: true }); + fs.writeFileSync( + tmpdir.resolve('valid-type-pkg/package.json'), + JSON.stringify({ type: 'commonjs' }) + ); + fs.writeFileSync( + tmpdir.resolve('valid-type-pkg/index.js'), + 'module.exports = 42;' + ); + + const { stderr } = await common.spawnPromisified(process.execPath, [ + tmpdir.resolve('valid-type-pkg/index.js'), + ]); + + assert.ok(!stderr.includes('MODULE_INVALID_TYPE')); + }); + + it('should not warn when type field is missing', async () => { + tmpdir.refresh(); + fs.mkdirSync(tmpdir.resolve('no-type-pkg'), { recursive: true }); + fs.writeFileSync( + tmpdir.resolve('no-type-pkg/package.json'), + JSON.stringify({ name: 'test' }) + ); + fs.writeFileSync( + tmpdir.resolve('no-type-pkg/index.js'), + 'module.exports = 42;' + ); + + const { stderr } = await common.spawnPromisified(process.execPath, [ + tmpdir.resolve('no-type-pkg/index.js'), + ]); + + assert.ok(!stderr.includes('MODULE_INVALID_TYPE')); + }); +}); \ No newline at end of file