diff --git a/common/changes/@rushstack/ts-command-line/ts-command-line-recommendation-updates_2024-05-15-20-16.json b/common/changes/@rushstack/ts-command-line/ts-command-line-recommendation-updates_2024-05-15-20-16.json new file mode 100644 index 00000000000..1b45e2752c8 --- /dev/null +++ b/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" +} \ No newline at end of file diff --git a/common/reviews/api/ts-command-line.api.md b/common/reviews/api/ts-command-line.api.md index b7bf3a85e81..6bf206e51d7 100644 --- a/common/reviews/api/ts-command-line.api.md +++ b/common/reviews/api/ts-command-line.api.md @@ -217,6 +217,7 @@ export abstract class CommandLineParameterProvider { getParameterStringMap(): Record; getStringListParameter(parameterLongName: string, parameterScope?: string): CommandLineStringListParameter; getStringParameter(parameterLongName: string, parameterScope?: string): CommandLineStringParameter; + // @deprecated (undocumented) protected onDefineParameters?(): void; get parameters(): ReadonlyArray; get parametersProcessed(): boolean; @@ -450,8 +451,8 @@ export abstract class ScopedCommandLineAction extends CommandLineAction { _executeAsync(): Promise; // @internal protected _getScopedCommandLineParser(): CommandLineParser; - protected onDefineParameters(): void; protected abstract onDefineScopedParameters(scopedParameterProvider: CommandLineParameterProvider): void; + // @deprecated (undocumented) protected onDefineUnscopedParameters?(): void; protected abstract onExecute(): Promise; get parameters(): ReadonlyArray; diff --git a/libraries/ts-command-line/README.md b/libraries/ts-command-line/README.md index 39d0dec42c9..55d4e6b05e2 100644 --- a/libraries/ts-command-line/README.md +++ b/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 { // 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 { // abstract + await BusinessLogic.doTheWork(this._force.value, this._protocol.value || "(none)"); + } } ``` @@ -111,9 +109,7 @@ export class WidgetCommandLine extends CommandLineParser { }); this.addAction(new PushAction()); - } - protected onDefineParameters(): void { // abstract this._verbose = this.defineFlagParameter({ parameterLongName: '--verbose', parameterShortName: '-v', @@ -121,9 +117,9 @@ export class WidgetCommandLine extends CommandLineParser { }); } - protected onExecute(): Promise { // override + protected async onExecute(): Promise { // 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); }); diff --git a/libraries/ts-command-line/src/providers/CommandLineParameterProvider.ts b/libraries/ts-command-line/src/providers/CommandLineParameterProvider.ts index c533bd8c280..5b0d211c630 100644 --- a/libraries/ts-command-line/src/providers/CommandLineParameterProvider.ts +++ b/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; diff --git a/libraries/ts-command-line/src/providers/ScopedCommandLineAction.ts b/libraries/ts-command-line/src/providers/ScopedCommandLineAction.ts index 5cd6b318f09..e5e005b1d0e 100644 --- a/libraries/ts-command-line/src/providers/ScopedCommandLineAction.ts +++ b/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; diff --git a/libraries/ts-command-line/src/test/ActionlessParser.test.ts b/libraries/ts-command-line/src/test/ActionlessParser.test.ts index c1703cb5118..b8fe4218a66 100644 --- a/libraries/ts-command-line/src/test/ActionlessParser.test.ts +++ b/libraries/ts-command-line/src/test/ActionlessParser.test.ts @@ -5,7 +5,7 @@ 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() { @@ -13,19 +13,17 @@ class TestCommandLine extends CommandLineParser { toolFilename: 'example', toolDescription: 'An example project' }); - } - - protected async onExecute(): Promise { - await super.onExecute(); - this.done = true; - } - protected onDefineParameters(): void { this.flag = this.defineFlagParameter({ parameterLongName: '--flag', description: 'The flag' }); } + + protected async onExecute(): Promise { + await super.onExecute(); + this.done = true; + } } describe(`Actionless ${CommandLineParser.name}`, () => { diff --git a/libraries/ts-command-line/src/test/AliasedCommandLineAction.test.ts b/libraries/ts-command-line/src/test/AliasedCommandLineAction.test.ts index 2516ed74723..69b83e67024 100644 --- a/libraries/ts-command-line/src/test/AliasedCommandLineAction.test.ts +++ b/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 { - 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 { + expect(this._flag.value).toEqual(true); + this.done = true; + } } class TestScopedAction extends ScopedCommandLineAction { diff --git a/libraries/ts-command-line/src/test/CommandLineParser.test.ts b/libraries/ts-command-line/src/test/CommandLineParser.test.ts index 1a5ed964ad8..f86ed17aeba 100644 --- a/libraries/ts-command-line/src/test/CommandLineParser.test.ts +++ b/libraries/ts-command-line/src/test/CommandLineParser.test.ts @@ -7,7 +7,7 @@ import { CommandLineParser } from '../providers/CommandLineParser'; class TestAction extends CommandLineAction { public done: boolean = false; - private _flag!: CommandLineFlagParameter; + private _flag: CommandLineFlagParameter; public constructor() { super({ @@ -15,19 +15,17 @@ class TestAction extends CommandLineAction { summary: 'does the job', documentation: 'a longer description' }); - } - protected async onExecute(): Promise { - 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 { + 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, () => { diff --git a/libraries/ts-command-line/src/test/ConflictingCommandLineParser.test.ts b/libraries/ts-command-line/src/test/ConflictingCommandLineParser.test.ts index cd820014f2d..35321eaa4fe 100644 --- a/libraries/ts-command-line/src/test/ConflictingCommandLineParser.test.ts +++ b/libraries/ts-command-line/src/test/ConflictingCommandLineParser.test.ts @@ -19,9 +19,9 @@ 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({ @@ -29,16 +29,7 @@ class TestAction extends CommandLineAction { summary: 'does the job', documentation: 'a longer description' }); - } - protected async onExecute(): Promise { - 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 { + 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 { - 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 { + 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 { - 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 { + throw new Error('This action should not be executed'); + } } describe(`Conflicting ${CommandLineParser.name}`, () => { diff --git a/libraries/ts-command-line/src/test/ScopedCommandLineAction.test.ts b/libraries/ts-command-line/src/test/ScopedCommandLineAction.test.ts index 5600bb623da..f8f50ea4272 100644 --- a/libraries/ts-command-line/src/test/ScopedCommandLineAction.test.ts +++ b/libraries/ts-command-line/src/test/ScopedCommandLineAction.test.ts @@ -12,8 +12,8 @@ import type { CommandLineFlagParameter } from '../parameters/CommandLineFlagPara class TestScopedAction extends ScopedCommandLineAction { public done: boolean = false; public scopedValue: string | undefined; - private _verboseArg!: CommandLineFlagParameter; - private _scopeArg!: CommandLineStringParameter; + private _verboseArg: CommandLineFlagParameter; + private _scopeArg: CommandLineStringParameter; private _scopedArg: CommandLineStringParameter | undefined; public constructor() { @@ -22,17 +22,7 @@ class TestScopedAction extends ScopedCommandLineAction { summary: 'does the scoped action', documentation: 'a longer description' }); - } - - protected async onExecute(): Promise { - if (this._scopedArg) { - expect(this._scopedArg.longName).toBe(`--scoped-${this._scopeArg.value}`); - this.scopedValue = this._scopedArg.value; - } - this.done = true; - } - protected onDefineUnscopedParameters(): void { this._verboseArg = this.defineFlagParameter({ parameterLongName: '--verbose', description: 'A flag parameter.' @@ -46,6 +36,14 @@ class TestScopedAction extends ScopedCommandLineAction { }); } + protected async onExecute(): Promise { + if (this._scopedArg) { + expect(this._scopedArg.longName).toBe(`--scoped-${this._scopeArg.value}`); + this.scopedValue = this._scopedArg.value; + } + this.done = true; + } + protected onDefineScopedParameters(scopedParameterProvider: CommandLineParameterProvider): void { if (this._scopeArg.value) { this._scopedArg = scopedParameterProvider.defineStringParameter({