[ts-command-line] Deprecate onDefineParameters.

common/changes/@rushstack/ts-command-line/ts-command-line-recommendation-updates_2024-05-15-20-16.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/ts-command-line",
+      "comment": "Mark `onDefineParameters` and `onDefineUnscopedParameters` as deprecated and update README accordingly because defining parameters causes issues when the compiler targets >=es2022.",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/ts-command-line"
+}

common/reviews/api/ts-command-line.api.md

@@ -217,6 +217,7 @@ export abstract class CommandLineParameterProvider {
     getParameterStringMap(): Record<string, string>;
     getStringListParameter(parameterLongName: string, parameterScope?: string): CommandLineStringListParameter;
     getStringParameter(parameterLongName: string, parameterScope?: string): CommandLineStringParameter;
+    // @deprecated (undocumented)
     protected onDefineParameters?(): void;
     get parameters(): ReadonlyArray<CommandLineParameter>;
     get parametersProcessed(): boolean;
@@ -450,8 +451,8 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
     _executeAsync(): Promise<void>;
     // @internal
     protected _getScopedCommandLineParser(): CommandLineParser;
-    protected onDefineParameters(): void;
     protected abstract onDefineScopedParameters(scopedParameterProvider: CommandLineParameterProvider): void;
+    // @deprecated (undocumented)
     protected onDefineUnscopedParameters?(): void;
     protected abstract onExecute(): Promise<void>;
     get parameters(): ReadonlyArray<CommandLineParameter>;

libraries/ts-command-line/README.md

@@ -74,13 +74,7 @@ export class PushAction extends CommandLineAction {
       summary: 'Pushes a widget to the service',
       documentation: 'Here we provide a longer description of how our action works.'
     });
-  }
-
-  protected onExecute(): Promise<void> { // abstract
-    return BusinessLogic.doTheWork(this._force.value, this._protocol.value || "(none)");
-  }
 
-  protected onDefineParameters(): void { // abstract
     this._force = this.defineFlagParameter({
       parameterLongName: '--force',
       parameterShortName: '-f',
@@ -95,6 +89,10 @@ export class PushAction extends CommandLineAction {
       defaultValue: 'scp'
     });
   }
+
+  protected async onExecute(): Promise<void> { // abstract
+    await BusinessLogic.doTheWork(this._force.value, this._protocol.value || "(none)");
+  }
 }
 ```
 
@@ -111,19 +109,17 @@ export class WidgetCommandLine extends CommandLineParser {
     });
 
     this.addAction(new PushAction());
-  }
 
-  protected onDefineParameters(): void { // abstract
     this._verbose = this.defineFlagParameter({
       parameterLongName: '--verbose',
       parameterShortName: '-v',
       description: 'Show extra logging detail'
     });
   }
 
-  protected onExecute(): Promise<void> { // override
+  protected async onExecute(): Promise<void> { // override
     BusinessLogic.configureLogger(this._verbose.value);
-    return super.onExecute();
+    await super.onExecute();
   }
 }
 ```
@@ -132,7 +128,7 @@ To invoke the parser, the application entry point will do something like this:
 
 ```typescript
 const commandLine: WidgetCommandLine = new WidgetCommandLine();
-commandLine.execute();
+commandLine.executeAsync();
 ```
 
 When we run `widget --verbose push --force`, the `PushAction.onExecute()` method will get invoked and then your business logic takes over.
@@ -226,7 +222,7 @@ action.defineChoiceParameter({
 });
 
 // Parse the command line
-commandLineParser.execute().then(() => {
+commandLineParser.executeAsync().then(() => {
   console.log('The action is: ' + commandLineParser.selectedAction!.actionName);
   console.log('The force flag is: ' + action.getFlagParameter('--force').value);
 });

libraries/ts-command-line/src/providers/CommandLineParameterProvider.ts

@@ -589,8 +589,7 @@ export abstract class CommandLineParameterProvider {
   }
 
   /**
-   * The child class should implement this hook to define its command-line parameters,
-   * e.g. by calling defineFlagParameter().
+   * @deprecated - Define parameters in the constructor
    */
   protected onDefineParameters?(): void;
 

libraries/ts-command-line/src/providers/ScopedCommandLineAction.ts

@@ -116,6 +116,17 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
 
     this._options = options;
     this._scopingParameters = [];
+
+    // Consume the remainder of the command-line, which will later be passed the scoped parser.
+    // This will also prevent developers from calling this.defineCommandLineRemainder(...) since
+    // we will have already defined it.
+    this.defineCommandLineRemainder({
+      description:
+        'Scoped parameters.  Must be prefixed with "--", ex. "-- --scopedParameter ' +
+        'foo --scopedFlag".  For more information on available scoped parameters, use "-- --help".'
+    });
+
+    this.onDefineUnscopedParameters?.();
   }
 
   /**
@@ -206,6 +217,14 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
 
   /** @internal */
   public _registerDefinedParameters(state: IRegisterDefinedParametersState): void {
+    if (!this._scopingParameters.length) {
+      throw new Error(
+        'No scoping parameters defined. At least one scoping parameter must be defined. ' +
+          'Scoping parameters are defined by setting the parameterGroupName to ' +
+          'ScopedCommandLineAction.ScopingParameterGroupName.'
+      );
+    }
+
     super._registerDefinedParameters(state);
 
     const { parentParameterNames } = state;
@@ -220,36 +239,6 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
     };
   }
 
