Skip to content

lib: use validators for argument validation #59416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Sep 21, 2025
Merged

lib: use validators for argument validation #59416

merged 2 commits into from
Sep 21, 2025

Conversation

meteorqz6
Copy link
Contributor

This refactors internal validation helpers in child_process to use the common validators in lib/internal/validators.js where possible.

This improves code consistency and maintainability.

@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. needs-ci PRs that need a full CI run. labels Aug 9, 2025
@codecov Codecov
Copy link

codecov bot commented Aug 9, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.88%. Comparing base (1be57c2) to head (49c5ea1).
⚠️ Report is 267 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #59416      +/-   ##
==========================================
- Coverage   89.89%   89.88%   -0.01%     
==========================================
  Files         656      656              
  Lines      192983   192981       -2     
  Branches    37849    37849              
==========================================
- Hits       173478   173466      -12     
- Misses      12055    12064       +9     
- Partials     7450     7451       +1
Files with missing lines Coverage Δ
lib/child_process.js 95.53% <100.00%> (-0.01%) ⬇️

... and 22 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.

BridgeAR
BridgeAR reviewed Aug 11, 2025
View reviewed changes
Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

I agree that this seems right. This is a major change though.

@meteorqz6 can you please have a look at the tests? They need to be adjusted.

@BridgeAR BridgeAR added the semver-major PRs that contain breaking changes and should be released in the next major version. label Aug 11, 2025
@meteorqz6
lib: use validators for argument validation
bb46bec 
This refactors internal validation helpers in `child_process` to use
the common validators in `lib/internal/validators.js` where possible.

This improves code consistency and maintainability.
@meteorqz6
test: fix validation error assertions in child_process
49c5ea1
@meteorqz6
Copy link
Contributor Author

I've adjusted the tests.
@BridgeAR Could you please take a look when you have a moment? Thanks!

@meteorqz6
Copy link
Contributor Author

Please take a look when you have a moment. @BridgeAR

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/69218/

@daeyeon
Copy link
Member

daeyeon commented Sep 14, 2025

@nodejs/tsc semver-major requires at least 2 TSC approvals to land.

@daeyeon daeyeon added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Sep 14, 2025
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/69235/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/69305/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/69306/

@daeyeon daeyeon added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 21, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 21, 2025
@nodejs-github-bot nodejs-github-bot merged commit 39d7303 into nodejs:main Sep 21, 2025
64 of 66 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 39d7303

@RafaelGSS RafaelGSS mentioned this pull request Oct 13, 2025
Open
9 tasks
RafaelGSS added a commit that referenced this pull request Oct 13, 2025
@RafaelGSS
2025-10-15, Version 25.0.0 (Current)
060d31c 
assert:
  * (SEMVER-MAJOR) move assert.fail with multiple arguments to eol (James M Snell) #58532
  * (SEMVER-MAJOR) move CallTracker to EOL (James M Snell) #58006
assert,util:
  * (SEMVER-MAJOR) fail promise comparison in deep equal checks (Ruben Bridgewater) #59448
  * (SEMVER-MAJOR) handle invalid dates as equal in deep comparison (Ruben Bridgewater) #57627
async_hooks:
  * (SEMVER-MAJOR) move `asyncResource` property on bound function to EOL (James M Snell) #58618
buffer:
  * (SEMVER-MAJOR) move SlowBuffer to EOL (Filip Skokan) #58220
build:
  * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #59805
  * (SEMVER-MAJOR) bump minimum Clang version to 19 (Michaël Zasso) #59048
  * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #58064
  * (SEMVER-MAJOR) stop distributing Corepack (Antoine du Hamel) #57617
child_process:
  * (SEMVER-MAJOR) move _channel to end-of-life (James M Snell) #58527
crypto:
  * (SEMVER-MAJOR) runtime-deprecate default shake128/256 output lengths (Filip Skokan) #59008
  * (SEMVER-MAJOR) move deprecated hash and mgf1Hash options to EOL (James M Snell) #58706
  * (SEMVER-MAJOR) runtime deprecate ECDH.setPublicKey() (James M Snell) #58620
deps:
  * (SEMVER-MAJOR) patch V8 for illumos (Dan McDonald) #59805
  * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #54077
  * (SEMVER-MAJOR) update V8 to 14.1.146.11 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) V8: backport 1d3362c55396 (Shu-yu Guo) #58064
  * (SEMVER-MAJOR) V8: cherry-pick 4f38995c8295 (Shu-yu Guo) #58064
  * (SEMVER-MAJOR) V8: cherry-pick 044b9b6f589d (Rezvan Mahdavi Hezaveh) #58064
  * (SEMVER-MAJOR) V8: cherry-pick d2ad518a0b57 (Joyee Cheung) #58064
  * (SEMVER-MAJOR) V8: revert 6d6c1e680c7b (Michaël Zasso) #58064
  * (SEMVER-MAJOR) V8: revert e3cddbedb205 (Michaël Zasso) #58064
  * (SEMVER-MAJOR) use std::map in MSVC STL for EphemeronRememberedSet (Joyee Cheung) #58064
  * (SEMVER-MAJOR) patch V8 for illumos (Dan McDonald) #58064
  * (SEMVER-MAJOR) remove problematic comment from v8-internal (Michaël Zasso) #58064
  * (SEMVER-MAJOR) define V8_PRESERVE_MOST as no-op on Windows (Stefan Stojanovic) #58064
  * (SEMVER-MAJOR) fix FP16 bitcasts.h (Stefan Stojanovic) #58064
  * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #58064
  * (SEMVER-MAJOR) update V8 to 13.7.152.9 (Michaël Zasso) #58064
dgram:
  * (SEMVER-MAJOR) move deprecated APIs to EOL (James M Snell) #58474
dns:
  * (SEMVER-MAJOR) move falsy hostname in lookup to end-of-life (James M Snell) #58619
doc,src,test:
  * (SEMVER-MAJOR) replace use of deprecated `GetIsolate` (Michaël Zasso) #59805
fs:
  * (SEMVER-MAJOR) move FileHandle close on GC to EOL (James M Snell) #58536
  * (SEMVER-MAJOR) move rmdir recursive option to end-of-life (James M Snell) #58616
  * (SEMVER-MAJOR) make `processReadResult()` and `readSyncRecursive()` private (Livia Medeiros) #58672
  * (SEMVER-MAJOR) move fs stream open method to eol (James M Snell) #58529
  * (SEMVER-MAJOR) remove `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #55862
http:
  * (SEMVER-MAJOR) deprecate writeHeader (Sebastian Beltran) #59060
lib:
  * (SEMVER-MAJOR) use validators for argument validation (Nam Yooseong) #59416
  * (SEMVER-MAJOR) expose global ErrorEvent (Richie Bendall) #58920
  * (SEMVER-MAJOR) deprecate `_stream_*` modules (Dario Piotrowicz) #58337
  * (SEMVER-MAJOR) deprecate _tls_common and _tls_wrap (Dario Piotrowicz) #57643
module:
  * (SEMVER-MAJOR) move Module._debug to end-of-life (James M Snell) #58473
node-api:
  * (SEMVER-MAJOR) add warning for NAPI_EXPERIMENTAL (Miguel Marcondes Filho) #58280
perf_hooks:
  * (SEMVER-MAJOR) move deprecated accessors to EOF (James M Snell) #58531
process:
  * (SEMVER-MAJOR) move multipleResolves event to EOL (James M Snell) #58707
repl:
  * (SEMVER-MAJOR) eol deprecate instantiating without new (Aviv Keller) #59495
src:
  * (SEMVER-MAJOR) update crypto.getCipherInfo() to use DictionaryTemplate (James M Snell) #60036
  * (SEMVER-MAJOR) fix calls to v8::Object::wrap (Andreas Haas) #59805
  * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 141 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) improve performance of dotenv ToObject (James M Snell) #60038
  * (SEMVER-MAJOR) use std::string_view from node_report (iknoom) #60006
  * (SEMVER-MAJOR) unflag --experimental-webstorage by default (Daniel M Brasil) #57666
  * (SEMVER-MAJOR) store `Local` for `CallbackScope` on stack (Anna Henningsen) #59705
  * (SEMVER-MAJOR) remove node.h APIs to make callback without an async context (Chengzhong Wu) #58471
  * (SEMVER-MAJOR) remove deprecated node::EmitBeforeExit and node::EmitExit (Chengzhong Wu) #58469
  * (SEMVER-MAJOR) remove deprecated node::CreatePlatform and node::FreePlatform (Chengzhong Wu) #58470
  * (SEMVER-MAJOR) remove deprecated node::InitializeNodeWithArgs (Chengzhong Wu) #58470
  * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 138 (Michaël Zasso) #58064
src,permission:
  * (SEMVER-MAJOR) add --allow-net permission (Rafael Gonzaga) #58517
test:
  * (SEMVER-MAJOR) update cppgc-object addon config (StefanStojanovic) #59805
  * (SEMVER-MAJOR) spin longer for sequential/test-worker-prof (Michaël Zasso) #59805
  * (SEMVER-MAJOR) remove `--always-turbofan` flag (Michaël Zasso) #59805
  * (SEMVER-MAJOR) update snapshot for V8 14.1 (Michaël Zasso) #59805
test,win:
  * (SEMVER-MAJOR) split addon tests (StefanStojanovic) #59805
tls:
  * (SEMVER-MAJOR) move IP-address servername deprecation to eol (James M Snell) #58533
tools:
  * (SEMVER-MAJOR) update V8 gypfiles for 14.0 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) update V8 gypfiles for 13.9 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) update V8 gypfiles for 13.8 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) enable leaptiering for aix/ibmi (Abdirahim Musse) #59805
  * (SEMVER-MAJOR) update V8 gypfiles for 13.7 (Michaël Zasso) #58064
url:
  * (SEMVER-MAJOR) move bad port deprecation in legacy url to end-of-life (James M Snell) #58617
util,console:
  * (SEMVER-MAJOR) colorize regexp groups, character classes, etc (Ruben Bridgewater) #59710
worker:
  * (SEMVER-MAJOR) move terminate callback to end-of-life (James M Snell) #58528

PR-URL: #59896
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
RafaelGSS added a commit that referenced this pull request Oct 13, 2025
@RafaelGSS
2025-10-15, Version 25.0.0 (Current)
0fbf580 
assert:
  * (SEMVER-MAJOR) move assert.fail with multiple arguments to eol (James M Snell) #58532
  * (SEMVER-MAJOR) move CallTracker to EOL (James M Snell) #58006
assert,util:
  * (SEMVER-MAJOR) fail promise comparison in deep equal checks (Ruben Bridgewater) #59448
  * (SEMVER-MAJOR) handle invalid dates as equal in deep comparison (Ruben Bridgewater) #57627
async_hooks:
  * (SEMVER-MAJOR) move `asyncResource` property on bound function to EOL (James M Snell) #58618
buffer:
  * (SEMVER-MAJOR) move SlowBuffer to EOL (Filip Skokan) #58220
build:
  * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #59805
  * (SEMVER-MAJOR) bump minimum Clang version to 19 (Michaël Zasso) #59048
  * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #58064
  * (SEMVER-MAJOR) stop distributing Corepack (Antoine du Hamel) #57617
child_process:
  * (SEMVER-MAJOR) move _channel to end-of-life (James M Snell) #58527
crypto:
  * (SEMVER-MAJOR) runtime-deprecate default shake128/256 output lengths (Filip Skokan) #59008
  * (SEMVER-MAJOR) move deprecated hash and mgf1Hash options to EOL (James M Snell) #58706
  * (SEMVER-MAJOR) runtime deprecate ECDH.setPublicKey() (James M Snell) #58620
deps:
  * (SEMVER-MAJOR) patch V8 for illumos (Dan McDonald) #59805
  * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #54077
  * (SEMVER-MAJOR) update V8 to 14.1.146.11 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) V8: backport 1d3362c55396 (Shu-yu Guo) #58064
  * (SEMVER-MAJOR) V8: cherry-pick 4f38995c8295 (Shu-yu Guo) #58064
  * (SEMVER-MAJOR) V8: cherry-pick 044b9b6f589d (Rezvan Mahdavi Hezaveh) #58064
  * (SEMVER-MAJOR) V8: cherry-pick d2ad518a0b57 (Joyee Cheung) #58064
  * (SEMVER-MAJOR) V8: revert 6d6c1e680c7b (Michaël Zasso) #58064
  * (SEMVER-MAJOR) V8: revert e3cddbedb205 (Michaël Zasso) #58064
  * (SEMVER-MAJOR) use std::map in MSVC STL for EphemeronRememberedSet (Joyee Cheung) #58064
  * (SEMVER-MAJOR) patch V8 for illumos (Dan McDonald) #58064
  * (SEMVER-MAJOR) remove problematic comment from v8-internal (Michaël Zasso) #58064
  * (SEMVER-MAJOR) define V8_PRESERVE_MOST as no-op on Windows (Stefan Stojanovic) #58064
  * (SEMVER-MAJOR) fix FP16 bitcasts.h (Stefan Stojanovic) #58064
  * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #58064
  * (SEMVER-MAJOR) update V8 to 13.7.152.9 (Michaël Zasso) #58064
dgram:
  * (SEMVER-MAJOR) move deprecated APIs to EOL (James M Snell) #58474
dns:
  * (SEMVER-MAJOR) move falsy hostname in lookup to end-of-life (James M Snell) #58619
doc,src,test:
  * (SEMVER-MAJOR) replace use of deprecated `GetIsolate` (Michaël Zasso) #59805
fs:
  * (SEMVER-MAJOR) move FileHandle close on GC to EOL (James M Snell) #58536
  * (SEMVER-MAJOR) move rmdir recursive option to end-of-life (James M Snell) #58616
  * (SEMVER-MAJOR) make `processReadResult()` and `readSyncRecursive()` private (Livia Medeiros) #58672
  * (SEMVER-MAJOR) move fs stream open method to eol (James M Snell) #58529
  * (SEMVER-MAJOR) remove `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #55862
http:
  * (SEMVER-MAJOR) deprecate writeHeader (Sebastian Beltran) #59060
lib:
  * (SEMVER-MAJOR) use validators for argument validation (Nam Yooseong) #59416
  * (SEMVER-MAJOR) expose global ErrorEvent (Richie Bendall) #58920
  * (SEMVER-MAJOR) deprecate `_stream_*` modules (Dario Piotrowicz) #58337
  * (SEMVER-MAJOR) deprecate _tls_common and _tls_wrap (Dario Piotrowicz) #57643
module:
  * (SEMVER-MAJOR) move Module._debug to end-of-life (James M Snell) #58473
node-api:
  * (SEMVER-MAJOR) add warning for NAPI_EXPERIMENTAL (Miguel Marcondes Filho) #58280
perf_hooks:
  * (SEMVER-MAJOR) move deprecated accessors to EOF (James M Snell) #58531
process:
  * (SEMVER-MAJOR) move multipleResolves event to EOL (James M Snell) #58707
repl:
  * (SEMVER-MAJOR) eol deprecate instantiating without new (Aviv Keller) #59495
src:
  * (SEMVER-MAJOR) update crypto.getCipherInfo() to use DictionaryTemplate (James M Snell) #60036
  * (SEMVER-MAJOR) fix calls to v8::Object::wrap (Andreas Haas) #59805
  * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 141 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) improve performance of dotenv ToObject (James M Snell) #60038
  * (SEMVER-MAJOR) use std::string_view from node_report (iknoom) #60006
  * (SEMVER-MAJOR) unflag --experimental-webstorage by default (Daniel M Brasil) #57666
  * (SEMVER-MAJOR) store `Local` for `CallbackScope` on stack (Anna Henningsen) #59705
  * (SEMVER-MAJOR) remove node.h APIs to make callback without an async context (Chengzhong Wu) #58471
  * (SEMVER-MAJOR) remove deprecated node::EmitBeforeExit and node::EmitExit (Chengzhong Wu) #58469
  * (SEMVER-MAJOR) remove deprecated node::CreatePlatform and node::FreePlatform (Chengzhong Wu) #58470
  * (SEMVER-MAJOR) remove deprecated node::InitializeNodeWithArgs (Chengzhong Wu) #58470
  * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 138 (Michaël Zasso) #58064
src,permission:
  * (SEMVER-MAJOR) add --allow-net permission (Rafael Gonzaga) #58517
test:
  * (SEMVER-MAJOR) update cppgc-object addon config (StefanStojanovic) #59805
  * (SEMVER-MAJOR) spin longer for sequential/test-worker-prof (Michaël Zasso) #59805
  * (SEMVER-MAJOR) remove `--always-turbofan` flag (Michaël Zasso) #59805
  * (SEMVER-MAJOR) update snapshot for V8 14.1 (Michaël Zasso) #59805
test,win:
  * (SEMVER-MAJOR) split addon tests (StefanStojanovic) #59805
tls:
  * (SEMVER-MAJOR) move IP-address servername deprecation to eol (James M Snell) #58533
tools:
  * (SEMVER-MAJOR) update V8 gypfiles for 14.0 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) update V8 gypfiles for 13.9 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) update V8 gypfiles for 13.8 (Michaël Zasso) #59805
  * (SEMVER-MAJOR) enable leaptiering for aix/ibmi (Abdirahim Musse) #59805
  * (SEMVER-MAJOR) update V8 gypfiles for 13.7 (Michaël Zasso) #58064
url:
  * (SEMVER-MAJOR) move bad port deprecation in legacy url to end-of-life (James M Snell) #58617
util,console:
  * (SEMVER-MAJOR) colorize regexp groups, character classes, etc (Ruben Bridgewater) #59710
worker:
  * (SEMVER-MAJOR) move terminate callback to end-of-life (James M Snell) #58528

PR-URL: #59896
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BridgeAR BridgeAR BridgeAR left review comments

@jasnell jasnell jasnell approved these changes

@anonrig anonrig anonrig approved these changes

@daeyeon daeyeon daeyeon approved these changes

@aduh95 aduh95 aduh95 approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. child_process Issues and PRs related to the child_process subsystem. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@meteorqz6 @nodejs-github-bot @daeyeon @jasnell @anonrig @BridgeAR @aduh95