Skip to content

fix: restore yargs-parser compatible argument parsing - #913

Merged
mcollina merged 2 commits into
mainfrom
fix/parse-args-yargs-compat
Sep 8, 2026
Merged

fix: restore yargs-parser compatible argument parsing#913
mcollina merged 2 commits into
mainfrom
fix/parse-args-yargs-compat

Conversation

@Tony133

@Tony133 Tony133 commented Sep 8, 2026

Copy link
Copy Markdown
Member

Proposal:

The migration to util.parseArgs (f2738f5) dropped several behaviors of yargs-parser:

  • unknown options threw Unknown option — the parser is now non-strict by default (strict only when explicitly true), like yargs-parser. generate-swagger, eject, etc. pass through the main args.js parser with options it does not know (--yaml=true).
  • unknown options did not consume their value (--hello worldhello: true). Now hello: 'world'; a value starting with - is still treated as a flag.
  • booleans could not be set to false: --watch=false, --watch false, --watch 0, -w false are now honored. util.parseArgs cannot express --bool=false, so the normalizer collects them and applies them after parsing.
  • everything after -- was being re-normalized; it is now passed through untouched, so '--' and plugin options match the input.
  • repeated string options (-r a -r b, --import) kept only the last value; start.js then crashed with opts.require.forEach is not a function. They are collected into arrays again from tokens.
  • a string argv is accepted again (with a quote-aware tokenizer); helper.build('./plugin.js -- --hello world') relied on it.
  • test/args.test.js restores hello: 'world' (the migration commit had changed the expectation to hello: true) and adds tests for each case above.

Test runner:

suite-runner.js never ran a single test: glob was bumped to v13 (#789) and the callback API no longer exists, so glob(pattern, cb) resolved a promise nobody awaited and the process exited 0. That is why CI stayed green through the regressions above. The runner now awaits the promise, exits 1 when no file matches, and ignores node_modules and the test/workdir* directories left behind by the generate tests.

With tests actually running, two more environment issues showed up:

  • pkg-up@5, is-docker@4 and chalk@6 (bumped in chore: bump the dependencies group with 3 updates #896) are ESM-only. They are loaded through require(esm) (.default / named export); the is-docker mocks in test/start.test.js are adjusted accordingly. Note: this requires Node ≥ 22.12 — if Node 20 must still be supported, these three dependencies need to be pinned to their CJS versions instead.
  • TypeScript 6 no longer includes every @types/* automatically: types: ["node"] is added to the two test tsconfigs.
  • unit:ts-esm fails on Node 24 with --loader ts-node/esm when run() spawns the test files; it now uses module.register() via --import ./test/configs/register-ts-esm.mjs, which also removes the ExperimentalWarning.

Also: generate-plugin.js no longer writes an empty tstyche: {} when the template has none.

Note:

Closes #912

@Tony133
Tony133 marked this pull request as ready for review September 8, 2026 07:48
@Tony133
Tony133 requested a review from mcollina September 8, 2026 07:48
@Tony133 Tony133 changed the title fix: restore yargs-parser compatible argument parsing (#912) fix: restore yargs-parser compatible argument parsing Sep 8, 2026
Co-authored-by: Matteo Collina <hello@matteocollina.com>

@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

@mcollina
mcollina merged commit a81a231 into main Sep 8, 2026
20 checks passed
@mcollina
mcollina deleted the fix/parse-args-yargs-compat branch September 8, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression in CLI argument parsing after yargs-parser migration

2 participants