Skip to content

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

Open
marco-ippolito wants to merge 1 commit intonodejs:mainfrom
marco-ippolito:sanitize-config-file
Open

src: make node.config.json throw at unknown fields#62992
marco-ippolito wants to merge 1 commit intonodejs:mainfrom
marco-ippolito:sanitize-config-file

Conversation

@marco-ippolito
Copy link
Copy Markdown
Member

The documentation is correct but the implementation was not

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config

@nodejs-github-bot 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
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.66%. Comparing base (2428030) to head (8923655).
⚠️ Report is 70 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 marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 27, 2026
@ljharb
Copy link
Copy Markdown
Member

ljharb commented Apr 27, 2026

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 github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 27, 2026
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@marco-ippolito
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.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@ljharb
Copy link
Copy Markdown
Member

ljharb commented Apr 28, 2026

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
Copy link
Copy Markdown
Member Author

marco-ippolito commented Apr 28, 2026

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)

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.

4 participants