From 7e80d9134f9d373438d4ab004e4d5f31d465456a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:01:03 +0000 Subject: [PATCH 01/11] Add commander migration plan and initial getCliOptions commander parsing --- commander-migration-plan.md | 178 +++++++++++++++ packages/beachball/package.json | 1 + .../beachball/src/options/getCliOptions.ts | 202 ++++++++++-------- yarn.lock | 1 + 4 files changed, 289 insertions(+), 93 deletions(-) create mode 100644 commander-migration-plan.md diff --git a/commander-migration-plan.md b/commander-migration-plan.md new file mode 100644 index 000000000..a68b2c489 --- /dev/null +++ b/commander-migration-plan.md @@ -0,0 +1,178 @@ +# Plan: migrate CLI option parsing from yargs-parser to commander@14 + +> **This is a temporary planning document.** It should be deleted before the migration is +> considered complete (or moved into a permanent doc/issue if we want to keep it). + +## Goal and scope + +Replace `yargs-parser` with `commander@14` for CLI option parsing in +`packages/beachball/src/options/getCliOptions.ts`. + +Constraints for this migration (per the task): + +- For now, **all options are defined on a single command**. A later change will split up which + options apply to which commands. +- Commander is used **only for option parsing**, not for command dispatch/execution. The existing + `cli.ts` dispatch (switch on `cliOptions.command`) stays as-is. +- The public shape of `getCliOptions` (returns `ParsedOptions['cliOptions']`, including `command`, + `path`, and `_extraPositionalArgs`) must not change. + +The behaviors we must preserve are documented by +`src/__functional__/options/getCliOptions.test.ts`. That test file explicitly notes it exists to +catch "undocumented breaking changes ... likely to commander". This plan maps each behavior to a +commander approach or a workaround. + +## Why this is not a drop-in replacement + +`yargs-parser` is a permissive, schema-light parser: it infers types, expands camelCase, splits +greedy arrays, and accepts arbitrary unknown flags. `commander` is a schema-first parser: every +option is declared up front, and anything not declared is either an error or passed through +untyped. The gaps are all in yargs' permissive behaviors. + +## Behavior mapping (today's yargs behavior -> commander) + +Legend: ✅ native, ⚙️ needs a workaround, ❌ not feasible / propose dropping. + +| # | Behavior (from `getCliOptions.test.ts`) | Example | Commander support | +|---|------------------------------------------|---------|-------------------| +| 1 | Command as first positional (default `change`) | `beachball check` | ✅ `.argument('[command]')` | +| 2 | String option, separate & `=` forms | `--type patch`, `--access=public` | ✅ `--type ` | +| 3 | Number option + reject non-numeric | `--depth 1`, `--depth foo` throws | ✅ via `.argParser` coercion | +| 4 | Boolean flag | `--fetch` | ✅ `--fetch` | +| 5 | Negated boolean | `--no-fetch` | ✅ define `--no-fetch` alongside `--fetch` | +| 6 | Array: greedy multiple values | `--scope foo bar` | ✅ variadic `` | +| 7 | Array: repeated flag | `--scope foo --scope bar` | ⚙️ variadic + collector fn | +| 8 | Array: single value via `=` becomes array | `--scope=foo` -> `['foo']` | ⚙️ collector fn | +| 9 | Commas NOT split | `--scope a,b` -> `['a,b']` | ✅ (commander never splits) | +| 10 | Boolean value as separate token | `--yes false`, `-y false` | ⚙️ argv preprocessing (see below) | +| 11 | Boolean value via `=` | `--fetch=false`, `--fetch=true` | ⚙️ argv preprocessing (see below) | +| 12 | Throw if non-array option repeated | `--tag a --tag b` throws | ⚙️ collector-style detector fn | +| 13 | camelCase accepted for dashed option | `--gitTags`, `--dependentChangeType` | ⚙️ argv normalization pass | +| 14 | dashed accepted for camelCase option | `--git-tags` | ✅ (declare dashed as canonical) | +| 15 | Negated camelCase | `--no-git-tags` (option `gitTags`) | ✅ declare `--no-git-tags` | +| 16 | Short aliases | `-t`, `-r`, `-y`, `-a`, `-b`, `-m`, `-p`, `-n`, `-v`, `-h` | ✅ in flags string | +| 17 | Extra long aliases | `--config`, `--force`, `--since` | ⚙️ argv normalization or dup options | +| 18 | Arbitrary unknown string option | `--foo bar`, `--foo=bar` | ⚙️ custom leftover parser | +| 19 | Arbitrary unknown boolean flag | `--foo`, `--no-bar` | ⚙️ custom leftover parser | +| 20 | Unknown value type inference | `--foo true` -> bool, `--foo 1` -> number | ⚙️ custom leftover parser | +| 21 | Unknown repeated -> array | `--foo bar --foo baz` -> `['bar','baz']` | ⚙️ custom leftover parser | +| 22 | `-?` alias for help | `-?` | ❌ commander rejects `?` as a short flag; drop or normalize | +| 23 | `config get ` subcommand args | `config get branch` | ✅ variadic `[extraArgs...]` | +| 24 | canary tag override, `NPM_TOKEN`, branch resolution | (post-parse) | ✅ unchanged post-parse logic | + +## Proposed workarounds + +The cleanest strategy is a small **argv normalization pass** run before `program.parse(...)`, plus +a couple of coercion functions and a **leftover unknown-option parser** run after. This keeps the +commander option declarations clean and localizes the yargs-compatibility shims. + +### A. Array options (#7, #8) — collector function + +Declare array options as variadic (`--scope `) and attach an `argParser` collector that +appends to the previous array. Variadic handles greedy multiple values; the collector handles +repeated flags and makes a single `=` value into a one-element array. (Implemented in step 1.) + +### B. Non-array "specified multiple times" error (#12) + +Attach an `argParser` to each string/number option that throws if it receives a value when one was +already set (commander calls the parser with the previous value as the second argument). This +reproduces yargs' "Option X only accepts a single value" error. + +### C. Boolean values as tokens or `=` (#10, #11) + +Commander boolean flags don't accept values (`--fetch=false` / `--yes false` don't work natively; +optional-value `[value]` options don't reliably consume a following token either). Proposed +workaround: an **argv normalization pass** that, for known boolean options, rewrites: + +- `--fetch=false` / `--fetch=true` -> drop the `=value`, emit `--fetch` or `--no-fetch`. +- `--yes false` / `-y false` (value in the next token, only when the next token is literally + `true`/`false`) -> collapse to `--yes` / `--no-yes` and consume that token. + +Only the literal strings `true`/`false` are treated as boolean values (matching today's behavior); +anything else is left for normal positional handling. + +### D. camelCase flag acceptance (#13) and extra long aliases (#17) + +Add an argv normalization pass that maps recognized alternate flag spellings to the canonical +dashed form before parsing: + +- camelCase -> dashed: `--gitTags` -> `--git-tags`, `--dependentChangeType` -> `--dependent-change-type`. + Build the lookup from the known option lists (we already generate the dashed form from the + camelCase name). +- long aliases -> canonical: `--config` -> `--config-path`, `--force` -> `--force-versions`, + `--since` -> `--from-ref`. Maintain a small alias map. (Commander only allows one long flag per + option, so aliases must be normalized rather than declared.) + +Handle both `--flag value` and `--flag=value` shapes in the normalizer. + +### E. Arbitrary unknown options (#18–#21) + +Today yargs collects any unknown `--foo`/`--foo=bar` into `cliOptions` with type inference (boolean +for bare flags and `true`/`false`, number for numeric strings, string otherwise, array when +repeated). Commander with `.allowUnknownOption()` just passes unknown tokens through as raw args — +it does not key/value-parse them, and their presence disrupts positional (command) detection. + +Proposed workaround: run a **small dedicated mini-parser over the leftover unknown tokens** +(`program.parseOptions(...)` exposes `unknown`, or we diff declared flags from argv). For each +unknown token, reproduce yargs' inference rules: + +- `--foo` (no value / next token is another flag) -> `true`; `--no-bar` -> `bar: false`. +- `--foo=bar` or `--foo bar` -> string `bar`; `true`/`false` -> boolean; numeric -> number. +- repeated unknown -> array. + +Keep this logic small and well-tested; it is the largest single behavioral gap. Note the existing +test at line 240 already documents that `--foo bar baz` treats `baz` as the command (greedy arrays +are not applied to unknown options), so the mini-parser should mirror that. + +### F. Positional / command detection (#1, #23) + +Declare `[command]` and `[extraArgs...]` positional arguments. Keep the existing validation that +only `config` may have extra positional args, and keep populating `_extraPositionalArgs`. Because +unknown-option handling (E) can interfere with positional detection, run the normalizer (D) and the +boolean fixups (C) first, then let commander parse known options + positionals, then run the +leftover unknown parser (E) on what remains. + +### G. Error / exit behavior + +Call `program.exitOverride()` and silence `configureOutput` so commander throws instead of calling +`process.exit()` / writing to stderr. This respects the repo rule that `process.exit()` only +belongs in `cli.ts`, and lets tests assert on thrown errors. (Implemented in step 1.) + +## Items proposed to drop or change (need sign-off) + +- **`-?` as a help alias (#22):** commander does not accept `?` as a short-flag character. Options: + (a) drop `-?` (recommended — `-h`/`--help` remain), or (b) rewrite `-?` to `--help` in the + normalizer. This is a minor, arguably-intentional breaking change; call it out in the change file + since `main` targets beachball v3 where breaking changes are allowed. +- **Greedy arrays for unknown options:** not supported today either (test #240), so no change. + +## Implementation steps + +1. **(this PR) Basic commander definitions + parsing, dashed forms only.** + - Add `commander@^14.0.3` dependency. + - Rewrite `getCliOptions.ts` to build a single commander `Command` with every option declared in + its canonical dashed form (string ``, number w/ numeric coercion, boolean w/ `--no-` + negation, array variadic + collector), short aliases included, plus `[command]`/`[extraArgs...]` + positionals. Keep all post-parse logic (branch resolution, canary tag, `NPM_TOKEN`, delete + undefined, `_extraPositionalArgs`, project-root lookup). + - Tests are expected to partially fail at this step (the permissive-syntax cases): camelCase + flags, extra long aliases, boolean values as tokens/`=`, "repeated non-array throws", and + arbitrary unknown-option inference. No change file yet. +2. **Non-array repeat detection (B)** and confirm array collector (A) matches all array tests. +3. **camelCase + long-alias normalization pass (D).** +4. **Boolean-value normalization (C)** and resolve the `-?` decision (G/#22). +5. **Unknown-option mini-parser (E)** to restore arbitrary-option inference and fix positional + interaction (F). +6. **Cleanup:** remove `yargs-parser` and `@types/yargs-parser` from `packages/beachball/package.json`, + remove the now-obsolete `parserOptions`/`allKeysOfType` machinery, run `yarn lint` (dep check), + `yarn build`, `yarn test`, `yarn format`. +7. **Docs + change file:** update any option docs/help text if behavior changed (e.g. `-?`), add a + Beachball change file via `/beachball-change-file`, and delete this planning document. + +## Testing strategy + +- `getCliOptions.test.ts` is the primary contract; drive the migration to green against it, only + editing tests where we deliberately change behavior (e.g. `-?`), with each such change justified + in the change file. +- Add focused unit tests for the new helpers (argv normalizer, boolean fixup, unknown mini-parser). +- Run the full `beachball` suite (`yarn test:all`) since option parsing feeds every command. diff --git a/packages/beachball/package.json b/packages/beachball/package.json index 30a68b024..ff3587255 100644 --- a/packages/beachball/package.json +++ b/packages/beachball/package.json @@ -36,6 +36,7 @@ }, "dependencies": { "@vercel/detect-agent": "^1.2.1", + "commander": "^14.0.3", "cosmiconfig": "^9.0.1", "execa": "^5.1.1", "minimatch": "^3.1.5", diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index c5645405c..26aaf9c27 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -1,5 +1,5 @@ import { findProjectRoot } from 'workspace-tools'; -import parser from 'yargs-parser'; +import { Command, Option } from 'commander'; import { env } from '../env'; import type { CliOptions, ParsedOptions } from '../types/BeachballOptions'; import { cacheRemoteBranch } from '../git/getRemoteBranch'; @@ -22,7 +22,15 @@ export interface ProcessInfo { env: NodeJS.ProcessEnv | { NPM_TOKEN?: string }; } -// For camelCased options, yargs will automatically accept them with-dashes too. +// NOTE: This file is being migrated from yargs-parser to commander@14. For now, all options are +// defined on a single command, and commander is used only for option parsing (not for dispatching +// to command implementations). A later change will split up which options apply to which commands. +// +// This first step defines each option in its canonical dashed form only. Additional forms that +// yargs-parser accepted today (camelCase flags, extra long aliases, boolean values passed as a +// separate token, arbitrary unknown options, "specified multiple times" errors, etc.) are not yet +// handled here; see the migration plan for the proposed workarounds. + const arrayOptions = ['disallowedChangeTypes', 'package', 'scope'] as const; const booleanOptions = [ 'all', @@ -61,70 +69,92 @@ const stringOptions = [ 'type', ] as const; -type AtLeastOne = [T, ...T[]]; -/** Type hack to verify that an array includes all keys of a type */ -const allKeysOfType = - () => - >( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...x: L extends any ? (Exclude extends never ? L : Exclude[]) : never - ) => - x; - -// Verify that all the known CLI options have types specified, to ensure correct parsing. -// -// NOTE: If a prop is missing, this will have a somewhat misleading error: -// Argument of type '"disallowedChangeTypes"' is not assignable to parameter of type '"tag" | "version"' -// -// To fix, add the missing names after "parameter of type" ("tag" and "version" in this example) -// to the appropriate array above. -const knownOptions = allKeysOfType()( - ...arrayOptions, - ...booleanOptions, - ...numberOptions, - ...stringOptions, - // these options are filled in below, not respected from the command line - 'path', - 'command', - '_extraPositionalArgs' -); - -const parserOptions: parser.Options = { - configuration: { - 'boolean-negation': true, - 'camel-case-expansion': true, - 'dot-notation': false, - 'duplicate-arguments-array': true, - 'flatten-duplicate-arrays': true, - 'greedy-arrays': true, // for now; we might want to change this to false in the future - 'parse-numbers': true, - 'parse-positional-numbers': false, - 'short-option-groups': false, - 'strip-aliased': true, - 'strip-dashed': true, - }, - // spread to get rid of readonly... - array: [...arrayOptions], - boolean: [...booleanOptions], - number: [...numberOptions], - string: [...stringOptions], - alias: { - authType: ['a'], - branch: ['b'], - configPath: ['c', 'config'], - forceVersions: ['force'], - fromRef: ['since'], - help: ['h', '?'], - message: ['m'], - package: ['p'], - registry: ['r'], - tag: ['t'], - token: ['n'], - version: ['v'], - yes: ['y'], - }, +/** Short single-character aliases for certain options (option name => short flag without dash). */ +const shortAliases: Partial> = { + authType: 'a', + branch: 'b', + configPath: 'c', + help: 'h', + message: 'm', + package: 'p', + registry: 'r', + tag: 't', + token: 'n', + version: 'v', + yes: 'y', }; +/** Convert a camelCase option name to its dashed CLI flag form (e.g. `gitTags` => `git-tags`). */ +function toDashed(name: string): string { + return name.replace(/[A-Z]/g, letter => `-${letter.toLowerCase()}`); +} + +/** Coerce a value to a number, throwing if it's not numeric (matches previous yargs behavior). */ +function parseNumber(name: string): (value: string) => number { + return (value: string) => { + const num = Number(value); + if (isNaN(num)) { + throw new Error(`Non-numeric value passed for numeric option "${name}"`); + } + return num; + }; +} + +/** Collector for array options: accumulate repeated/variadic values into a single array. */ +function collectArray(value: string, previous: string[] | undefined): string[] { + return previous ? [...previous, value] : [value]; +} + +/** + * Build a commander `Command` with all beachball options defined (dashed forms only for now). + * Commander is currently used only for parsing, so this single command holds every option. + */ +function buildCommand(): Command { + const program = new Command(); + + // Throw instead of calling process.exit() or writing to stdout/stderr on error, so callers and + // tests can handle failures. + program.exitOverride(); + program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); + + // The command name and any extra positional args (e.g. `config get `). + program.argument('[command]', 'beachball command to run'); + program.argument('[extraArgs...]', 'extra positional arguments (e.g. for `config get `)'); + + // Allow options we haven't explicitly defined to pass through without erroring. + // (Full parsing of arbitrary unknown options is handled in a later step; see the plan.) + program.allowUnknownOption(); + program.allowExcessArguments(); + + const flags = (name: string, valuePlaceholder?: string): string => { + const dashed = toDashed(name); + const short = shortAliases[name as keyof CliOptions]; + const long = valuePlaceholder ? `--${dashed} ${valuePlaceholder}` : `--${dashed}`; + return short ? `-${short}, ${long}` : long; + }; + + for (const name of stringOptions) { + program.addOption(new Option(flags(name, ''))); + } + + for (const name of numberOptions) { + program.addOption(new Option(flags(name, '')).argParser(parseNumber(name))); + } + + for (const name of arrayOptions) { + // Variadic to allow multiple space-separated values, plus a collector for repeated usage. + program.addOption(new Option(flags(name, '')).argParser(collectArray)); + } + + for (const name of booleanOptions) { + program.addOption(new Option(flags(name))); + // Negated form (e.g. `--no-fetch`). + program.addOption(new Option(`--no-${toDashed(name)}`)); + } + + return program; +} + /** * Gets CLI options. Also gets the `NPM_TOKEN` environment variable if present. */ @@ -140,9 +170,12 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti // Be careful not to mutate the input argv const trimmedArgv = processInfo.argv.slice(2); - const args = parser(trimmedArgv, parserOptions); + const program = buildCommand(); + program.parse(trimmedArgv, { from: 'user' }); + + const options = program.opts(); + const [command, extraPositionalArgs = []] = program.processedArgs as [string | undefined, string[] | undefined]; - const { _: positionalArgs, ...options } = args; let cwd = processInfo.cwd; try { // If a non-empty cwd is provided, find the project root from there. @@ -154,22 +187,20 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti // use the provided cwd } - if (positionalArgs.length > 1 && String(positionalArgs[0]) !== 'config') { - throw new Error(`Only one positional argument (the command) is allowed. Received: ${positionalArgs.join(' ')}`); + if (extraPositionalArgs.length && command !== 'config') { + throw new Error( + `Only one positional argument (the command) is allowed. Received: ${[command, ...extraPositionalArgs].join(' ')}` + ); } const cliOptions: ParsedOptions['cliOptions'] = { ...options, - command: positionalArgs.length ? String(positionalArgs[0]) : 'change', + command: command || 'change', path: cwd, }; - // Save extra positional args for commands that support subcommands (e.g. 'config get ') - // (yargs-parser doesn't support positional arguments directly) - const extraPositionalArgs = positionalArgs.length > 1 ? positionalArgs.slice(1).map(String) : undefined; - - if (args.branch) { - cliOptions.branch = resolveBranchOption(args as Partial>, cwd); + if (cliOptions.branch) { + cliOptions.branch = resolveBranchOption(cliOptions as Partial>, cwd); } if (cliOptions.command === 'canary') { @@ -177,28 +208,13 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti } for (const key of Object.keys(cliOptions) as (keyof CliOptions)[]) { - const value = cliOptions[key]; - if (value === undefined) { + if (cliOptions[key] === undefined) { delete cliOptions[key]; - } else if (typeof value === 'number' && isNaN(value)) { - throw new Error(`Non-numeric value passed for numeric option "${key}"`); - } else if (knownOptions.includes(key)) { - if (Array.isArray(value) && !arrayOptions.includes(key as (typeof arrayOptions)[number])) { - throw new Error(`Option "${key}" only accepts a single value. Received: ${value.join(' ')}`); - } - } else if (value === 'true') { - // For unknown arguments like --foo=true or --bar=false, yargs will handle the value as a string. - // Convert it to a boolean to avoid subtle bugs. - // eslint-disable-next-line - (cliOptions as any)[key] = true; - } else if (value === 'false') { - // eslint-disable-next-line - (cliOptions as any)[key] = false; } } - // Set extra positional args after the validation loop (it's an internal array, not from CLI parsing) - if (extraPositionalArgs) { + // Save extra positional args for commands that support subcommands (e.g. 'config get '). + if (extraPositionalArgs.length) { cliOptions._extraPositionalArgs = extraPositionalArgs; } diff --git a/yarn.lock b/yarn.lock index d86d84198..df4d62cce 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3117,6 +3117,7 @@ __metadata: "@types/yargs-parser": "npm:^21.0.0" "@vercel/detect-agent": "npm:^1.2.1" "@verdaccio/types": "npm:^13.0.0" + commander: "npm:^14.0.3" cosmiconfig: "npm:^9.0.1" cross-env: "npm:^10.1.0" execa: "npm:^5.1.1" From 0928823e1c22be7aa2f46a94159100726884d32b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:02:56 +0000 Subject: [PATCH 02/11] Address review feedback: Number.isNaN, output-suppression comment, plan wording --- commander-migration-plan.md | 5 +++-- packages/beachball/src/options/getCliOptions.ts | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/commander-migration-plan.md b/commander-migration-plan.md index a68b2c489..91726b527 100644 --- a/commander-migration-plan.md +++ b/commander-migration-plan.md @@ -1,7 +1,8 @@ # Plan: migrate CLI option parsing from yargs-parser to commander@14 -> **This is a temporary planning document.** It should be deleted before the migration is -> considered complete (or moved into a permanent doc/issue if we want to keep it). +> **This is a temporary planning document.** It should be deleted as part of the final cleanup +> (step 7 below), once the migration is complete (or moved into a permanent doc/issue if we want to +> keep it). ## Goal and scope diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index 26aaf9c27..75842918b 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -93,7 +93,7 @@ function toDashed(name: string): string { function parseNumber(name: string): (value: string) => number { return (value: string) => { const num = Number(value); - if (isNaN(num)) { + if (Number.isNaN(num)) { throw new Error(`Non-numeric value passed for numeric option "${name}"`); } return num; @@ -115,7 +115,7 @@ function buildCommand(): Command { // Throw instead of calling process.exit() or writing to stdout/stderr on error, so callers and // tests can handle failures. program.exitOverride(); - program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); + program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); // suppress commander output // The command name and any extra positional args (e.g. `config get `). program.argument('[command]', 'beachball command to run'); From 6fdf6886f3e4ef6ae2c5c6aa928a268de5721e4c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:13:39 +0000 Subject: [PATCH 03/11] Use commander sub-commands for CLI parsing (options on parent + each command) --- .../beachball/src/options/getCliOptions.ts | 115 ++++++++++++------ 1 file changed, 77 insertions(+), 38 deletions(-) diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index 75842918b..7459982c8 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -1,5 +1,5 @@ import { findProjectRoot } from 'workspace-tools'; -import { Command, Option } from 'commander'; +import { Command, Option, type OptionValues } from 'commander'; import { env } from '../env'; import type { CliOptions, ParsedOptions } from '../types/BeachballOptions'; import { cacheRemoteBranch } from '../git/getRemoteBranch'; @@ -22,15 +22,25 @@ export interface ProcessInfo { env: NodeJS.ProcessEnv | { NPM_TOKEN?: string }; } -// NOTE: This file is being migrated from yargs-parser to commander@14. For now, all options are -// defined on a single command, and commander is used only for option parsing (not for dispatching -// to command implementations). A later change will split up which options apply to which commands. +// NOTE: This file is being migrated from yargs-parser to commander@14. Commander is currently used +// only for option parsing (not for dispatching to command implementations). // -// This first step defines each option in its canonical dashed form only. Additional forms that +// Options are defined on proper sub-commands, one per beachball command. For now, every option is +// added to each sub-command as well as to the parent command, so options can be specified either +// before the command (as "global" options on the parent) or after it (on the sub-command). A later +// change will split up which options apply to which commands. +// +// This step defines each option in its canonical dashed form only. Additional forms that // yargs-parser accepted today (camelCase flags, extra long aliases, boolean values passed as a // separate token, arbitrary unknown options, "specified multiple times" errors, etc.) are not yet // handled here; see the migration plan for the proposed workarounds. +/** All beachball commands. Each becomes a commander sub-command. */ +const commands = ['change', 'check', 'publish', 'bump', 'canary', 'sync', 'init', 'config', 'migrate'] as const; + +/** Command run when none is specified on the command line. */ +const defaultCommand = 'change'; + const arrayOptions = ['disallowedChangeTypes', 'package', 'scope'] as const; const booleanOptions = [ 'all', @@ -105,27 +115,8 @@ function collectArray(value: string, previous: string[] | undefined): string[] { return previous ? [...previous, value] : [value]; } -/** - * Build a commander `Command` with all beachball options defined (dashed forms only for now). - * Commander is currently used only for parsing, so this single command holds every option. - */ -function buildCommand(): Command { - const program = new Command(); - - // Throw instead of calling process.exit() or writing to stdout/stderr on error, so callers and - // tests can handle failures. - program.exitOverride(); - program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); // suppress commander output - - // The command name and any extra positional args (e.g. `config get `). - program.argument('[command]', 'beachball command to run'); - program.argument('[extraArgs...]', 'extra positional arguments (e.g. for `config get `)'); - - // Allow options we haven't explicitly defined to pass through without erroring. - // (Full parsing of arbitrary unknown options is handled in a later step; see the plan.) - program.allowUnknownOption(); - program.allowExcessArguments(); - +/** Add every beachball option (dashed forms only for now) to the given command. */ +function addAllOptions(command: Command): void { const flags = (name: string, valuePlaceholder?: string): string => { const dashed = toDashed(name); const short = shortAliases[name as keyof CliOptions]; @@ -134,25 +125,75 @@ function buildCommand(): Command { }; for (const name of stringOptions) { - program.addOption(new Option(flags(name, ''))); + command.addOption(new Option(flags(name, ''))); } for (const name of numberOptions) { - program.addOption(new Option(flags(name, '')).argParser(parseNumber(name))); + command.addOption(new Option(flags(name, '')).argParser(parseNumber(name))); } for (const name of arrayOptions) { // Variadic to allow multiple space-separated values, plus a collector for repeated usage. - program.addOption(new Option(flags(name, '')).argParser(collectArray)); + command.addOption(new Option(flags(name, '')).argParser(collectArray)); } for (const name of booleanOptions) { - program.addOption(new Option(flags(name))); + command.addOption(new Option(flags(name))); // Negated form (e.g. `--no-fetch`). - program.addOption(new Option(`--no-${toDashed(name)}`)); + command.addOption(new Option(`--no-${toDashed(name)}`)); + } +} + +/** Result captured from whichever sub-command commander dispatches to. */ +interface ParseResult { + command: string; + options: OptionValues; + extraArgs: string[]; +} + +/** + * Build a commander program with one sub-command per beachball command. All options are added to + * both the parent (so they can be given before the command) and each sub-command (so they can be + * given after it). Commander is currently used only for parsing, not command dispatch. + * + * @returns The program plus a getter for the parse result (populated by the matched sub-command's + * action handler when `program.parse()` is called). + */ +function buildProgram(): { program: Command; getResult: () => ParseResult } { + const program = new Command(); + + // Throw instead of calling process.exit() or writing to stdout/stderr on error, so callers and + // tests can handle failures. + program.exitOverride(); + program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); // suppress commander output + + // Allow options we haven't explicitly defined to pass through without erroring. + // (Full parsing of arbitrary unknown options is handled in a later step; see the plan.) + program.allowUnknownOption(); + + // Add all options to the parent so they can be specified before the command name. + addAllOptions(program); + + let result: ParseResult = { command: defaultCommand, options: {}, extraArgs: [] }; + + for (const name of commands) { + const subcommand = program.command(name, { isDefault: name === defaultCommand }); + addAllOptions(subcommand); + subcommand.allowUnknownOption(); + // Capture any extra positional args (e.g. `config get `). A consistent error for + // non-config commands with extra args is produced later in getCliOptions. + subcommand.argument('[extraArgs...]', 'extra positional arguments (e.g. for `config get `)'); + subcommand.action(() => { + result = { + command: name, + // Merge parent ("global") options with this sub-command's options. + options: subcommand.optsWithGlobals(), + extraArgs: (subcommand.processedArgs[0] as string[] | undefined) ?? [], + }; + }); } - return program; + return { program, getResult: () => result }; } /** @@ -170,11 +211,9 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti // Be careful not to mutate the input argv const trimmedArgv = processInfo.argv.slice(2); - const program = buildCommand(); + const { program, getResult } = buildProgram(); program.parse(trimmedArgv, { from: 'user' }); - - const options = program.opts(); - const [command, extraPositionalArgs = []] = program.processedArgs as [string | undefined, string[] | undefined]; + const { command, options, extraArgs: extraPositionalArgs } = getResult(); let cwd = processInfo.cwd; try { @@ -195,12 +234,12 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti const cliOptions: ParsedOptions['cliOptions'] = { ...options, - command: command || 'change', + command, path: cwd, }; if (cliOptions.branch) { - cliOptions.branch = resolveBranchOption(cliOptions as Partial>, cwd); + cliOptions.branch = resolveBranchOption(cliOptions, cwd); } if (cliOptions.command === 'canary') { From 277c413805f074e290ff9e0379cdddfb2c683d79 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:15:14 +0000 Subject: [PATCH 04/11] Clarify processedArgs access with an explanatory comment --- packages/beachball/src/options/getCliOptions.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index 7459982c8..ed19bc438 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -188,6 +188,9 @@ function buildProgram(): { program: Command; getResult: () => ParseResult } { command: name, // Merge parent ("global") options with this sub-command's options. options: subcommand.optsWithGlobals(), + // processedArgs is positional in declaration order; index 0 is the `[extraArgs...]` + // variadic declared above, which commander populates as a string array (or undefined + // when no extra args were given). extraArgs: (subcommand.processedArgs[0] as string[] | undefined) ?? [], }; }); From 641db7e66752021e2c570898c531f43146360564 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:32:15 +0000 Subject: [PATCH 05/11] Complete commander migration: normalization, unknown-option parsing, cleanup --- commander-migration-plan.md | 52 +-- packages/beachball/package.json | 4 +- .../beachball/src/options/getCliOptions.ts | 299 +++++++++++++++--- .../beachball/src/types/BeachballOptions.ts | 1 - yarn.lock | 2 - 5 files changed, 281 insertions(+), 77 deletions(-) diff --git a/commander-migration-plan.md b/commander-migration-plan.md index 91726b527..1b00313e6 100644 --- a/commander-migration-plan.md +++ b/commander-migration-plan.md @@ -34,32 +34,32 @@ untyped. The gaps are all in yargs' permissive behaviors. Legend: ✅ native, ⚙️ needs a workaround, ❌ not feasible / propose dropping. -| # | Behavior (from `getCliOptions.test.ts`) | Example | Commander support | -|---|------------------------------------------|---------|-------------------| -| 1 | Command as first positional (default `change`) | `beachball check` | ✅ `.argument('[command]')` | -| 2 | String option, separate & `=` forms | `--type patch`, `--access=public` | ✅ `--type ` | -| 3 | Number option + reject non-numeric | `--depth 1`, `--depth foo` throws | ✅ via `.argParser` coercion | -| 4 | Boolean flag | `--fetch` | ✅ `--fetch` | -| 5 | Negated boolean | `--no-fetch` | ✅ define `--no-fetch` alongside `--fetch` | -| 6 | Array: greedy multiple values | `--scope foo bar` | ✅ variadic `` | -| 7 | Array: repeated flag | `--scope foo --scope bar` | ⚙️ variadic + collector fn | -| 8 | Array: single value via `=` becomes array | `--scope=foo` -> `['foo']` | ⚙️ collector fn | -| 9 | Commas NOT split | `--scope a,b` -> `['a,b']` | ✅ (commander never splits) | -| 10 | Boolean value as separate token | `--yes false`, `-y false` | ⚙️ argv preprocessing (see below) | -| 11 | Boolean value via `=` | `--fetch=false`, `--fetch=true` | ⚙️ argv preprocessing (see below) | -| 12 | Throw if non-array option repeated | `--tag a --tag b` throws | ⚙️ collector-style detector fn | -| 13 | camelCase accepted for dashed option | `--gitTags`, `--dependentChangeType` | ⚙️ argv normalization pass | -| 14 | dashed accepted for camelCase option | `--git-tags` | ✅ (declare dashed as canonical) | -| 15 | Negated camelCase | `--no-git-tags` (option `gitTags`) | ✅ declare `--no-git-tags` | -| 16 | Short aliases | `-t`, `-r`, `-y`, `-a`, `-b`, `-m`, `-p`, `-n`, `-v`, `-h` | ✅ in flags string | -| 17 | Extra long aliases | `--config`, `--force`, `--since` | ⚙️ argv normalization or dup options | -| 18 | Arbitrary unknown string option | `--foo bar`, `--foo=bar` | ⚙️ custom leftover parser | -| 19 | Arbitrary unknown boolean flag | `--foo`, `--no-bar` | ⚙️ custom leftover parser | -| 20 | Unknown value type inference | `--foo true` -> bool, `--foo 1` -> number | ⚙️ custom leftover parser | -| 21 | Unknown repeated -> array | `--foo bar --foo baz` -> `['bar','baz']` | ⚙️ custom leftover parser | -| 22 | `-?` alias for help | `-?` | ❌ commander rejects `?` as a short flag; drop or normalize | -| 23 | `config get ` subcommand args | `config get branch` | ✅ variadic `[extraArgs...]` | -| 24 | canary tag override, `NPM_TOKEN`, branch resolution | (post-parse) | ✅ unchanged post-parse logic | +| # | Behavior (from `getCliOptions.test.ts`) | Example | Commander support | +| --- | --------------------------------------------------- | ---------------------------------------------------------- | ----------------------------------------------------------- | +| 1 | Command as first positional (default `change`) | `beachball check` | ✅ `.argument('[command]')` | +| 2 | String option, separate & `=` forms | `--type patch`, `--access=public` | ✅ `--type ` | +| 3 | Number option + reject non-numeric | `--depth 1`, `--depth foo` throws | ✅ via `.argParser` coercion | +| 4 | Boolean flag | `--fetch` | ✅ `--fetch` | +| 5 | Negated boolean | `--no-fetch` | ✅ define `--no-fetch` alongside `--fetch` | +| 6 | Array: greedy multiple values | `--scope foo bar` | ✅ variadic `` | +| 7 | Array: repeated flag | `--scope foo --scope bar` | ⚙️ variadic + collector fn | +| 8 | Array: single value via `=` becomes array | `--scope=foo` -> `['foo']` | ⚙️ collector fn | +| 9 | Commas NOT split | `--scope a,b` -> `['a,b']` | ✅ (commander never splits) | +| 10 | Boolean value as separate token | `--yes false`, `-y false` | ⚙️ argv preprocessing (see below) | +| 11 | Boolean value via `=` | `--fetch=false`, `--fetch=true` | ⚙️ argv preprocessing (see below) | +| 12 | Throw if non-array option repeated | `--tag a --tag b` throws | ⚙️ collector-style detector fn | +| 13 | camelCase accepted for dashed option | `--gitTags`, `--dependentChangeType` | ⚙️ argv normalization pass | +| 14 | dashed accepted for camelCase option | `--git-tags` | ✅ (declare dashed as canonical) | +| 15 | Negated camelCase | `--no-git-tags` (option `gitTags`) | ✅ declare `--no-git-tags` | +| 16 | Short aliases | `-t`, `-r`, `-y`, `-a`, `-b`, `-m`, `-p`, `-n`, `-v`, `-h` | ✅ in flags string | +| 17 | Extra long aliases | `--config`, `--force`, `--since` | ⚙️ argv normalization or dup options | +| 18 | Arbitrary unknown string option | `--foo bar`, `--foo=bar` | ⚙️ custom leftover parser | +| 19 | Arbitrary unknown boolean flag | `--foo`, `--no-bar` | ⚙️ custom leftover parser | +| 20 | Unknown value type inference | `--foo true` -> bool, `--foo 1` -> number | ⚙️ custom leftover parser | +| 21 | Unknown repeated -> array | `--foo bar --foo baz` -> `['bar','baz']` | ⚙️ custom leftover parser | +| 22 | `-?` alias for help | `-?` | ❌ commander rejects `?` as a short flag; drop or normalize | +| 23 | `config get ` subcommand args | `config get branch` | ✅ variadic `[extraArgs...]` | +| 24 | canary tag override, `NPM_TOKEN`, branch resolution | (post-parse) | ✅ unchanged post-parse logic | ## Proposed workarounds diff --git a/packages/beachball/package.json b/packages/beachball/package.json index ff3587255..fe117032e 100644 --- a/packages/beachball/package.json +++ b/packages/beachball/package.json @@ -44,8 +44,7 @@ "p-limit": "^3.1.0", "prompts": "^2.4.2", "semver": "^7.7.4", - "workspace-tools": "catalog:prod", - "yargs-parser": "^21.1.1" + "workspace-tools": "catalog:prod" }, "devDependencies": { "@microsoft/beachball-scripts": "workspace:^", @@ -54,7 +53,6 @@ "@types/prompts": "^2.4.2", "@types/semver": "^7.3.13", "@types/tmp": "^0.2.3", - "@types/yargs-parser": "^21.0.0", "@verdaccio/types": "^13.0.0", "cross-env": "^10.1.0", "get-port": "^7.0.0", diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index ed19bc438..baea6d2d8 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -22,21 +22,24 @@ export interface ProcessInfo { env: NodeJS.ProcessEnv | { NPM_TOKEN?: string }; } -// NOTE: This file is being migrated from yargs-parser to commander@14. Commander is currently used -// only for option parsing (not for dispatching to command implementations). +// NOTE: This file was migrated from yargs-parser to commander@14. Commander is currently used only +// for option parsing (not for dispatching to command implementations); the existing `cli.ts` +// dispatch (switch on `cliOptions.command`) is unchanged. // -// Options are defined on proper sub-commands, one per beachball command. For now, every option is -// added to each sub-command as well as to the parent command, so options can be specified either -// before the command (as "global" options on the parent) or after it (on the sub-command). A later -// change will split up which options apply to which commands. +// A single commander command declares every option (so options can be given before or after the +// command name) plus two positional arguments: the command name and any extra positional args +// (e.g. `config get `). A later change will split up which options apply to which commands. // -// This step defines each option in its canonical dashed form only. Additional forms that -// yargs-parser accepted today (camelCase flags, extra long aliases, boolean values passed as a -// separate token, arbitrary unknown options, "specified multiple times" errors, etc.) are not yet -// handled here; see the migration plan for the proposed workarounds. - -/** All beachball commands. Each becomes a commander sub-command. */ -const commands = ['change', 'check', 'publish', 'bump', 'canary', 'sync', 'init', 'config', 'migrate'] as const; +// Each option is declared in its canonical dashed form. Commander is a schema-first parser, so +// several permissive behaviors that yargs-parser accepted are reproduced with small argv +// preprocessing passes (see `normalizeArgv`) plus a mini-parser for arbitrary unknown options +// (`extractUnknownOptions`): +// - camelCase flags (`--gitTags`) and extra long aliases (`--config`, `--force`, `--since`) +// are normalized to their canonical dashed form before parsing. +// - boolean values passed via `=` or as a separate token (`--fetch=false`, `--yes false`) are +// rewritten to commander's flag / `--no-` negation form. +// - arbitrary unknown options are parsed with yargs-like type inference. +// - non-array options specified more than once throw (matching yargs). /** Command run when none is specified on the command line. */ const defaultCommand = 'change'; @@ -94,6 +97,20 @@ const shortAliases: Partial> = { yes: 'y', }; +/** + * Extra long-flag aliases accepted for certain options (alias => canonical option name). + * Commander only allows one long flag per option, so these are normalized before parsing rather + * than declared as real options. + */ +const longAliases: Record = { + config: 'configPath', + force: 'forceVersions', + since: 'fromRef', +}; + +/** All option names (any value type). */ +const allOptionNames = [...arrayOptions, ...booleanOptions, ...numberOptions, ...stringOptions]; + /** Convert a camelCase option name to its dashed CLI flag form (e.g. `gitTags` => `git-tags`). */ function toDashed(name: string): string { return name.replace(/[A-Z]/g, letter => `-${letter.toLowerCase()}`); @@ -110,12 +127,204 @@ function parseNumber(name: string): (value: string) => number { }; } +/** + * Build an `argParser` for a non-array option that throws if the option is specified more than once + * (commander passes the previously-parsed value as the second argument). This matches yargs-parser, + * which errored on repeated single-value options. An optional `coerce` transforms each value. + */ +function parseSingle(name: string, coerce?: (value: string) => T): (value: string, previous: unknown) => T | string { + return (value: string, previous: unknown) => { + if (previous !== undefined) { + throw new Error(`Option "${name}" can only be specified once`); + } + return coerce ? coerce(value) : value; + }; +} + /** Collector for array options: accumulate repeated/variadic values into a single array. */ function collectArray(value: string, previous: string[] | undefined): string[] { return previous ? [...previous, value] : [value]; } -/** Add every beachball option (dashed forms only for now) to the given command. */ +/** Infer the type of an unknown option's value the way yargs-parser did. */ +function inferUnknownValue(value: string): string | number | boolean { + if (value === 'true') return true; + if (value === 'false') return false; + if (value !== '' && !Number.isNaN(Number(value))) return Number(value); + return value; +} + +/** + * Map of alternate long-flag spellings to their canonical dashed flag (without leading `--`). + * Covers camelCase spellings of dashed options (`gitTags` => `git-tags`) and extra long aliases + * (`config` => `config-path`). + */ +const flagAliasMap: Record = (() => { + const map: Record = {}; + for (const name of allOptionNames) { + const dashed = toDashed(name); + if (name !== dashed) { + map[name] = dashed; // camelCase spelling => dashed canonical + } + } + for (const [alias, name] of Object.entries(longAliases)) { + map[alias] = toDashed(name); + } + return map; +})(); + +/** Dashed names of boolean options (e.g. `git-tags`). */ +const booleanDashedSet = new Set(booleanOptions.map(toDashed)); + +/** Short flag character => camelCase option name (e.g. `y` => `yes`). */ +const shortToName: Record = (() => { + const map: Record = {}; + for (const [name, short] of Object.entries(shortAliases)) { + map[short] = name as keyof CliOptions; + } + return map; +})(); + +/** All known long flag names (dashed), including `no-*` negations for boolean options. */ +const knownLongSet: Set = (() => { + const set = new Set(); + for (const name of allOptionNames) { + set.add(toDashed(name)); + } + for (const name of booleanOptions) { + set.add(`no-${toDashed(name)}`); + } + return set; +})(); + +/** Split a `--flag` or `--flag=value` token into its name and (optional) inline value. */ +function splitLongFlag(token: string): { name: string; value?: string } { + const rest = token.slice(2); + const eq = rest.indexOf('='); + return eq === -1 ? { name: rest } : { name: rest.slice(0, eq), value: rest.slice(eq + 1) }; +} + +/** + * Preprocess argv to reproduce yargs-parser behaviors that commander doesn't support natively: + * - normalize camelCase and long-alias long flags to their canonical dashed form; + * - rewrite boolean values passed via `=` or as a separate `true`/`false` token to commander's + * flag / `--no-` negation form. + */ +function normalizeArgv(argv: string[]): string[] { + const result: string[] = []; + + for (let i = 0; i < argv.length; i++) { + const token = argv[i]; + + if (token.startsWith('--')) { + const split = splitLongFlag(token); + const { value } = split; + // Normalize alternate long-flag spellings to the canonical dashed form. + const name = flagAliasMap[split.name] ?? split.name; + + // Boolean value passed via `=` (e.g. `--fetch=false` => `--no-fetch`). + if (value !== undefined && booleanDashedSet.has(name) && (value === 'true' || value === 'false')) { + result.push(value === 'true' ? `--${name}` : `--no-${name}`); + continue; + } + + // Boolean value passed as a separate token (e.g. `--yes false` => `--no-yes`). + if (value === undefined && booleanDashedSet.has(name)) { + const next = argv[i + 1]; + if (next === 'true' || next === 'false') { + result.push(next === 'true' ? `--${name}` : `--no-${name}`); + i++; // consume the value token + continue; + } + } + + // Push the (possibly renamed) flag, preserving any inline value. + result.push(value === undefined ? `--${name}` : `--${name}=${value}`); + continue; + } + + // Short boolean flag with a separate `true`/`false` value (e.g. `-y false` => `--no-yes`). + if (token.length === 2 && token[0] === '-' && token[1] !== '-') { + const optionName = shortToName[token[1]]; + if (optionName && (booleanOptions as readonly string[]).includes(optionName)) { + const next = argv[i + 1]; + if (next === 'true' || next === 'false') { + result.push(next === 'true' ? `--${toDashed(optionName)}` : `--no-${toDashed(optionName)}`); + i++; // consume the value token + continue; + } + } + } + + result.push(token); + } + + return result; +} + +/** + * Extract arbitrary unknown options from argv, reproducing yargs-parser's type inference (boolean + * for bare flags, number for numeric strings, string otherwise, array when repeated). Known options + * and positionals are left in place for commander to parse. + * + * @returns the remaining argv (known options + positionals) and a map of the parsed unknown options. + */ +function extractUnknownOptions(argv: string[]): { remaining: string[]; unknown: Record } { + const remaining: string[] = []; + const unknown: Record = {}; + + const record = (name: string, value: unknown): void => { + if (name in unknown) { + const prev = unknown[name]; + unknown[name] = Array.isArray(prev) ? [...(prev as unknown[]), value] : [prev, value]; + } else { + unknown[name] = value; + } + }; + + for (let i = 0; i < argv.length; i++) { + const token = argv[i]; + + if (token.startsWith('--')) { + const { name, value } = splitLongFlag(token); + + // Known options (including `no-*` negations) are handled by commander. + if (knownLongSet.has(name)) { + remaining.push(token); + continue; + } + + // Unknown negated boolean (e.g. `--no-bar` => `bar: false`). + if (name.startsWith('no-')) { + record(name.slice(3), false); + continue; + } + + // Unknown option with an inline value (e.g. `--foo=bar`). + if (value !== undefined) { + record(name, inferUnknownValue(value)); + continue; + } + + // Unknown option; consume the next token as its value unless it's another flag. + const next = argv[i + 1]; + if (next !== undefined && !next.startsWith('-')) { + record(name, inferUnknownValue(next)); + i++; // consume the value token + } else { + record(name, true); + } + continue; + } + + // Known/unknown short flags and positionals are left for commander. + remaining.push(token); + } + + return { remaining, unknown }; +} + +/** Add every beachball option (in its canonical dashed form) to the given command. */ function addAllOptions(command: Command): void { const flags = (name: string, valuePlaceholder?: string): string => { const dashed = toDashed(name); @@ -125,11 +334,11 @@ function addAllOptions(command: Command): void { }; for (const name of stringOptions) { - command.addOption(new Option(flags(name, ''))); + command.addOption(new Option(flags(name, '')).argParser(parseSingle(name))); } for (const name of numberOptions) { - command.addOption(new Option(flags(name, '')).argParser(parseNumber(name))); + command.addOption(new Option(flags(name, '')).argParser(parseSingle(name, parseNumber(name)))); } for (const name of arrayOptions) { @@ -144,7 +353,7 @@ function addAllOptions(command: Command): void { } } -/** Result captured from whichever sub-command commander dispatches to. */ +/** Result captured from parsing. */ interface ParseResult { command: string; options: OptionValues; @@ -152,12 +361,12 @@ interface ParseResult { } /** - * Build a commander program with one sub-command per beachball command. All options are added to - * both the parent (so they can be given before the command) and each sub-command (so they can be - * given after it). Commander is currently used only for parsing, not command dispatch. + * Build a single commander command with every option (so options can be given before or after the + * command name) plus positional arguments for the command name and any extra positional args. + * Commander is currently used only for parsing, not command dispatch. * - * @returns The program plus a getter for the parse result (populated by the matched sub-command's - * action handler when `program.parse()` is called). + * @returns The program plus a getter for the parse result (populated by the action handler when + * `program.parse()` is called). */ function buildProgram(): { program: Command; getResult: () => ParseResult } { const program = new Command(); @@ -167,34 +376,27 @@ function buildProgram(): { program: Command; getResult: () => ParseResult } { program.exitOverride(); program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); // suppress commander output - // Allow options we haven't explicitly defined to pass through without erroring. - // (Full parsing of arbitrary unknown options is handled in a later step; see the plan.) + // Allow any short flags not explicitly defined to pass through without erroring. (Unknown long + // options are pulled out beforehand by `extractUnknownOptions`.) program.allowUnknownOption(); + program.allowExcessArguments(); - // Add all options to the parent so they can be specified before the command name. addAllOptions(program); + // The first positional is the command name (any value; validated by the caller/cli.ts), and any + // remaining positionals are extra args (e.g. for `config get `). + program.argument('[command]', 'beachball command to run'); + program.argument('[extraArgs...]', 'extra positional arguments (e.g. for `config get `)'); + let result: ParseResult = { command: defaultCommand, options: {}, extraArgs: [] }; - for (const name of commands) { - const subcommand = program.command(name, { isDefault: name === defaultCommand }); - addAllOptions(subcommand); - subcommand.allowUnknownOption(); - // Capture any extra positional args (e.g. `config get `). A consistent error for - // non-config commands with extra args is produced later in getCliOptions. - subcommand.argument('[extraArgs...]', 'extra positional arguments (e.g. for `config get `)'); - subcommand.action(() => { - result = { - command: name, - // Merge parent ("global") options with this sub-command's options. - options: subcommand.optsWithGlobals(), - // processedArgs is positional in declaration order; index 0 is the `[extraArgs...]` - // variadic declared above, which commander populates as a string array (or undefined - // when no extra args were given). - extraArgs: (subcommand.processedArgs[0] as string[] | undefined) ?? [], - }; - }); - } + program.action((command: string | undefined, extraArgs: string[]) => { + result = { + command: command ?? defaultCommand, + options: program.opts(), + extraArgs: extraArgs ?? [], + }; + }); return { program, getResult: () => result }; } @@ -214,8 +416,14 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti // Be careful not to mutate the input argv const trimmedArgv = processInfo.argv.slice(2); + // Preprocess argv to reproduce yargs-parser behaviors commander doesn't support natively: + // normalize alternate flag spellings and boolean values, then pull out arbitrary unknown options + // (with type inference) so they don't interfere with commander's positional/command detection. + const normalizedArgv = normalizeArgv(trimmedArgv); + const { remaining, unknown } = extractUnknownOptions(normalizedArgv); + const { program, getResult } = buildProgram(); - program.parse(trimmedArgv, { from: 'user' }); + program.parse(remaining, { from: 'user' }); const { command, options, extraArgs: extraPositionalArgs } = getResult(); let cwd = processInfo.cwd; @@ -236,6 +444,7 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti } const cliOptions: ParsedOptions['cliOptions'] = { + ...unknown, ...options, command, path: cwd, diff --git a/packages/beachball/src/types/BeachballOptions.ts b/packages/beachball/src/types/BeachballOptions.ts index ba02a8693..257355b53 100644 --- a/packages/beachball/src/types/BeachballOptions.ts +++ b/packages/beachball/src/types/BeachballOptions.ts @@ -75,7 +75,6 @@ export interface CliOptions extends Pick< /** * Extra positional arguments after the command (for subcommands like `config get `). - * This is a workaround for `yargs-parser`'s lack of positional argument support. * @internal */ _extraPositionalArgs?: string[]; diff --git a/yarn.lock b/yarn.lock index df4d62cce..eba52161a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3114,7 +3114,6 @@ __metadata: "@types/prompts": "npm:^2.4.2" "@types/semver": "npm:^7.3.13" "@types/tmp": "npm:^0.2.3" - "@types/yargs-parser": "npm:^21.0.0" "@vercel/detect-agent": "npm:^1.2.1" "@verdaccio/types": "npm:^13.0.0" commander: "npm:^14.0.3" @@ -3134,7 +3133,6 @@ __metadata: verdaccio-auth-memory: "npm:^13.0.0" verdaccio-memory: "npm:^10.4.3" workspace-tools: "catalog:prod" - yargs-parser: "npm:^21.1.1" bin: beachball: ./bin/beachball.js languageName: unknown From ba80c18c86467cbc0cc7f12b84fa1f0d1dddd4c2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 08:34:55 +0000 Subject: [PATCH 06/11] Add change file for commander migration --- change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json diff --git a/change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json b/change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json new file mode 100644 index 000000000..f0bc8abc0 --- /dev/null +++ b/change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Migrate CLI option parsing from `yargs-parser` to `commander`. Documented parsing behavior is preserved, with one intentional breaking change: `-?` is no longer accepted as an alias for `--help` (use `-h` or `--help`).", + "packageName": "beachball", + "email": "198982749+Copilot@users.noreply.github.com", + "dependentChangeType": "patch" +} From a13e38288cc11421760d2bfb4309a61b9abb87ad Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 22:49:34 +0000 Subject: [PATCH 07/11] Address review: restore type validation, error on unknown options, config subcommand --- ...-550b8306-eccd-4acd-93d4-13855cec0127.json | 4 +- commander-migration-plan.md | 179 ------------------ .../options/getCliOptions.test.ts | 40 ++-- .../beachball/src/options/getCliOptions.ts | 178 ++++++----------- 4 files changed, 72 insertions(+), 329 deletions(-) delete mode 100644 commander-migration-plan.md diff --git a/change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json b/change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json index f0bc8abc0..4e2c1bd32 100644 --- a/change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json +++ b/change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json @@ -1,6 +1,6 @@ { - "type": "minor", - "comment": "Migrate CLI option parsing from `yargs-parser` to `commander`. Documented parsing behavior is preserved, with one intentional breaking change: `-?` is no longer accepted as an alias for `--help` (use `-h` or `--help`).", + "type": "major", + "comment": "Migrate CLI option parsing from `yargs-parser` to `commander`. Most documented parsing behavior is preserved, with two intentional breaking changes: unknown options now cause an error instead of being passed through, and `-?` is no longer accepted as an alias for `--help` (use `-h` or `--help`).", "packageName": "beachball", "email": "198982749+Copilot@users.noreply.github.com", "dependentChangeType": "patch" diff --git a/commander-migration-plan.md b/commander-migration-plan.md deleted file mode 100644 index 1b00313e6..000000000 --- a/commander-migration-plan.md +++ /dev/null @@ -1,179 +0,0 @@ -# Plan: migrate CLI option parsing from yargs-parser to commander@14 - -> **This is a temporary planning document.** It should be deleted as part of the final cleanup -> (step 7 below), once the migration is complete (or moved into a permanent doc/issue if we want to -> keep it). - -## Goal and scope - -Replace `yargs-parser` with `commander@14` for CLI option parsing in -`packages/beachball/src/options/getCliOptions.ts`. - -Constraints for this migration (per the task): - -- For now, **all options are defined on a single command**. A later change will split up which - options apply to which commands. -- Commander is used **only for option parsing**, not for command dispatch/execution. The existing - `cli.ts` dispatch (switch on `cliOptions.command`) stays as-is. -- The public shape of `getCliOptions` (returns `ParsedOptions['cliOptions']`, including `command`, - `path`, and `_extraPositionalArgs`) must not change. - -The behaviors we must preserve are documented by -`src/__functional__/options/getCliOptions.test.ts`. That test file explicitly notes it exists to -catch "undocumented breaking changes ... likely to commander". This plan maps each behavior to a -commander approach or a workaround. - -## Why this is not a drop-in replacement - -`yargs-parser` is a permissive, schema-light parser: it infers types, expands camelCase, splits -greedy arrays, and accepts arbitrary unknown flags. `commander` is a schema-first parser: every -option is declared up front, and anything not declared is either an error or passed through -untyped. The gaps are all in yargs' permissive behaviors. - -## Behavior mapping (today's yargs behavior -> commander) - -Legend: ✅ native, ⚙️ needs a workaround, ❌ not feasible / propose dropping. - -| # | Behavior (from `getCliOptions.test.ts`) | Example | Commander support | -| --- | --------------------------------------------------- | ---------------------------------------------------------- | ----------------------------------------------------------- | -| 1 | Command as first positional (default `change`) | `beachball check` | ✅ `.argument('[command]')` | -| 2 | String option, separate & `=` forms | `--type patch`, `--access=public` | ✅ `--type ` | -| 3 | Number option + reject non-numeric | `--depth 1`, `--depth foo` throws | ✅ via `.argParser` coercion | -| 4 | Boolean flag | `--fetch` | ✅ `--fetch` | -| 5 | Negated boolean | `--no-fetch` | ✅ define `--no-fetch` alongside `--fetch` | -| 6 | Array: greedy multiple values | `--scope foo bar` | ✅ variadic `` | -| 7 | Array: repeated flag | `--scope foo --scope bar` | ⚙️ variadic + collector fn | -| 8 | Array: single value via `=` becomes array | `--scope=foo` -> `['foo']` | ⚙️ collector fn | -| 9 | Commas NOT split | `--scope a,b` -> `['a,b']` | ✅ (commander never splits) | -| 10 | Boolean value as separate token | `--yes false`, `-y false` | ⚙️ argv preprocessing (see below) | -| 11 | Boolean value via `=` | `--fetch=false`, `--fetch=true` | ⚙️ argv preprocessing (see below) | -| 12 | Throw if non-array option repeated | `--tag a --tag b` throws | ⚙️ collector-style detector fn | -| 13 | camelCase accepted for dashed option | `--gitTags`, `--dependentChangeType` | ⚙️ argv normalization pass | -| 14 | dashed accepted for camelCase option | `--git-tags` | ✅ (declare dashed as canonical) | -| 15 | Negated camelCase | `--no-git-tags` (option `gitTags`) | ✅ declare `--no-git-tags` | -| 16 | Short aliases | `-t`, `-r`, `-y`, `-a`, `-b`, `-m`, `-p`, `-n`, `-v`, `-h` | ✅ in flags string | -| 17 | Extra long aliases | `--config`, `--force`, `--since` | ⚙️ argv normalization or dup options | -| 18 | Arbitrary unknown string option | `--foo bar`, `--foo=bar` | ⚙️ custom leftover parser | -| 19 | Arbitrary unknown boolean flag | `--foo`, `--no-bar` | ⚙️ custom leftover parser | -| 20 | Unknown value type inference | `--foo true` -> bool, `--foo 1` -> number | ⚙️ custom leftover parser | -| 21 | Unknown repeated -> array | `--foo bar --foo baz` -> `['bar','baz']` | ⚙️ custom leftover parser | -| 22 | `-?` alias for help | `-?` | ❌ commander rejects `?` as a short flag; drop or normalize | -| 23 | `config get ` subcommand args | `config get branch` | ✅ variadic `[extraArgs...]` | -| 24 | canary tag override, `NPM_TOKEN`, branch resolution | (post-parse) | ✅ unchanged post-parse logic | - -## Proposed workarounds - -The cleanest strategy is a small **argv normalization pass** run before `program.parse(...)`, plus -a couple of coercion functions and a **leftover unknown-option parser** run after. This keeps the -commander option declarations clean and localizes the yargs-compatibility shims. - -### A. Array options (#7, #8) — collector function - -Declare array options as variadic (`--scope `) and attach an `argParser` collector that -appends to the previous array. Variadic handles greedy multiple values; the collector handles -repeated flags and makes a single `=` value into a one-element array. (Implemented in step 1.) - -### B. Non-array "specified multiple times" error (#12) - -Attach an `argParser` to each string/number option that throws if it receives a value when one was -already set (commander calls the parser with the previous value as the second argument). This -reproduces yargs' "Option X only accepts a single value" error. - -### C. Boolean values as tokens or `=` (#10, #11) - -Commander boolean flags don't accept values (`--fetch=false` / `--yes false` don't work natively; -optional-value `[value]` options don't reliably consume a following token either). Proposed -workaround: an **argv normalization pass** that, for known boolean options, rewrites: - -- `--fetch=false` / `--fetch=true` -> drop the `=value`, emit `--fetch` or `--no-fetch`. -- `--yes false` / `-y false` (value in the next token, only when the next token is literally - `true`/`false`) -> collapse to `--yes` / `--no-yes` and consume that token. - -Only the literal strings `true`/`false` are treated as boolean values (matching today's behavior); -anything else is left for normal positional handling. - -### D. camelCase flag acceptance (#13) and extra long aliases (#17) - -Add an argv normalization pass that maps recognized alternate flag spellings to the canonical -dashed form before parsing: - -- camelCase -> dashed: `--gitTags` -> `--git-tags`, `--dependentChangeType` -> `--dependent-change-type`. - Build the lookup from the known option lists (we already generate the dashed form from the - camelCase name). -- long aliases -> canonical: `--config` -> `--config-path`, `--force` -> `--force-versions`, - `--since` -> `--from-ref`. Maintain a small alias map. (Commander only allows one long flag per - option, so aliases must be normalized rather than declared.) - -Handle both `--flag value` and `--flag=value` shapes in the normalizer. - -### E. Arbitrary unknown options (#18–#21) - -Today yargs collects any unknown `--foo`/`--foo=bar` into `cliOptions` with type inference (boolean -for bare flags and `true`/`false`, number for numeric strings, string otherwise, array when -repeated). Commander with `.allowUnknownOption()` just passes unknown tokens through as raw args — -it does not key/value-parse them, and their presence disrupts positional (command) detection. - -Proposed workaround: run a **small dedicated mini-parser over the leftover unknown tokens** -(`program.parseOptions(...)` exposes `unknown`, or we diff declared flags from argv). For each -unknown token, reproduce yargs' inference rules: - -- `--foo` (no value / next token is another flag) -> `true`; `--no-bar` -> `bar: false`. -- `--foo=bar` or `--foo bar` -> string `bar`; `true`/`false` -> boolean; numeric -> number. -- repeated unknown -> array. - -Keep this logic small and well-tested; it is the largest single behavioral gap. Note the existing -test at line 240 already documents that `--foo bar baz` treats `baz` as the command (greedy arrays -are not applied to unknown options), so the mini-parser should mirror that. - -### F. Positional / command detection (#1, #23) - -Declare `[command]` and `[extraArgs...]` positional arguments. Keep the existing validation that -only `config` may have extra positional args, and keep populating `_extraPositionalArgs`. Because -unknown-option handling (E) can interfere with positional detection, run the normalizer (D) and the -boolean fixups (C) first, then let commander parse known options + positionals, then run the -leftover unknown parser (E) on what remains. - -### G. Error / exit behavior - -Call `program.exitOverride()` and silence `configureOutput` so commander throws instead of calling -`process.exit()` / writing to stderr. This respects the repo rule that `process.exit()` only -belongs in `cli.ts`, and lets tests assert on thrown errors. (Implemented in step 1.) - -## Items proposed to drop or change (need sign-off) - -- **`-?` as a help alias (#22):** commander does not accept `?` as a short-flag character. Options: - (a) drop `-?` (recommended — `-h`/`--help` remain), or (b) rewrite `-?` to `--help` in the - normalizer. This is a minor, arguably-intentional breaking change; call it out in the change file - since `main` targets beachball v3 where breaking changes are allowed. -- **Greedy arrays for unknown options:** not supported today either (test #240), so no change. - -## Implementation steps - -1. **(this PR) Basic commander definitions + parsing, dashed forms only.** - - Add `commander@^14.0.3` dependency. - - Rewrite `getCliOptions.ts` to build a single commander `Command` with every option declared in - its canonical dashed form (string ``, number w/ numeric coercion, boolean w/ `--no-` - negation, array variadic + collector), short aliases included, plus `[command]`/`[extraArgs...]` - positionals. Keep all post-parse logic (branch resolution, canary tag, `NPM_TOKEN`, delete - undefined, `_extraPositionalArgs`, project-root lookup). - - Tests are expected to partially fail at this step (the permissive-syntax cases): camelCase - flags, extra long aliases, boolean values as tokens/`=`, "repeated non-array throws", and - arbitrary unknown-option inference. No change file yet. -2. **Non-array repeat detection (B)** and confirm array collector (A) matches all array tests. -3. **camelCase + long-alias normalization pass (D).** -4. **Boolean-value normalization (C)** and resolve the `-?` decision (G/#22). -5. **Unknown-option mini-parser (E)** to restore arbitrary-option inference and fix positional - interaction (F). -6. **Cleanup:** remove `yargs-parser` and `@types/yargs-parser` from `packages/beachball/package.json`, - remove the now-obsolete `parserOptions`/`allKeysOfType` machinery, run `yarn lint` (dep check), - `yarn build`, `yarn test`, `yarn format`. -7. **Docs + change file:** update any option docs/help text if behavior changed (e.g. `-?`), add a - Beachball change file via `/beachball-change-file`, and delete this planning document. - -## Testing strategy - -- `getCliOptions.test.ts` is the primary contract; drive the migration to green against it, only - editing tests where we deliberately change behavior (e.g. `-?`), with each such change justified - in the change file. -- Add focused unit tests for the new helpers (argv normalizer, boolean fixup, unknown mini-parser). -- Run the full `beachball` suite (`yarn test:all`) since option parsing feeds every command. diff --git a/packages/beachball/src/__functional__/options/getCliOptions.test.ts b/packages/beachball/src/__functional__/options/getCliOptions.test.ts index 3f313adc1..35ebc4edf 100644 --- a/packages/beachball/src/__functional__/options/getCliOptions.test.ts +++ b/packages/beachball/src/__functional__/options/getCliOptions.test.ts @@ -211,37 +211,21 @@ describe('getCliOptions', () => { }); }); - it('preserves additional string options', () => { - const options = getCliOptionsTest(['--foo', 'bar', '--baz=qux']); - expect(options).toEqual({ ...defaults, foo: 'bar', baz: 'qux' }); + it('throws on unknown long option', () => { + // Unlike yargs-parser, commander errors on unknown options (intentional breaking change for v3) + expect(() => getCliOptionsTest(['--foo', 'bar'])).toThrow("unknown option '--foo'"); }); - it('handles additional boolean flags as booleans', () => { - const options = getCliOptionsTest(['--foo', '--no-bar']); - expect(options).toEqual({ ...defaults, foo: true, bar: false }); + it('throws on unknown boolean flag', () => { + expect(() => getCliOptionsTest(['--foo'])).toThrow("unknown option '--foo'"); }); - it('handles additional boolean text values as booleans', () => { - const options = getCliOptionsTest(['--foo', 'true', '--bar=false']); - expect(options).toEqual({ ...defaults, foo: true, bar: false }); + it('throws on unknown negated boolean flag', () => { + expect(() => getCliOptionsTest(['--no-bar'])).toThrow("unknown option '--no-bar'"); }); - it('handles additional numeric values as numbers', () => { - const options = getCliOptionsTest(['--foo', '1', '--bar=2']); - expect(options).toEqual({ ...defaults, foo: 1, bar: 2 }); - }); - - it('handles additional option specified multiple times as array', () => { - const options = getCliOptionsTest(['--foo', 'bar', '--foo', 'baz']); - expect(options).toEqual({ ...defaults, foo: ['bar', 'baz'] }); - }); - - // documenting current behavior (doesn't have to stay this way) - it('for additional options, does not handle multiple values as part of array', () => { - // in this case the trailing value "baz" would be treated as the command since it's the first - // positional option - const options = getCliOptionsTest(['--foo', 'bar', 'baz']); - expect(options).toEqual({ ...defaults, foo: 'bar', command: 'baz' }); + it('throws on unknown option combined with a valid option', () => { + expect(() => getCliOptionsTest(['--tag', 'foo', '--bar=2'])).toThrow("unknown option '--bar=2'"); }); it('gets NPM_TOKEN from environment', () => { @@ -275,10 +259,8 @@ describe('getCliOptions', () => { }); }); - it('still throws for non-config command with extra positional args', () => { - expect(() => getCliOptionsTest(['check', 'extra'])).toThrow( - 'Only one positional argument (the command) is allowed' - ); + it('throws for non-config command with extra positional args', () => { + expect(() => getCliOptionsTest(['check', 'extra'])).toThrow('too many arguments'); }); }); }); diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index baea6d2d8..470a6f462 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -26,19 +26,21 @@ export interface ProcessInfo { // for option parsing (not for dispatching to command implementations); the existing `cli.ts` // dispatch (switch on `cliOptions.command`) is unchanged. // -// A single commander command declares every option (so options can be given before or after the -// command name) plus two positional arguments: the command name and any extra positional args -// (e.g. `config get `). A later change will split up which options apply to which commands. +// The parent command declares every option (so options can be given before or after the command +// name) plus a positional `[command]` argument. The `config` command is declared as a commander +// subcommand so its extra positional args (e.g. `config get `) are handled natively, while +// commander errors on excess positional args for all other commands. // -// Each option is declared in its canonical dashed form. Commander is a schema-first parser, so -// several permissive behaviors that yargs-parser accepted are reproduced with small argv -// preprocessing passes (see `normalizeArgv`) plus a mini-parser for arbitrary unknown options -// (`extractUnknownOptions`): +// Unlike yargs-parser (which accepted arbitrary unknown flags), commander errors on unknown +// options. This is an intentional breaking change for v3. +// +// Each option is declared in its canonical dashed form. Commander is a schema-first parser, so a +// few permissive behaviors that yargs-parser accepted are reproduced with small argv preprocessing +// passes (see `normalizeArgv`): // - camelCase flags (`--gitTags`) and extra long aliases (`--config`, `--force`, `--since`) // are normalized to their canonical dashed form before parsing. // - boolean values passed via `=` or as a separate token (`--fetch=false`, `--yes false`) are // rewritten to commander's flag / `--no-` negation form. -// - arbitrary unknown options are parsed with yargs-like type inference. // - non-array options specified more than once throw (matching yargs). /** Command run when none is specified on the command line. */ @@ -82,6 +84,35 @@ const stringOptions = [ 'type', ] as const; +type AtLeastOne = [T, ...T[]]; +/** Type hack to verify that an array includes all keys of a type */ +const allKeysOfType = + () => + >( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ...x: L extends any ? (Exclude extends never ? L : Exclude[]) : never + ) => + x; + +// Verify that all the known CLI options have types specified, to ensure correct parsing. +// Otherwise it's possible that new CliOptions could be introduced without corresponding parsers. +// +// NOTE: If a prop is missing, this will have a somewhat misleading error: +// Argument of type '"disallowedChangeTypes"' is not assignable to parameter of type '"tag" | "version"' +// +// To fix, add the missing names after "parameter of type" ("tag" and "version" in this example) +// to the appropriate array above. +allKeysOfType()( + ...arrayOptions, + ...booleanOptions, + ...numberOptions, + ...stringOptions, + // these options are filled in below, not respected from the command line + 'path', + 'command', + '_extraPositionalArgs' +); + /** Short single-character aliases for certain options (option name => short flag without dash). */ const shortAliases: Partial> = { authType: 'a', @@ -146,14 +177,6 @@ function collectArray(value: string, previous: string[] | undefined): string[] { return previous ? [...previous, value] : [value]; } -/** Infer the type of an unknown option's value the way yargs-parser did. */ -function inferUnknownValue(value: string): string | number | boolean { - if (value === 'true') return true; - if (value === 'false') return false; - if (value !== '' && !Number.isNaN(Number(value))) return Number(value); - return value; -} - /** * Map of alternate long-flag spellings to their canonical dashed flag (without leading `--`). * Covers camelCase spellings of dashed options (`gitTags` => `git-tags`) and extra long aliases @@ -185,18 +208,6 @@ const shortToName: Record = (() => { return map; })(); -/** All known long flag names (dashed), including `no-*` negations for boolean options. */ -const knownLongSet: Set = (() => { - const set = new Set(); - for (const name of allOptionNames) { - set.add(toDashed(name)); - } - for (const name of booleanOptions) { - set.add(`no-${toDashed(name)}`); - } - return set; -})(); - /** Split a `--flag` or `--flag=value` token into its name and (optional) inline value. */ function splitLongFlag(token: string): { name: string; value?: string } { const rest = token.slice(2); @@ -262,68 +273,6 @@ function normalizeArgv(argv: string[]): string[] { return result; } -/** - * Extract arbitrary unknown options from argv, reproducing yargs-parser's type inference (boolean - * for bare flags, number for numeric strings, string otherwise, array when repeated). Known options - * and positionals are left in place for commander to parse. - * - * @returns the remaining argv (known options + positionals) and a map of the parsed unknown options. - */ -function extractUnknownOptions(argv: string[]): { remaining: string[]; unknown: Record } { - const remaining: string[] = []; - const unknown: Record = {}; - - const record = (name: string, value: unknown): void => { - if (name in unknown) { - const prev = unknown[name]; - unknown[name] = Array.isArray(prev) ? [...(prev as unknown[]), value] : [prev, value]; - } else { - unknown[name] = value; - } - }; - - for (let i = 0; i < argv.length; i++) { - const token = argv[i]; - - if (token.startsWith('--')) { - const { name, value } = splitLongFlag(token); - - // Known options (including `no-*` negations) are handled by commander. - if (knownLongSet.has(name)) { - remaining.push(token); - continue; - } - - // Unknown negated boolean (e.g. `--no-bar` => `bar: false`). - if (name.startsWith('no-')) { - record(name.slice(3), false); - continue; - } - - // Unknown option with an inline value (e.g. `--foo=bar`). - if (value !== undefined) { - record(name, inferUnknownValue(value)); - continue; - } - - // Unknown option; consume the next token as its value unless it's another flag. - const next = argv[i + 1]; - if (next !== undefined && !next.startsWith('-')) { - record(name, inferUnknownValue(next)); - i++; // consume the value token - } else { - record(name, true); - } - continue; - } - - // Known/unknown short flags and positionals are left for commander. - remaining.push(token); - } - - return { remaining, unknown }; -} - /** Add every beachball option (in its canonical dashed form) to the given command. */ function addAllOptions(command: Command): void { const flags = (name: string, valuePlaceholder?: string): string => { @@ -361,41 +310,41 @@ interface ParseResult { } /** - * Build a single commander command with every option (so options can be given before or after the - * command name) plus positional arguments for the command name and any extra positional args. + * Build the commander program. Every option is declared on the parent command (so options can be + * given before or after the command name), plus a positional `[command]` argument. The `config` + * command is declared as a subcommand so its extra positional args (`config get `) are + * handled natively and commander errors on excess positional args for all other commands. * Commander is currently used only for parsing, not command dispatch. * - * @returns The program plus a getter for the parse result (populated by the action handler when + * @returns The program plus a getter for the parse result (populated by the action handlers when * `program.parse()` is called). */ function buildProgram(): { program: Command; getResult: () => ParseResult } { const program = new Command(); // Throw instead of calling process.exit() or writing to stdout/stderr on error, so callers and - // tests can handle failures. + // tests can handle failures. (This also makes commander error on unknown options and excess + // positional args, which is an intentional breaking change from yargs-parser.) program.exitOverride(); program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); // suppress commander output - // Allow any short flags not explicitly defined to pass through without erroring. (Unknown long - // options are pulled out beforehand by `extractUnknownOptions`.) - program.allowUnknownOption(); - program.allowExcessArguments(); - addAllOptions(program); - // The first positional is the command name (any value; validated by the caller/cli.ts), and any - // remaining positionals are extra args (e.g. for `config get `). + // The single positional is the command name (any value; validated by the caller/cli.ts). program.argument('[command]', 'beachball command to run'); - program.argument('[extraArgs...]', 'extra positional arguments (e.g. for `config get `)'); let result: ParseResult = { command: defaultCommand, options: {}, extraArgs: [] }; - program.action((command: string | undefined, extraArgs: string[]) => { - result = { - command: command ?? defaultCommand, - options: program.opts(), - extraArgs: extraArgs ?? [], - }; + program.action((command: string | undefined) => { + result = { command: command ?? defaultCommand, options: program.opts(), extraArgs: [] }; + }); + + // The `config` command takes extra positional args (its subcommand and arguments, e.g. + // `config get ` or `config list`), which are validated by the config command itself. + const configCommand = program.command('config'); + configCommand.argument('[args...]', 'config subcommand and arguments (e.g. `get ` or `list`)'); + configCommand.action((args: string[]) => { + result = { command: 'config', options: program.opts(), extraArgs: args ?? [] }; }); return { program, getResult: () => result }; @@ -417,13 +366,11 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti const trimmedArgv = processInfo.argv.slice(2); // Preprocess argv to reproduce yargs-parser behaviors commander doesn't support natively: - // normalize alternate flag spellings and boolean values, then pull out arbitrary unknown options - // (with type inference) so they don't interfere with commander's positional/command detection. + // normalize alternate flag spellings and boolean values. const normalizedArgv = normalizeArgv(trimmedArgv); - const { remaining, unknown } = extractUnknownOptions(normalizedArgv); const { program, getResult } = buildProgram(); - program.parse(remaining, { from: 'user' }); + program.parse(normalizedArgv, { from: 'user' }); const { command, options, extraArgs: extraPositionalArgs } = getResult(); let cwd = processInfo.cwd; @@ -437,14 +384,7 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti // use the provided cwd } - if (extraPositionalArgs.length && command !== 'config') { - throw new Error( - `Only one positional argument (the command) is allowed. Received: ${[command, ...extraPositionalArgs].join(' ')}` - ); - } - const cliOptions: ParsedOptions['cliOptions'] = { - ...unknown, ...options, command, path: cwd, From e2c63e479ad956de990ad554e7ebd237ac621ddb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 22:51:16 +0000 Subject: [PATCH 08/11] Remove redundant nullish fallback on config args --- packages/beachball/src/options/getCliOptions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index 470a6f462..ae1d25629 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -344,7 +344,7 @@ function buildProgram(): { program: Command; getResult: () => ParseResult } { const configCommand = program.command('config'); configCommand.argument('[args...]', 'config subcommand and arguments (e.g. `get ` or `list`)'); configCommand.action((args: string[]) => { - result = { command: 'config', options: program.opts(), extraArgs: args ?? [] }; + result = { command: 'config', options: program.opts(), extraArgs: args }; }); return { program, getResult: () => result }; From 5ed9629a58e4494ee825a93b5b9838072003f3d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 23:29:30 +0000 Subject: [PATCH 09/11] Add descriptions for each CLI option in getCliOptions --- .../beachball/src/options/getCliOptions.ts | 62 +++++++++++++++++-- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index ae1d25629..c023146c2 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -113,6 +113,58 @@ allKeysOfType()( '_extraPositionalArgs' ); +/** + * Short descriptions for each option, shown in commander's help output. Sourced from `help.ts` + * and the doc comments in `BeachballOptions`. (Keyed by every parseable option name to require a + * description whenever a new option is added.) + */ +const optionDescriptions: Record<(typeof allOptionNames)[number], string> = { + // array options + disallowedChangeTypes: 'change types that are not allowed', + package: 'force creating a change file for this package (can be specified multiple times)', + scope: 'only consider package paths matching this pattern (can be specified multiple times; supports negations)', + // boolean options + all: 'generate change files for all packages', + bump: 'bump versions during publish (use --no-bump to skip)', + bumpDeps: 'bump dependent packages during publish (use --no-bump-deps to skip)', + commit: 'commit change files after "change" (use --no-commit to only stage them)', + disallowDeletedChangeFiles: 'verify that no change files were deleted between head and target branch', + fetch: 'fetch from the remote before determining changes (use --no-fetch to skip)', + forceVersions: "for 'sync': use the version from the registry even if it's older than local", + gitTags: 'create git tags for each published package version (use --no-git-tags to skip)', + help: 'show usage information', + keepChangeFiles: "don't delete the change files from disk after bumping", + publish: 'publish to the npm registry (use --no-publish to skip)', + push: 'push changes back to the remote git branch (use --no-push to skip)', + verbose: 'print additional information to the console', + version: 'show the beachball version', + yes: 'skip the confirmation prompts', + // number options + concurrency: 'maximum concurrency for write operations such as publishing (default: 1)', + depth: 'for shallow clones: depth of git history to consider when fetching', + npmReadConcurrency: 'maximum concurrency for reading package versions from the registry (default: 5)', + gitTimeout: 'timeout in ms for git push operations', + retries: 'number of retries for an npm publish before failing (default: 3)', + timeout: 'timeout in ms for npm operations (other than install)', + // string options + access: 'npm publish access level: "public" or "restricted"', + authType: 'npm auth type for NPM_TOKEN: "authtoken" or "password"', + branch: 'target branch from remote (default: git config init.defaultBranch)', + canaryName: 'dist-tag and version name to use for canary publishes', + changehint: 'customized hint message shown when a change file is needed but missing', + changeDir: 'name of the directory to store change files (default: change)', + configPath: 'custom beachball config path (default: cosmiconfig standard paths)', + dependentChangeType: 'change type to use for dependent packages (default: patch)', + fromRef: 'consider changes or change files since this git ref (branch name, commit SHA)', + message: 'for "change", the change description; for "publish", the commit message', + packToPath: 'pack packages to tgz files under this path instead of publishing to npm', + prereleasePrefix: 'prerelease prefix for packages that will receive a prerelease bump', + registry: 'npm registry (default: https://registry.npmjs.org)', + tag: 'npm dist-tag for publishes (default: "latest")', + token: 'npm auth token (defaults to the NPM_TOKEN environment variable)', + type: 'type of change: e.g. major, minor, patch, none (instead of prompting)', +}; + /** Short single-character aliases for certain options (option name => short flag without dash). */ const shortAliases: Partial> = { authType: 'a', @@ -283,20 +335,22 @@ function addAllOptions(command: Command): void { }; for (const name of stringOptions) { - command.addOption(new Option(flags(name, '')).argParser(parseSingle(name))); + command.addOption(new Option(flags(name, ''), optionDescriptions[name]).argParser(parseSingle(name))); } for (const name of numberOptions) { - command.addOption(new Option(flags(name, '')).argParser(parseSingle(name, parseNumber(name)))); + command.addOption( + new Option(flags(name, ''), optionDescriptions[name]).argParser(parseSingle(name, parseNumber(name))) + ); } for (const name of arrayOptions) { // Variadic to allow multiple space-separated values, plus a collector for repeated usage. - command.addOption(new Option(flags(name, '')).argParser(collectArray)); + command.addOption(new Option(flags(name, ''), optionDescriptions[name]).argParser(collectArray)); } for (const name of booleanOptions) { - command.addOption(new Option(flags(name))); + command.addOption(new Option(flags(name), optionDescriptions[name])); // Negated form (e.g. `--no-fetch`). command.addOption(new Option(`--no-${toDashed(name)}`)); } From 7750da9d3ce7408460edf911030f25095146b8e5 Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Fri, 3 Jul 2026 17:48:23 -0700 Subject: [PATCH 10/11] fix commander version --- .yarnrc.yml | 1 + packages/beachball/package.json | 2 +- packages/proper-changelog/package.json | 2 +- yarn.lock | 11 ++--------- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/.yarnrc.yml b/.yarnrc.yml index 6a1abdaaa..8e7b4e0e9 100644 --- a/.yarnrc.yml +++ b/.yarnrc.yml @@ -20,4 +20,5 @@ yarnPath: .yarn/releases/yarn-4.17.0.cjs catalogs: prod: + commander: ^14.0.3 workspace-tools: ^0.41.10 diff --git a/packages/beachball/package.json b/packages/beachball/package.json index fe117032e..fa5f687ae 100644 --- a/packages/beachball/package.json +++ b/packages/beachball/package.json @@ -36,7 +36,7 @@ }, "dependencies": { "@vercel/detect-agent": "^1.2.1", - "commander": "^14.0.3", + "commander": "catalog:prod", "cosmiconfig": "^9.0.1", "execa": "^5.1.1", "minimatch": "^3.1.5", diff --git a/packages/proper-changelog/package.json b/packages/proper-changelog/package.json index 3315e24c7..a976b7f18 100644 --- a/packages/proper-changelog/package.json +++ b/packages/proper-changelog/package.json @@ -34,7 +34,7 @@ "cross-env": "^10.1.0" }, "dependencies": { - "commander": "^15.0.0", + "commander": "catalog:prod", "nano-spawn": "^2.1.0" } } diff --git a/yarn.lock b/yarn.lock index eba52161a..41b44afd4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3116,7 +3116,7 @@ __metadata: "@types/tmp": "npm:^0.2.3" "@vercel/detect-agent": "npm:^1.2.1" "@verdaccio/types": "npm:^13.0.0" - commander: "npm:^14.0.3" + commander: "catalog:prod" cosmiconfig: "npm:^9.0.1" cross-env: "npm:^10.1.0" execa: "npm:^5.1.1" @@ -3509,13 +3509,6 @@ __metadata: languageName: node linkType: hard -"commander@npm:^15.0.0": - version: 15.0.0 - resolution: "commander@npm:15.0.0" - checksum: 10c0/539229c171914ea1ccd45ee5f10d924289a12a684ea3a7a44147abe54003c35ed6de9ae4ad198d88c29fdf403dca0428451a388234e6dc01b6faa978fb207206 - languageName: node - linkType: hard - "compressible@npm:~2.0.18": version: 2.0.18 resolution: "compressible@npm:2.0.18" @@ -7258,7 +7251,7 @@ __metadata: dependencies: "@microsoft/beachball-scripts": "workspace:^" "@octokit/openapi-types": "npm:^27.0.0" - commander: "npm:^15.0.0" + commander: "catalog:prod" cross-env: "npm:^10.1.0" nano-spawn: "npm:^2.1.0" bin: From 566d9059f4afff000d27cd17eab7f3fc6e38a19d Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Fri, 3 Jul 2026 18:16:12 -0700 Subject: [PATCH 11/11] moving and testing helpers --- ...-56d77230-303c-4c76-8957-1951ac09b5bd.json | 7 + .../options/cliOptionsHelpers.test.ts | 206 +++++++++++++++++ packages/beachball/src/cli.ts | 4 + .../src/options/cliOptionsHelpers.ts | 194 ++++++++++++++++ .../beachball/src/options/getCliOptions.ts | 207 ++---------------- .../beachball/src/options/getRepoOptions.ts | 2 +- 6 files changed, 435 insertions(+), 185 deletions(-) create mode 100644 change/proper-changelog-56d77230-303c-4c76-8957-1951ac09b5bd.json create mode 100644 packages/beachball/src/__tests__/options/cliOptionsHelpers.test.ts create mode 100644 packages/beachball/src/options/cliOptionsHelpers.ts diff --git a/change/proper-changelog-56d77230-303c-4c76-8957-1951ac09b5bd.json b/change/proper-changelog-56d77230-303c-4c76-8957-1951ac09b5bd.json new file mode 100644 index 000000000..b425986ae --- /dev/null +++ b/change/proper-changelog-56d77230-303c-4c76-8957-1951ac09b5bd.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Downgrade commander to v14 to match other beachball-related packages", + "packageName": "proper-changelog", + "email": "elcraig@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/beachball/src/__tests__/options/cliOptionsHelpers.test.ts b/packages/beachball/src/__tests__/options/cliOptionsHelpers.test.ts new file mode 100644 index 000000000..484ceb21c --- /dev/null +++ b/packages/beachball/src/__tests__/options/cliOptionsHelpers.test.ts @@ -0,0 +1,206 @@ +import { describe, expect, it } from '@jest/globals'; +import { Command, InvalidArgumentError } from 'commander'; +import type { CliOptions } from '../../types/BeachballOptions'; +import { + _getFlagAliasMap, + _parseNumber, + _parseSingle, + _toDashed, + addAllOptions, + normalizeArgv, +} from '../../options/cliOptionsHelpers'; + +describe('_toDashed', () => { + it('leaves all-lowercase names unchanged', () => { + expect(_toDashed('branch')).toBe('branch'); + expect(_toDashed('access')).toBe('access'); + }); + + it('converts camelCase to dashed', () => { + expect(_toDashed('gitTags')).toBe('git-tags'); + expect(_toDashed('bumpDeps')).toBe('bump-deps'); + expect(_toDashed('disallowedChangeTypes')).toBe('disallowed-change-types'); + }); +}); + +describe('_parseNumber', () => { + it('parses numeric strings', () => { + expect(_parseNumber('1')).toBe(1); + expect(_parseNumber('0')).toBe(0); + expect(_parseNumber('-3')).toBe(-3); + expect(_parseNumber('1.5')).toBe(1.5); + }); + + it('throws InvalidArgumentError for non-numeric values', () => { + expect(() => _parseNumber('abc')).toThrow(InvalidArgumentError); + expect(() => _parseNumber('abc')).toThrow('Expected numeric value.'); + expect(() => _parseNumber('')).not.toThrow(); // empty string coerces to 0 + }); +}); + +describe('_parseSingle', () => { + it('returns the value on first use', () => { + expect(_parseSingle()('main', undefined)).toBe('main'); + }); + + it('throws InvalidArgumentError if the option is specified more than once', () => { + expect(() => _parseSingle()('main', 'other')).toThrow(InvalidArgumentError); + expect(() => _parseSingle()('main', 'other')).toThrow('Option can only be specified once.'); + }); + + it('applies the coerce function when provided', () => { + const parse = _parseSingle(_parseNumber); + expect(parse('5', undefined)).toBe(5); + }); + + it('applies coerce and still throws on repeated use', () => { + const parse = _parseSingle(_parseNumber); + expect(() => parse('5', 5)).toThrow('Option can only be specified once.'); + }); +}); + +describe('_getFlagAliasMap', () => { + it('maps camelCase spellings to their dashed canonical form', () => { + const map = _getFlagAliasMap({ + allOptionNames: ['gitTags', 'bumpDeps', 'branch'], + longAliases: {}, + }); + expect(map).toEqual({ gitTags: 'git-tags', bumpDeps: 'bump-deps' }); + }); + + it('does not map names that are already all-lowercase', () => { + const map = _getFlagAliasMap({ + allOptionNames: ['branch', 'access'], + longAliases: {}, + }); + expect(map).toEqual({}); + }); + + it('maps long aliases to their dashed canonical form', () => { + const map = _getFlagAliasMap({ + allOptionNames: ['configPath', 'fromRef'], + longAliases: { config: 'configPath', since: 'fromRef' }, + }); + expect(map).toEqual({ configPath: 'config-path', fromRef: 'from-ref', config: 'config-path', since: 'from-ref' }); + }); +}); + +describe('normalizeArgv', () => { + const params = { + allOptionNames: ['gitTags', 'branch', 'fetch', 'yes', 'configPath', 'fromRef'] as const, + longAliases: { config: 'configPath', since: 'fromRef' } as Record, + booleanOptions: ['fetch', 'yes', 'gitTags'] as const, + shortAliases: { yes: 'y' }, + }; + + const normalize = (argv: string[]) => normalizeArgv({ ...params, argv }); + + it('leaves already-canonical args unchanged', () => { + expect(normalize(['check', '--branch', 'main'])).toEqual(['check', '--branch', 'main']); + }); + + it('normalizes camelCase long flags to dashed', () => { + expect(normalize(['--gitTags'])).toEqual(['--git-tags']); + }); + + it('normalizes long aliases to their canonical dashed form', () => { + expect(normalize(['--config', 'foo'])).toEqual(['--config-path', 'foo']); + expect(normalize(['--since', 'HEAD'])).toEqual(['--from-ref', 'HEAD']); + }); + + it('preserves inline values when renaming flags', () => { + expect(normalize(['--config=foo'])).toEqual(['--config-path=foo']); + }); + + it('rewrites a boolean value passed via = to flag/negation form', () => { + expect(normalize(['--fetch=false'])).toEqual(['--no-fetch']); + expect(normalize(['--fetch=true'])).toEqual(['--fetch']); + }); + + it('rewrites a boolean value passed as a separate token', () => { + expect(normalize(['--yes', 'false'])).toEqual(['--no-yes']); + expect(normalize(['--yes', 'true'])).toEqual(['--yes']); + }); + + it('rewrites a camelCase boolean flag with a separate value token', () => { + expect(normalize(['--gitTags', 'false'])).toEqual(['--no-git-tags']); + }); + + it('leaves a boolean flag alone if the next token is not true/false', () => { + expect(normalize(['--fetch', 'check'])).toEqual(['--fetch', 'check']); + }); + + it('does not treat = values on non-boolean options as negations', () => { + expect(normalize(['--branch=false'])).toEqual(['--branch=false']); + }); + + it('rewrites a short boolean flag with a separate value token', () => { + expect(normalize(['-y', 'false'])).toEqual(['--no-yes']); + expect(normalize(['-y', 'true'])).toEqual(['--yes']); + }); + + it('leaves a short boolean flag alone if the next token is not true/false', () => { + expect(normalize(['-y', 'other'])).toEqual(['-y', 'other']); + }); + + it('does not mutate the input argv', () => { + const argv = ['--gitTags', 'false']; + normalizeArgv({ ...params, argv }); + expect(argv).toEqual(['--gitTags', 'false']); + }); +}); + +describe('addAllOptions', () => { + function buildCommand() { + const command = new Command(); + addAllOptions({ + command, + stringOptions: ['branch'], + numberOptions: ['depth'], + arrayOptions: ['scope'], + booleanOptions: ['fetch'], + optionDescriptions: { + branch: 'target branch', + depth: 'clone depth', + scope: 'scope pattern', + fetch: 'fetch first', + } as Record, + shortAliases: { branch: 'b' }, + }); + return command; + } + + it('parses a string option with its short alias and description', () => { + const command = buildCommand(); + const branchOption = command.options.find(o => o.long === '--branch'); + expect(branchOption?.short).toBe('-b'); + expect(branchOption?.description).toBe('target branch'); + + expect(command.parse(['-b', 'main'], { from: 'user' }).opts().branch).toBe('main'); + expect(buildCommand().parse(['--branch', 'main'], { from: 'user' }).opts().branch).toBe('main'); + }); + + it('parses a number option coerced to a number', () => { + const opts = buildCommand().parse(['--depth', '3'], { from: 'user' }).opts(); + expect(opts.depth).toBe(3); + }); + + it('throws when a single-value option is specified twice', () => { + const command = buildCommand(); + command.exitOverride(); + expect(() => command.parse(['--branch', 'a', '--branch', 'b'], { from: 'user' })).toThrow( + 'Option can only be specified once.' + ); + }); + + it('collects array options from repeated and variadic usage', () => { + expect(buildCommand().parse(['--scope', 'a', '--scope', 'b'], { from: 'user' }).opts().scope).toEqual(['a', 'b']); + expect(buildCommand().parse(['--scope', 'a', 'b'], { from: 'user' }).opts().scope).toEqual(['a', 'b']); + }); + + it('parses a boolean option and its negation', () => { + expect(buildCommand().parse(['--fetch'], { from: 'user' }).opts().fetch).toBe(true); + expect(buildCommand().parse(['--no-fetch'], { from: 'user' }).opts().fetch).toBe(false); + expect(buildCommand().parse([], { from: 'user' }).opts().fetch).toBeUndefined(); + }); +}); diff --git a/packages/beachball/src/cli.ts b/packages/beachball/src/cli.ts index 3fbe1b92f..6b890ef7a 100644 --- a/packages/beachball/src/cli.ts +++ b/packages/beachball/src/cli.ts @@ -1,3 +1,4 @@ +import { CommanderError } from 'commander'; import { findGitRoot } from 'workspace-tools'; import { bump } from './commands/bump'; import { canary } from './commands/canary'; @@ -125,6 +126,9 @@ import { getPackageGroups } from './monorepo/getPackageGroups'; } else if (e instanceof BeachballError) { // Expected error, not yet logged -- print the message (no stack trace) console.error(e.message); + } else if (e instanceof CommanderError) { + // Commander error -- print the message (no stack trace) + console.error(e.message); } else { // Unexpected error -- print full details including stack showVersion(); diff --git a/packages/beachball/src/options/cliOptionsHelpers.ts b/packages/beachball/src/options/cliOptionsHelpers.ts new file mode 100644 index 000000000..cad0133ae --- /dev/null +++ b/packages/beachball/src/options/cliOptionsHelpers.ts @@ -0,0 +1,194 @@ +import { type Command, Option, InvalidArgumentError } from 'commander'; +import type { CliOptions } from '../types/BeachballOptions'; +import { cacheRemoteBranch } from '../git/getRemoteBranch'; +import { resolveRemoteAndBranch } from '../git/tempGetDefaultRemoteBranch'; + +/** Convert a camelCase option name to its dashed CLI flag form (e.g. `gitTags` => `git-tags`). */ +export function _toDashed(name: string): string { + return name.replace(/[A-Z]/g, letter => `-${letter.toLowerCase()}`); +} + +/** Coerce a value to a number, throwing `InvalidArgumentError` if it's not numeric. */ +export function _parseNumber(value: string): number { + const num = Number(value); + if (Number.isNaN(num)) { + throw new InvalidArgumentError('Expected numeric value.'); + } + return num; +} + +/** + * Build an `argParser` for a non-array option that throws `InvalidArgumentError` if the option is + * specified more than once (commander passes the previously-parsed value as the second argument). + * @param coerce Optional function to transform the value before returning it. + */ +export function _parseSingle(coerce?: (value: string) => T): (value: string, previous: unknown) => T | string { + return (value: string, previous: unknown) => { + if (previous !== undefined) { + throw new InvalidArgumentError('Option can only be specified once.'); + } + return coerce ? coerce(value) : value; + }; +} + +/** Collector for array options: accumulate repeated/variadic values into a single array. */ +function collectArray(value: string, previous: string[] | undefined): string[] { + return previous ? [...previous, value] : [value]; +} + +/** + * Get a map of alternate long-flag spellings to their canonical dashed flag (without leading `--`). + * Covers camelCase spellings of dashed options (`gitTags` => `git-tags`) and extra long aliases + * (`config` => `config-path`). + */ +export function _getFlagAliasMap(params: { + allOptionNames: readonly (keyof CliOptions)[]; + longAliases: Record; +}): Record { + const { allOptionNames, longAliases } = params; + const map: Record = {}; + for (const name of allOptionNames) { + const dashed = _toDashed(name); + if (name !== dashed) { + map[name] = dashed; // camelCase spelling => dashed canonical + } + } + for (const [alias, name] of Object.entries(longAliases)) { + map[alias] = _toDashed(name); + } + return map; +} + +/** + * Preprocess argv to reproduce yargs-parser behaviors that commander doesn't support natively: + * - normalize camelCase and long-alias long flags to their canonical dashed form; + * - rewrite boolean values passed via `=` or as a separate `true`/`false` token to commander's + * flag / `--no-` negation form. + */ +export function normalizeArgv(params: { + argv: string[]; + allOptionNames: readonly (keyof CliOptions)[]; + longAliases: Record; + booleanOptions: readonly (keyof CliOptions)[]; + shortAliases: Partial>; +}): string[] { + const { argv, booleanOptions, shortAliases } = params; + const result: string[] = []; + + /** Dashed names of boolean options (e.g. `git-tags`). */ + const booleanDashedSet = new Set(booleanOptions.map(_toDashed)); + + /** map of alternate long-flag spellings to their canonical dashed flag (without leading `--`) */ + const flagAliasMap = _getFlagAliasMap(params); + + /** Short flag character => camelCase option name mapping (e.g. `y` => `yes`). */ + const shortToName = Object.fromEntries( + Object.entries(shortAliases).map(([name, short]) => [short, name as keyof CliOptions]) + ); + + for (let i = 0; i < argv.length; i++) { + const token = argv[i]; + + if (token.startsWith('--')) { + // Split a `--flag` or `--flag=value` token into its name and (optional) inline value + const [splitName, value] = token.slice(2).split('=', 2); + // Normalize alternate long-flag spellings to the canonical dashed form. + const name = flagAliasMap[splitName] ?? splitName; + + // Boolean value passed via `=` (e.g. `--fetch=false` => `--no-fetch`). + if (value !== undefined && booleanDashedSet.has(name) && (value === 'true' || value === 'false')) { + result.push(value === 'true' ? `--${name}` : `--no-${name}`); + continue; + } + + // Boolean value passed as a separate token (e.g. `--yes false` => `--no-yes`). + if (value === undefined && booleanDashedSet.has(name)) { + const next = argv[i + 1]; + if (next === 'true' || next === 'false') { + result.push(next === 'true' ? `--${name}` : `--no-${name}`); + i++; // consume the value token + continue; + } + } + + // Push the (possibly renamed) flag, preserving any inline value. + result.push(value === undefined ? `--${name}` : `--${name}=${value}`); + continue; + } + + // Short boolean flag with a separate `true`/`false` value (e.g. `-y false` => `--no-yes`). + if (token.length === 2 && token[0] === '-' && token[1] !== '-') { + const optionName = shortToName[token[1]]; + if (optionName && (booleanOptions as readonly string[]).includes(optionName)) { + const next = argv[i + 1]; + if (next === 'true' || next === 'false') { + result.push(next === 'true' ? `--${_toDashed(optionName)}` : `--no-${_toDashed(optionName)}`); + i++; // consume the value token + continue; + } + } + } + + result.push(token); + } + + return result; +} + +/** Add every beachball option (in its canonical dashed form) to the given command. */ +export function addAllOptions(params: { + command: Command; + stringOptions: readonly (keyof CliOptions)[]; + numberOptions: readonly (keyof CliOptions)[]; + arrayOptions: readonly (keyof CliOptions)[]; + booleanOptions: readonly (keyof CliOptions)[]; + optionDescriptions: Record; + shortAliases: Partial>; +}): void { + const { command, stringOptions, numberOptions, arrayOptions, booleanOptions, optionDescriptions, shortAliases } = + params; + + const flags = (name: string, valuePlaceholder?: string): string => { + const dashed = _toDashed(name); + const short = shortAliases[name as keyof CliOptions]; + const long = valuePlaceholder ? `--${dashed} ${valuePlaceholder}` : `--${dashed}`; + return short ? `-${short}, ${long}` : long; + }; + + for (const name of stringOptions) { + command.addOption(new Option(flags(name, ''), optionDescriptions[name]).argParser(_parseSingle())); + } + + for (const name of numberOptions) { + command.addOption( + new Option(flags(name, ''), optionDescriptions[name]).argParser(_parseSingle(_parseNumber)) + ); + } + + for (const name of arrayOptions) { + // Variadic to allow multiple space-separated values, plus a collector for repeated usage. + command.addOption(new Option(flags(name, ''), optionDescriptions[name]).argParser(collectArray)); + } + + for (const name of booleanOptions) { + command.addOption(new Option(flags(name), optionDescriptions[name])); + // Negated form (e.g. `--no-fetch`). + command.addOption(new Option(`--no-${_toDashed(name)}`)); + } +} + +/** + * Resolves `rawOptions.branch` if provided to ensure it includes the remote name. + * If no branch is provided, returns the default branch. + */ +export function resolveBranchOption(rawOptions: Partial>, cwd: string): string { + const branchResult = resolveRemoteAndBranch({ + branch: rawOptions.branch, + cwd, + verbose: rawOptions.verbose, + strict: true, + }); + cacheRemoteBranch(branchResult, cwd); + + return `${branchResult.remote}/${branchResult.remoteBranch}`; +} diff --git a/packages/beachball/src/options/getCliOptions.ts b/packages/beachball/src/options/getCliOptions.ts index c023146c2..583bbf956 100644 --- a/packages/beachball/src/options/getCliOptions.ts +++ b/packages/beachball/src/options/getCliOptions.ts @@ -1,9 +1,8 @@ import { findProjectRoot } from 'workspace-tools'; -import { Command, Option, type OptionValues } from 'commander'; +import { Command, type OptionValues } from 'commander'; import { env } from '../env'; import type { CliOptions, ParsedOptions } from '../types/BeachballOptions'; -import { cacheRemoteBranch } from '../git/getRemoteBranch'; -import { resolveRemoteAndBranch } from '../git/tempGetDefaultRemoteBranch'; +import { addAllOptions, normalizeArgv, resolveBranchOption } from './cliOptionsHelpers'; export interface ProcessInfo { /** Complete argv (node and script path aren't used but elements must be present) */ @@ -118,7 +117,7 @@ allKeysOfType()( * and the doc comments in `BeachballOptions`. (Keyed by every parseable option name to require a * description whenever a new option is added.) */ -const optionDescriptions: Record<(typeof allOptionNames)[number], string> = { +const optionDescriptions: Record = { // array options disallowedChangeTypes: 'change types that are not allowed', package: 'force creating a change file for this package (can be specified multiple times)', @@ -163,6 +162,10 @@ const optionDescriptions: Record<(typeof allOptionNames)[number], string> = { tag: 'npm dist-tag for publishes (default: "latest")', token: 'npm auth token (defaults to the NPM_TOKEN environment variable)', type: 'type of change: e.g. major, minor, patch, none (instead of prompting)', + // not handled by commander parsing + _extraPositionalArgs: '', + command: '', + path: '', }; /** Short single-character aliases for certain options (option name => short flag without dash). */ @@ -194,168 +197,6 @@ const longAliases: Record = { /** All option names (any value type). */ const allOptionNames = [...arrayOptions, ...booleanOptions, ...numberOptions, ...stringOptions]; -/** Convert a camelCase option name to its dashed CLI flag form (e.g. `gitTags` => `git-tags`). */ -function toDashed(name: string): string { - return name.replace(/[A-Z]/g, letter => `-${letter.toLowerCase()}`); -} - -/** Coerce a value to a number, throwing if it's not numeric (matches previous yargs behavior). */ -function parseNumber(name: string): (value: string) => number { - return (value: string) => { - const num = Number(value); - if (Number.isNaN(num)) { - throw new Error(`Non-numeric value passed for numeric option "${name}"`); - } - return num; - }; -} - -/** - * Build an `argParser` for a non-array option that throws if the option is specified more than once - * (commander passes the previously-parsed value as the second argument). This matches yargs-parser, - * which errored on repeated single-value options. An optional `coerce` transforms each value. - */ -function parseSingle(name: string, coerce?: (value: string) => T): (value: string, previous: unknown) => T | string { - return (value: string, previous: unknown) => { - if (previous !== undefined) { - throw new Error(`Option "${name}" can only be specified once`); - } - return coerce ? coerce(value) : value; - }; -} - -/** Collector for array options: accumulate repeated/variadic values into a single array. */ -function collectArray(value: string, previous: string[] | undefined): string[] { - return previous ? [...previous, value] : [value]; -} - -/** - * Map of alternate long-flag spellings to their canonical dashed flag (without leading `--`). - * Covers camelCase spellings of dashed options (`gitTags` => `git-tags`) and extra long aliases - * (`config` => `config-path`). - */ -const flagAliasMap: Record = (() => { - const map: Record = {}; - for (const name of allOptionNames) { - const dashed = toDashed(name); - if (name !== dashed) { - map[name] = dashed; // camelCase spelling => dashed canonical - } - } - for (const [alias, name] of Object.entries(longAliases)) { - map[alias] = toDashed(name); - } - return map; -})(); - -/** Dashed names of boolean options (e.g. `git-tags`). */ -const booleanDashedSet = new Set(booleanOptions.map(toDashed)); - -/** Short flag character => camelCase option name (e.g. `y` => `yes`). */ -const shortToName: Record = (() => { - const map: Record = {}; - for (const [name, short] of Object.entries(shortAliases)) { - map[short] = name as keyof CliOptions; - } - return map; -})(); - -/** Split a `--flag` or `--flag=value` token into its name and (optional) inline value. */ -function splitLongFlag(token: string): { name: string; value?: string } { - const rest = token.slice(2); - const eq = rest.indexOf('='); - return eq === -1 ? { name: rest } : { name: rest.slice(0, eq), value: rest.slice(eq + 1) }; -} - -/** - * Preprocess argv to reproduce yargs-parser behaviors that commander doesn't support natively: - * - normalize camelCase and long-alias long flags to their canonical dashed form; - * - rewrite boolean values passed via `=` or as a separate `true`/`false` token to commander's - * flag / `--no-` negation form. - */ -function normalizeArgv(argv: string[]): string[] { - const result: string[] = []; - - for (let i = 0; i < argv.length; i++) { - const token = argv[i]; - - if (token.startsWith('--')) { - const split = splitLongFlag(token); - const { value } = split; - // Normalize alternate long-flag spellings to the canonical dashed form. - const name = flagAliasMap[split.name] ?? split.name; - - // Boolean value passed via `=` (e.g. `--fetch=false` => `--no-fetch`). - if (value !== undefined && booleanDashedSet.has(name) && (value === 'true' || value === 'false')) { - result.push(value === 'true' ? `--${name}` : `--no-${name}`); - continue; - } - - // Boolean value passed as a separate token (e.g. `--yes false` => `--no-yes`). - if (value === undefined && booleanDashedSet.has(name)) { - const next = argv[i + 1]; - if (next === 'true' || next === 'false') { - result.push(next === 'true' ? `--${name}` : `--no-${name}`); - i++; // consume the value token - continue; - } - } - - // Push the (possibly renamed) flag, preserving any inline value. - result.push(value === undefined ? `--${name}` : `--${name}=${value}`); - continue; - } - - // Short boolean flag with a separate `true`/`false` value (e.g. `-y false` => `--no-yes`). - if (token.length === 2 && token[0] === '-' && token[1] !== '-') { - const optionName = shortToName[token[1]]; - if (optionName && (booleanOptions as readonly string[]).includes(optionName)) { - const next = argv[i + 1]; - if (next === 'true' || next === 'false') { - result.push(next === 'true' ? `--${toDashed(optionName)}` : `--no-${toDashed(optionName)}`); - i++; // consume the value token - continue; - } - } - } - - result.push(token); - } - - return result; -} - -/** Add every beachball option (in its canonical dashed form) to the given command. */ -function addAllOptions(command: Command): void { - const flags = (name: string, valuePlaceholder?: string): string => { - const dashed = toDashed(name); - const short = shortAliases[name as keyof CliOptions]; - const long = valuePlaceholder ? `--${dashed} ${valuePlaceholder}` : `--${dashed}`; - return short ? `-${short}, ${long}` : long; - }; - - for (const name of stringOptions) { - command.addOption(new Option(flags(name, ''), optionDescriptions[name]).argParser(parseSingle(name))); - } - - for (const name of numberOptions) { - command.addOption( - new Option(flags(name, ''), optionDescriptions[name]).argParser(parseSingle(name, parseNumber(name))) - ); - } - - for (const name of arrayOptions) { - // Variadic to allow multiple space-separated values, plus a collector for repeated usage. - command.addOption(new Option(flags(name, ''), optionDescriptions[name]).argParser(collectArray)); - } - - for (const name of booleanOptions) { - command.addOption(new Option(flags(name), optionDescriptions[name])); - // Negated form (e.g. `--no-fetch`). - command.addOption(new Option(`--no-${toDashed(name)}`)); - } -} - /** Result captured from parsing. */ interface ParseResult { command: string; @@ -382,7 +223,15 @@ function buildProgram(): { program: Command; getResult: () => ParseResult } { program.exitOverride(); program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); // suppress commander output - addAllOptions(program); + addAllOptions({ + command: program, + stringOptions, + numberOptions, + arrayOptions, + booleanOptions, + optionDescriptions, + shortAliases, + }); // The single positional is the command name (any value; validated by the caller/cli.ts). program.argument('[command]', 'beachball command to run'); @@ -421,7 +270,13 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti // Preprocess argv to reproduce yargs-parser behaviors commander doesn't support natively: // normalize alternate flag spellings and boolean values. - const normalizedArgv = normalizeArgv(trimmedArgv); + const normalizedArgv = normalizeArgv({ + argv: trimmedArgv, + allOptionNames, + longAliases, + booleanOptions, + shortAliases, + }); const { program, getResult } = buildProgram(); program.parse(normalizedArgv, { from: 'user' }); @@ -471,19 +326,3 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti return cliOptions; } - -/** - * Resolves `rawOptions.branch` if provided to ensure it includes the remote name. - * If no branch is provided, returns the default branch. - */ -export function resolveBranchOption(rawOptions: Partial>, cwd: string): string { - const branchResult = resolveRemoteAndBranch({ - branch: rawOptions.branch, - cwd, - verbose: rawOptions.verbose, - strict: true, - }); - cacheRemoteBranch(branchResult, cwd); - - return `${branchResult.remote}/${branchResult.remoteBranch}`; -} diff --git a/packages/beachball/src/options/getRepoOptions.ts b/packages/beachball/src/options/getRepoOptions.ts index 055a552ee..21db8ecbc 100644 --- a/packages/beachball/src/options/getRepoOptions.ts +++ b/packages/beachball/src/options/getRepoOptions.ts @@ -3,7 +3,7 @@ import path from 'path'; import { findGitRoot } from 'workspace-tools'; import { BeachballError } from '../types/BeachballError'; import type { ParsedOptions, RepoOptions } from '../types/BeachballOptions'; -import { resolveBranchOption } from './getCliOptions'; +import { resolveBranchOption } from './cliOptionsHelpers'; /** * Find the beachball config file and return the repo options.