Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainnodejs/node:mainfrom
marco-ippolito:sanitize-config-filemarco-ippolito/node:sanitize-config-fileCopy head branch name to clipboard
Apr 29, 2026
Merged

src: make node.config.json throw at unknown fields#62992
nodejs-github-bot merged 1 commit into
nodejs:mainnodejs/node:mainfrom
marco-ippolito:sanitize-config-filemarco-ippolito/node:sanitize-config-fileCopy head branch name to clipboard

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
Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>
@codecov

codecov Bot commented Apr 27, 2026

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 marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 27, 2026
@ljharb

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 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

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

marco-ippolito commented Apr 28, 2026

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)

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@marco-ippolito marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2026
@nodejs-github-bot 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 nodejs-github-bot merged commit 5d578c5 into nodejs:main Apr 29, 2026
73 checks passed
@nodejs-github-bot

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>
araujogui pushed a commit to araujogui/node that referenced this pull request May 26, 2026
Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: nodejs#62992
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.