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

Support subclassing Command #1002

Closed
ngocdaothanh opened this issue Jul 25, 2019 · 4 comments
Closed

Support subclassing Command #1002

ngocdaothanh opened this issue Jul 25, 2019 · 4 comments

Comments

@ngocdaothanh
Copy link

Currently, the return type of the methods in class Command is Command:

public command(name: string, desc?: string, opts?: commander.CommandOptions): Command;

I cannot extend Command to add more methods, like this:

class MyCommandWithUtilities extends Command {
  public util1(): this {
     // ...
     return this;
  }
}

const cmd = new MyCommandWithUtilities();
cmd
  .command('foo')
  .util1()  // Syntax error, because `command` returns `Command`, not `MyCommandWithUtilities`
  .option(...);

So, please change the return type of the methods in Command to this.

public command(name: string, desc?: string, opts?: commander.CommandOptions): this;

It seems that simply changing the definitions in index.d.ts is not enough. The main source code must also be changed.

See:
microsoft/TypeScript#4910

@ngocdaothanh
Copy link
Author

public command(name: string, desc?: string, opts?: commander.CommandOptions): this;

It's not that simple, because the current design is to return a new Command instance, unless desc is specified; the return type is Command | this.

So to support subclassing, we need to change the design of command.

@shadowspawn
Copy link
Collaborator

Yes, for action handler based subcommands (no description), .command returns a new object representing the new command. (That might be doable using a factory method to create the new object, but definitely more complicated than just changing the TypeScript definition for .command.)

@ngocdaothanh
Copy link
Author

Workaround: use a wrapper.

Example (TypeScript):

export class MyCommand {
  private readonly cmd: commander.Command;

  constructor(cmd?: commander.Command) {
    this.cmd = cmd ? cmd : new commander.Command();
  }

  public command(name: string): MyCommand {
    const cmd = this.cmd.command(name);
    return new MyCommand(cmd);
  }

  public description(desc: string): this {
    this.cmd.description(desc);
    return this;
  }

  public util1(): this {
     // ...
     return this;
  }

  ...
}

@shadowspawn
Copy link
Collaborator

See #1191 for work in progress which resolves this issue.

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

No branches or pull requests

2 participants