module: warn on invalid package.json type field - #60180
Conversation
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: nodejs#60085
ljharb
left a comment
There was a problem hiding this comment.
tests look good to me, i'll let someone else review the C++
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #60180 +/- ##
==========================================
+ Coverage 88.52% 88.56% +0.03%
==========================================
Files 703 704 +1
Lines 207825 208092 +267
Branches 40003 40012 +9
==========================================
+ Hits 183976 184294 +318
+ Misses 15862 15811 -51
Partials 7987 7987
🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| if (field_value != "commonjs" && field_value != "module") { | ||
| fprintf(stderr, |
There was a problem hiding this comment.
This should be using the usual machinery (process.emitWarning() in JS, or ProcessEmitWarning() in C++) to emit warning so that it can be suppressed with NODE_NO_WARNINGS=1 etc. or captured through event listeners by users.
There was a problem hiding this comment.
Done! Changed to use ProcessEmitWarning() instead of fprintf(). Let me know if the implementation looks correct.
| if (package_json == nullptr) { | ||
| return; | ||
| } | ||
| } |
|
|
||
| }); | ||
|
|
||
| describe('package.json type field validation', () => { |
There was a problem hiding this comment.
Can you add a new test file instead of appending it to the same test file? See
node/doc/contributing/writing-tests.md
Lines 30 to 32 in 3983ef6
There was a problem hiding this comment.
Done! I've added a new test file.
|
lint errors in some lines |
|
@ljharb, @mertcanaltin I have done the necessary changes, please have a look at my PR as per your convenience. |
|
This pull request has been marked as stale due to 90 days of inactivity. |
|
This pull request has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days). |
|
i think this should land once somebody reviews the C++ |
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: #60085