Switch to commander for arg parsing

.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

change/beachball-550b8306-eccd-4acd-93d4-13855cec0127.json

@@ -0,0 +1,7 @@
+{
+  "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"
+}

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"
+}

packages/beachball/package.json

@@ -36,15 +36,15 @@
   },
   "dependencies": {
     "@vercel/detect-agent": "^1.2.1",
+    "commander": "catalog:prod",
     "cosmiconfig": "^9.0.1",
     "execa": "^5.1.1",
     "minimatch": "^3.1.5",
     "p-graph": "^1.3.0",
     "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:^",
@@ -53,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",

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');
     });
   });
 });

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<string, keyof CliOptions>,
+    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<keyof CliOptions, string>,
+      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();
+  });
+});

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();