JSON output breaks if a custom base Command is defined.

[richburdon]

JSON output breaks if a custom base Command is defined.

[mdonnalley]

@richburdon thanks for creating the issue. I have a couple of comments

1. For the --json flag to show up for the status command's help. You need to set static flags = {}. However, I don't like that pattern so I have an incoming PR to make it so you don't need to do that anymore
2. this.parse is failing because it's parsing the flags for BaseCommand, which doesn't have enableJsonFlag, while executing Status, which does have enableJsonFlag. So your init method in BaseCommand needs a way to know which command class to parse

Off the top of my head you could accomplish this by maintaining a map of your command classes, e.g.

const classes: Record<string, typeof Command> = {
  Status,
}

export abstract class BaseCommand extends Command {
  static _globalFlags = {
    config: Flags.string({
      description: 'Specify config file',
    }),
  };

  async init(): Promise<void> {
    await super.init()
    const {flags} = await this.parse(classes[this.constructor.name])
  }
}

[richburdon]

Hello Mike -- thanks for the quick response.

Re the --json flag, something very strange happens depending on the order in which flags and enableJsonFlag are declared.

// This works
export default class Status extends BaseCommand {
  static enableJsonFlag = true;
  static flags = {};
  async run (): Promise<{ status: number }> {
    return { status: 200 };
  }
}
▶ ./bin/dev kube status --help
USAGE
  $ dx kube status [--json] [--config <value>]

// This doesn't
export default class Status extends BaseCommand {
  static flags = {};
  static enableJsonFlag = true;
  async run (): Promise<{ status: number }> {
    return { status: 200 };
  }
}
▶ ./bin/dev kube status --help
USAGE
  $ dx kube status [--config <value>]

Re the init/parse. Strangely, calling parse with undefined works:

  async init (): Promise<void> {
    await super.init();
    const { flags } = await this.parse(undefined); // This works!
    const { config: configFile } = flags;
    if (fs.existsSync(configFile)) {
      this.userConfig = yaml.load(String(fs.readFileSync(configFile))) as any;
    }
  }

Otherwise I was going to add a new param to the constructor requiring derived classes to pass down their type -- but it seems to me that it was a reasonable thing to want to do in the init method -- and have the framework support this.

I think my suggestion would be to either pass a config object into the constructor, or require a virtual method where this config state is provided. Then the base class and caller can access this information. I feel this is more stable than relying on overloading properties. (Just one view point.)

class MyCommand extends Command {
  constructor () {
    super({
       flags: ...
       jsonRequired: ...
    })
  }
}

Thanks again. RB