-  /**
-   * {@inheritdoc CommandLineParameterProvider.onDefineParameters}
-   */
-  protected onDefineParameters(): void {
-    this.onDefineUnscopedParameters?.();
-
-    if (!this._scopingParameters.length) {
-      throw new Error(
-        'No scoping parameters defined. At least one scoping parameter must be defined. ' +
-          'Scoping parameters are defined by setting the parameterGroupName to ' +
-          'ScopedCommandLineAction.ScopingParameterGroupName.'
-      );
-    }
-    if (this.remainder) {
-      throw new Error(
-        'Unscoped remainder parameters are not allowed. Remainder parameters can only be defined on ' +
-          'the scoped parameter provider in onDefineScopedParameters().'
-      );
-    }
-
-    // Consume the remainder of the command-line, which will later be passed the scoped parser.
-    // This will also prevent developers from calling this.defineCommandLineRemainder(...) since
-    // we will have already defined it.
-    this.defineCommandLineRemainder({
-      description:
-        'Scoped parameters.  Must be prefixed with "--", ex. "-- --scopedParameter ' +
-        'foo --scopedFlag".  For more information on available scoped parameters, use "-- --help".'
-    });
-  }
-
   /**
    * Retrieves the scoped CommandLineParser, which is populated after the ScopedCommandLineAction is executed.
    * @internal
@@ -270,10 +259,7 @@ export abstract class ScopedCommandLineAction extends CommandLineAction {
   }
 
   /**
-   * The child class should implement this hook to define its unscoped command-line parameters,
-   * e.g. by calling defineFlagParameter(). At least one scoping parameter must be defined.
-   * Scoping parameters are defined by setting the parameterGroupName to
-   * ScopedCommandLineAction.ScopingParameterGroupName.
+   * @deprecated - Define parameters in the constructor
    */
   protected onDefineUnscopedParameters?(): void;
 

libraries/ts-command-line/src/test/ActionlessParser.test.ts

@@ -5,27 +5,25 @@ import { CommandLineParser } from '../providers/CommandLineParser';
 import type { CommandLineFlagParameter } from '../parameters/CommandLineFlagParameter';
 
 class TestCommandLine extends CommandLineParser {
-  public flag!: CommandLineFlagParameter;
+  public flag: CommandLineFlagParameter;
   public done: boolean = false;
 
   public constructor() {
     super({
       toolFilename: 'example',
       toolDescription: 'An example project'
     });
-  }
-
-  protected async onExecute(): Promise<void> {
-    await super.onExecute();
-    this.done = true;
-  }
 
-  protected onDefineParameters(): void {
     this.flag = this.defineFlagParameter({
       parameterLongName: '--flag',
       description: 'The flag'
     });
   }
+
+  protected async onExecute(): Promise<void> {
+    await super.onExecute();
+    this.done = true;
+  }
 }
 
 describe(`Actionless ${CommandLineParser.name}`, () => {

libraries/ts-command-line/src/test/AliasedCommandLineAction.test.ts

@@ -32,19 +32,17 @@ class TestAction extends CommandLineAction {
       summary: 'does the action',
       documentation: 'a longer description'
     });
-  }
-
-  protected async onExecute(): Promise<void> {
-    expect(this._flag.value).toEqual(true);
-    this.done = true;
-  }
 
-  protected onDefineParameters(): void {
     this._flag = this.defineFlagParameter({
       parameterLongName: '--flag',
       description: 'The flag'
     });
   }
+
+  protected async onExecute(): Promise<void> {
+    expect(this._flag.value).toEqual(true);
+    this.done = true;
+  }
 }
 
 class TestScopedAction extends ScopedCommandLineAction {

libraries/ts-command-line/src/test/CommandLineParser.test.ts

@@ -7,27 +7,25 @@ import { CommandLineParser } from '../providers/CommandLineParser';
 
 class TestAction extends CommandLineAction {
   public done: boolean = false;
-  private _flag!: CommandLineFlagParameter;
+  private _flag: CommandLineFlagParameter;
 
   public constructor() {
     super({
       actionName: 'do:the-job',
       summary: 'does the job',
       documentation: 'a longer description'
     });
-  }
 
-  protected async onExecute(): Promise<void> {
-    expect(this._flag.value).toEqual(true);
-    this.done = true;
-  }
-
-  protected onDefineParameters(): void {
     this._flag = this.defineFlagParameter({
       parameterLongName: '--flag',
       description: 'The flag'
     });
   }
+
+  protected async onExecute(): Promise<void> {
+    expect(this._flag.value).toEqual(true);
+    this.done = true;
+  }
 }
 
 class TestCommandLine extends CommandLineParser {
@@ -39,10 +37,6 @@ class TestCommandLine extends CommandLineParser {
 
     this.addAction(new TestAction());
   }
-
-  protected onDefineParameters(): void {
-    // no parameters
-  }
 }
 
 describe(CommandLineParser.name, () => {

libraries/ts-command-line/src/test/ConflictingCommandLineParser.test.ts

@@ -19,26 +19,17 @@ class GenericCommandLine extends CommandLineParser {
 
 class TestAction extends CommandLineAction {
   public done: boolean = false;
-  private _scope1Arg!: CommandLineStringParameter;
-  private _scope2Arg!: CommandLineStringParameter;
-  private _nonConflictingArg!: CommandLineStringParameter;
+  private _scope1Arg: CommandLineStringParameter;
+  private _scope2Arg: CommandLineStringParameter;
+  private _nonConflictingArg: CommandLineStringParameter;
 
   public constructor() {
     super({
       actionName: 'do:the-job',
       summary: 'does the job',
       documentation: 'a longer description'
     });
-  }
 
-  protected async onExecute(): Promise<void> {
-    expect(this._scope1Arg.value).toEqual('scope1value');
-    expect(this._scope2Arg.value).toEqual('scope2value');
-    expect(this._nonConflictingArg.value).toEqual('nonconflictingvalue');
-    this.done = true;
-  }
-
-  protected onDefineParameters(): void {
     // Used to validate that conflicting parameters with different scopes return different values
     this._scope1Arg = this.defineStringParameter({
       parameterLongName: '--arg',
@@ -62,6 +53,13 @@ class TestAction extends CommandLineAction {
       description: 'The argument'
     });
   }
+
+  protected async onExecute(): Promise<void> {
+    expect(this._scope1Arg.value).toEqual('scope1value');
+    expect(this._scope2Arg.value).toEqual('scope2value');
+    expect(this._nonConflictingArg.value).toEqual('nonconflictingvalue');
+    this.done = true;
+  }
 }
 
 class UnscopedDuplicateArgumentTestAction extends CommandLineAction {
@@ -71,19 +69,14 @@ class UnscopedDuplicateArgumentTestAction extends CommandLineAction {
       summary: 'does the job',
       documentation: 'a longer description'
     });
-  }
-
-  protected async onExecute(): Promise<void> {
-    throw new Error('This action should not be executed');
-  }
 
-  protected onDefineParameters(): void {
     // Used to validate that conflicting parameters with at least one being unscoped fails
     this.defineStringParameter({
       parameterLongName: '--arg',
       argumentName: 'ARG',
       description: 'The argument'
     });
+
     // Used to validate that conflicting parameters with at least one being unscoped fails
     this.defineStringParameter({
       parameterLongName: '--arg',
@@ -92,6 +85,10 @@ class UnscopedDuplicateArgumentTestAction extends CommandLineAction {
       description: 'The argument'
     });
   }
+
+  protected async onExecute(): Promise<void> {
+    throw new Error('This action should not be executed');
+  }
 }
 
 class ScopedDuplicateArgumentTestAction extends CommandLineAction {
@@ -101,13 +98,7 @@ class ScopedDuplicateArgumentTestAction extends CommandLineAction {
       summary: 'does the job',
       documentation: 'a longer description'
     });
-  }
 
-  protected async onExecute(): Promise<void> {
-    throw new Error('This action should not be executed');
-  }
-
-  protected onDefineParameters(): void {
     // Used to validate that conflicting parameters with at least one being unscoped fails
     this.defineStringParameter({
       parameterLongName: '--arg',
@@ -123,6 +114,10 @@ class ScopedDuplicateArgumentTestAction extends CommandLineAction {
       description: 'The argument'
     });
   }
+
+  protected async onExecute(): Promise<void> {
+    throw new Error('This action should not be executed');
+  }
 }
 
 describe(`Conflicting ${CommandLineParser.name}`, () => {