Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ts-command-line] Parameters in CommandLineParser onExecute are undefined #4708

Closed
Petah opened this issue May 15, 2024 · 4 comments
Closed

Comments

@Petah
Copy link

Petah commented May 15, 2024

Summary

I followed the example in the readme, trying to add a --verbose parameter to all actions, but in the onExecute method this._verbose is undefined.

Repro steps

import { CommandLineAction, CommandLineChoiceParameter, CommandLineFlagParameter, CommandLineParser } from '@rushstack/ts-command-line';

export class PushAction extends CommandLineAction {
    private _force!: CommandLineFlagParameter;
    private _protocol!: CommandLineChoiceParameter;

    public constructor() {
        super({
            actionName: 'push',
            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)");
        console.log('PushAction.onExecute', this._force.value, this._protocol.value);
        return Promise.resolve();
    }

    protected onDefineParameters(): void { // abstract
        this._force = this.defineFlagParameter({
            parameterLongName: '--force',
            parameterShortName: '-f',
            description: 'Push and overwrite any existing state'
        });

        this._protocol = this.defineChoiceParameter({
            parameterLongName: '--protocol',
            description: 'Specify the protocol to use',
            alternatives: ['ftp', 'webdav', 'scp'],
            environmentVariable: 'WIDGET_PROTOCOL',
            defaultValue: 'scp'
        });
    }
}

export class WidgetCommandLine extends CommandLineParser {
    private _verbose!: CommandLineFlagParameter;

    public constructor() {
        super({
            toolFilename: 'widget',
            toolDescription: 'The "widget" tool is a code sample for using the @rushstack/ts-command-line library.',
        });

        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
        console.log('WidgetCommandLine.onExecute', this._verbose);
        return super.onExecute();
    }
}

const commandLine: WidgetCommandLine = new WidgetCommandLine();
commandLine.execute();

Expected result:

WidgetCommandLine.onExecute [the verbose parameter object]
PushAction.onExecute true scp

Actual result:

WidgetCommandLine.onExecute undefined
PushAction.onExecute true scp

Details

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
Package name: @rushstack/ts-command-line
Package version? 4.19.5
Operating system? Mac
Would you consider contributing a PR? No
Node.js version (node -v)? v21.7.1
@Petah Petah changed the title [ts-command-line] Parameters defined in CommandLineParser are undefined [ts-command-line] Parameters undefined in CommandLineParser are undefined May 15, 2024
@Petah Petah changed the title [ts-command-line] Parameters undefined in CommandLineParser are undefined [ts-command-line] Parameters in CommandLineParser onExecute are undefined May 15, 2024
@iclanton
Copy link
Member

iclanton commented May 15, 2024

I'm not getting a repro of this issue. I've created a repo here with your example and invited you to it: https://github.com/iclanton/rushstack-4708-repro. Can you update that with your repro?

This is the output I get from various commands (I've run this on Windows and Linux, but not Mac):

> node lib\test
usage: widget [-h] [-v] <command> ...

The "widget" tool is a code sample for using the @rushstack/ts-command-line
library.

Positional arguments:
  <command>
    push         Pushes a widget to the service

Optional arguments:
  -h, --help     Show this help message and exit.
  -v, --verbose  Show extra logging detail

For detailed help about a specific command, use: widget <command> -h

> node lib\test push
WidgetCommandLine.onExecute CommandLineFlagParameter {
  longName: '--verbose',
  _shortNameValue: '-v',
  parameterGroup: undefined,
  parameterScope: undefined,
  description: 'Show extra logging detail',
  required: false,
  environmentVariable: undefined,
  undocumentedSynonyms: undefined,
  allowNonStandardEnvironmentVariableNames: undefined,
  _value: false,
  kind: 1,
  _parserKey: 'key_0'
}
PushAction.onExecute false scp

@Petah
Copy link
Author

Petah commented May 15, 2024

Hmm, it seems to be an issue with my tsconfig.json. Not sure if thats a bug, or just incorrect setup on my part, but I have updated the tsconfig.json in that repo.

> node lib/test.js --verbose push --force
WidgetCommandLine.onExecute undefined
PushAction.onExecute true scp
> ./node_modules/.bin/ts-node src/test.ts --verbose push --force
WidgetCommandLine.onExecute undefined
PushAction.onExecute true scp

@iclanton
Copy link
Member

This issue is the "target": "es2022" compiler option. See TypeScript docs here: https://www.typescriptlang.org/docs/handbook/2/classes.html#initialization-order

Comparing the TypeScript compiler output for es2021 vs es2022:

class WidgetCommandLine extends ts_command_line_1.CommandLineParser {
+    _verbose;
    constructor() {
        super({
            toolFilename: "widget",
            toolDescription: 'The "widget" tool is a code sample for using the @rushstack/ts-command-line library.',
        });
        this.addAction(new PushAction());
    }
    onDefineParameters() {
        // abstract
        this._verbose = this.defineFlagParameter({
            parameterLongName: "--verbose",
            parameterShortName: "-v",
            description: "Show extra logging detail",
        });
    }
    onExecute() {
        // override
        console.log("WidgetCommandLine.onExecute", this._verbose);
        return super.onExecute();
    }
}

Notice that the _verbose field is now emitted in the class. The onDefineParameters function is called by the CommandLineParser's constructor, which causes the problem. From the TypeScript docs, the initialization order is:

  1. The base class fields are initialized - _verbose is not defined at this point
  2. The base class constructor runs - **onDefineParameters is called, and _verbose is now defined as expected`
  3. The derived class fields are initialized - the _verbose field initializer, emitted by TS in >=ES2022, is executed and redefines _verbose as undefined
  4. The derived class constructor runs - other stuff happens, like the this.addAction(new PushAction()); line

To fix this issue, target <es2022, or move the parameter declarations into the WidgetCommandLine constructor, like ths:

export class WidgetCommandLine extends CommandLineParser {
-  private _verbose!: CommandLineFlagParameter;
+  private _verbose: CommandLineFlagParameter;

  public constructor() {
    super({
      toolFilename: "widget",
      toolDescription:
        'The "widget" tool is a code sample for using the @rushstack/ts-command-line library.',
    });

    this.addAction(new PushAction());
-  }
-
-  protected onDefineParameters(): void {
-    // abstract
+
+  // define parameters
    this._verbose = this.defineFlagParameter({
      parameterLongName: "--verbose",
      parameterShortName: "-v",
      description: "Show extra logging detail",
    });
  }

  protected onExecute(): Promise<void> {
    // override
    console.log("WidgetCommandLine.onExecute", this._verbose);
    return super.onExecute();
  }
}

We should probably consider removing the onDefineParameters function and, instead, explicitly recommending that users of ts-command-line initialize parameters directly in the constructor. This allows the removal of the non-null assertion on the _verbose field.

@iclanton
Copy link
Member

Closing this out, since it seems like we've figured out the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants