watch: fix watch args not being properly filtered
Currently the logic for filtering out watch related flags before passing them to the watch target script is flawed, as it includes any flag (starting with -) that follows a flag starting with --watch (code), this allows some watch flags to make it pass the filtering resulting in the generation of multiple watch processes which is both wasteful and can cause duplicate terminal logs (alongside other issues if the target script is side-effectful).
For example if I run
node --watch --watch index.js
the current implementation will spawn a process equivalent to:
node --watch index.js
which will then finally spawn the correct:
node index.js
Here I'm addressing the incorrect filtering so that all the watch flags are correctly filtered out (making the extra process spawning disappear)
Fixes https://github.com/nodejs/node/issues/57124
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 90.27%. Comparing base (
25842c5) to head (685fb31). Report is 162 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #57936 +/- ##
==========================================
- Coverage 92.26% 90.27% -1.99%
==========================================
Files 325 630 +305
Lines 126673 186112 +59439
Branches 20783 36470 +15687
==========================================
+ Hits 116869 168017 +51148
- Misses 9576 10976 +1400
- Partials 228 7119 +6891
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
CI: https://ci.nodejs.org/job/node-test-pull-request/66587/
Landed in 4acb85403950320773352ab127bee9fc85818153
Actually, this PR landed despite failing many platforms in the test file it appends to: https://ci.nodejs.org/job/node-test-pull-request/66587/
Sending a revert since it should not have landed https://github.com/nodejs/node/pull/58190
Actually, this PR landed despite failing many platforms in the test file it appends to: https://ci.nodejs.org/job/node-test-pull-request/66587/
Note: I've addressed those issues in https://github.com/nodejs/node/pull/58183, I wonder if there were actually other issues as well :thinking: