FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: make node.config.json throw at unknown fields by marco-ippolito · Pull Request #62992 · nodejs/node · GitHub

/ node Public

src: make node.config.json throw at unknown fields - #62992

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
marco-ippolito:sanitize-config-file
Apr 29, 2026
Merged

src: make node.config.json throw at unknown fields#62992
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
marco-ippolito:sanitize-config-file

Conversation

Copy link
Copy Markdown
Member

The documentation is correct but the implementation was not

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. config Issues or PRs related to the config subsystem needs-ci PRs that need a full CI run. labels Apr 27, 2026
Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>

codecov Bot commented Apr 27, 2026
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.66%. Comparing base (2428030) to head (8923655).
⚠️ Report is 87 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62992      +/-   ##
==========================================
+ Coverage   89.63%   89.66%   +0.03%     
==========================================
  Files         706      707       +1     
  Lines      219219   219512     +293     
  Branches    42004    42088      +84     
==========================================
+ Hits       196499   196832     +333     
+ Misses      14622    14581      -41     
- Partials     8098     8099       +1     
Files with missing lines Coverage Δ
src/node_config_file.cc 83.96% <100.00%> (+0.23%) ⬆️

... and 60 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 27, 2026

ljharb commented Apr 27, 2026

Copy link
Copy Markdown
Member

won't this mean that you can't make a config file that supports multiple versions of node and gracefully uses features from the newer ones when available?

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 27, 2026

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Member Author

won't this mean that you can't make a config file that supports multiple versions of node and gracefully uses features from the newer ones when available?

Yes but also means if you mispell a configuration you will know. I prefer correctness over convenience.

Copy link
Copy Markdown
Collaborator

ljharb commented Apr 28, 2026

Copy link
Copy Markdown
Member

It's not just about convenience, though - it's about being able to support multiple node versions at one time, which also makes upgrading easier (and not supporting that makes upgrading harder).

I very much prioritize correctness, but having a closed config every time is highly likely to hold back the ecosystem.

marco-ippolito commented Apr 28, 2026
edited
Loading

Copy link
Copy Markdown
Member Author

The support for multiple versions of node in the same configuration was never planned and should be discouraged. The $schema needs to match with version being used. I think a possible solution to this problem would be support an array of configurations and node can pick the right one based on version. This plus the ability to extend (like tsconfig)

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Collaborator

marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2026
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2026
nodejs-github-bot merged commit 5d578c5 into nodejs:main Apr 29, 2026
73 checks passed

Copy link
Copy Markdown
Collaborator

Landed in 5d578c5

aduh95 pushed a commit that referenced this pull request May 5, 2026
Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: #62992
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: #62992
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. config Issues or PRs related to the config subsystem needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL