Skip to content

src: mark config file as stable - #66431

Open
marco-ippolito wants to merge 2 commits into
nodejs:mainfrom
marco-ippolito:config-file-stable
Open

marco-ippolito wants to merge 2 commits into
nodejs:mainfrom
marco-ippolito:config-file-stable

Conversation

@marco-ippolito

Copy link
Copy Markdown
Member

No description provided.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/security-wg
  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Oct 1, 2026
@marco-ippolito marco-ippolito added the config Issues and PRs related to Node.js configuration and feature settings. label Oct 1, 2026
@JakobJingleheimer

JakobJingleheimer commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

I would love for this to get to stable (I one of my recent favourites in node), but I think we still have an important unresolved issue (that I admittedly haven't raised properly):

The schema(s) cite a specific node version, but that is not checked against the actual version of node running. So looking at the code, all looks well; and then it runs and 💥

This really reared its head with the recent breaking change of "testRunner" → "test" (which was also backported as a minor, breaking a stable line). I agree the change was good, but it broke the hell out of anyone using the config file for tests with no way to catch it.

Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>
@marco-ippolito

Copy link
Copy Markdown
Member Author

I would love for this to get to stable (I one of my recent favourites in node), but I think we still have an important unresolved issue (that I admittedly haven't raised much):

The schema(s) cite a specific node version, but that is not checked against the actual version of node running. So looking at the code, all looks well; and then it runs and 💥

This really reared its head with the recent breaking change of "testRunner" → "test" (which was also backported as a minor, breaking a stable line). I agree the change was good, but it broke the hell out of anyone using the config file for tests with no way to catch it.

that's because it's an experimental feature, it made no sense to make the breaking change a semver major. it's trivial to add a check to match the version

@JakobJingleheimer

Copy link
Copy Markdown
Member

that's because it's an experimental feature, it made no sense to make the breaking change a semver major.

For sure yes not a major; I would expect it on the current line (just not backported to a stable line).

it's trivial to add a check to match the version

Yeah, I'm suggesting we do that before marking this stable 🙂

@marco-ippolito

marco-ippolito commented Oct 1, 2026 •

Copy link
Copy Markdown
Member Author

Mind that this feature you are asking already partially xists:
https://nodejs.org/docs/latest/api/cli.html#--experimental-config-filepath---experimental-config-file

{
  "nodeVersion": 25,
  "nodeOptions": {
    "watch-path": "src"
  }
}

If you specify a nodeVersion it will be enforced

@JakobJingleheimer

Copy link
Copy Markdown
Member

Mind that this feature you are asking already partially xists: https://github.com/nodejs/node/actions/runs/36847018850/job/110320230489?pr=66431

{
  "nodeVersion": 25,
  "nodeOptions": {
    "watch-path": "src"
  }
}

If you specify a nodeVersion it will be enforced

Ish. It wouldn't have addressed the above problem though because it's only major (and the break was in a minor); but maybe that's a non-issue once it becomes stable? 🤔

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>
@marco-ippolito

Copy link
Copy Markdown
Member Author

Ish. It wouldn't have addressed the above problem though because it's only major (and the break was in a minor); but maybe that's a non-issue once it becomes stable? 🤔

yes because it is stable across majors, at worse node has a more features thatn the config file but not breaking changes

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.38%. Comparing base (8bf7793) to head (89646eb).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
src/node_config_file.cc 96.77% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66431      +/-   ##
==========================================
- Coverage   90.39%   90.38%   -0.01%     
==========================================
  Files         792      792              
  Lines      275580   275717     +137     
  Branches    52840    52861      +21     
==========================================
+ Hits       249104   249206     +102     
- Misses      16897    16899       +2     
- Partials     9579     9612      +33     
Files with missing lines Coverage Δ
lib/internal/bench_runner/cli.js 94.15% <100.00%> (-0.07%) ⬇️
lib/internal/debugger/inspect_helpers.js 92.52% <100.00%> (+0.01%) ⬆️
lib/internal/process/pre_execution.js 95.59% <ø> (-0.46%) ⬇️
lib/internal/test_runner/runner.js 95.19% <100.00%> (+<0.01%) ⬆️
src/node_config_file.h 100.00% <ø> (ø)
src/node_options.cc 81.60% <100.00%> (+0.01%) ⬆️
src/node_options.h 95.67% <ø> (ø)
src/node_config_file.cc 84.41% <96.77%> (+0.73%) ⬆️

... and 33 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. Only starts once the PR has an approving review. label Oct 1, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 1, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config Issues and PRs related to Node.js configuration and feature settings. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants