Class inheritance does not extend all functionality

[DanHulton]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

If you want to have a "parent command class" for global options, a common logging config, shared functions or constructor behaviour, it does not work as expected. Basic class functionality works, i.e. if you create a protected logger instance, that works, and shared functions work as expected, but trying to create an @Options definition that would be "global" among Commands that inherit this class does not work.

Minimum reproduction code

base-command.ts:

import { CommandRunner } from 'nest-commander';
import { InjectPinoLogger, PinoLogger } from 'nestjs-pino';

export abstract class BaseCommand extends CommandRunner {
  @InjectPinoLogger()
  protected readonly logger: PinoLogger;

  @Option({
    flags: '-c, --common <common>',
    description: 'A common flag',
  })
  parseCommon(val: string) {
    return val;
  }
}

test-command.ts:

import { Command, Option } from 'nest-commander';
import { BaseCommand } from './base-command';

@Command({
  name: 'test',
})
export class TestCommand extends BaseCommand {
  async run(inputs: string[], options: Record<string, any>): Promise<void> {
    this.logger.info(options, 'Options');
  }
}

Expected behavior

When running this code with the --common=something flag set, I would expect to see {'common': 'something'} in the logged options, but instead, the logged object is empty.

Package

• nest-commander
• nest-commander-schematics
• nest-commander-testing

Package version

3.7.1

Node.js version

18.16.0

In which operating systems have you tested?

• macOS
• Windows
• Linux

Other

No response

[jmcdo29]

I believe this most likely has to do with my use of @golevelup/nestjs-discovery and how it discovers metadata of each class. I'll see if I can reproduce the functionality away from nest-commander and report it to that library, or adjust my use if necessary

[ruscon]

@jmcdo29 any ideas how to do it ?

[jmcdo29]

Hmm, I was not able to reproduce this functionality in an e2e test I created for this issue. Can someone provide me a minimum reproduction

[nkoterba]

I just encountered this issue.

I have a base class with @Option defined (API url and auth token) and would like to use that base class for all my API "commands".

I have a command that inherits that base class. This command then has subcommands that also inherit the base class.

Diagram:

// Inheritance structure
BaseCommand.ts (defines @Option for url and token)
  | --- DeviceCommand.ts
  | --- GetCommand.ts 
  | --- ListCommand.ts (defines @Option for page and page-size)

// Command structure
DeviceCommand.ts
  | --- GetCommand.ts (sub-command)
  | --- LisCommand.ts (sub-command)

The issue here is really sub-commands and how they operate in commander.js.

Here is a commander.js comment that's relevant:
tj/commander.js#243

It sounds like updates in the commander.js now fix this situation.

Solutions that should work:

• I think the easiest option is to just use this.command.optsWithGlobals() in the run method of your sub-commands. I tested this and it works -- you can use this instead of the options value passed into the run method. See this PR and the documented example: Add support for getting merged options including globals tj/commander.js#1671
• Don't have parent command inherit the same base class as sub commands (e.g., Parent Command extends CommandRunner, sub-commands inherit custom BaseCommand.ts).
• Like the option above, have two different base classes: Parent command extend one; sub-commands extend the other.

Some additional commander.js examples that help showcase ways to access/use "global" options:

• https://github.com/tj/commander.js/blob/master/examples/global-options-added.js
• https://github.com/tj/commander.js/blob/master/examples/global-options-nested.js

@jmcdo29 We could update the nest-commander code or update the documentation on how to access global Options in a sub-command's run method.

I think the run method in CommandRunner could be updated to either include the global options as a third callback parameter or just return all options instead of the local ones (so both global and local ones).

Oddly enough, commander.js has several getOptionValue methods and although they have one to get the source of a global option, they don't have a method to get a global option value (at least as of version 11.1.0):