Question: proper way for cloning a command

[mdrozdovz]

Hello, it's a bit unusual request since lib is intended for one-shot execution.
However, I have some functionality where I'd like to re-use existing commands during app lifetime. Basically, exposing a remote code execution interface for internal use.

In order to avoid duplication, here's the snippet I use for cloning recursively:

const cloneCommand = (cmd) => {
    const clone = new Command()
        .copyInheritedSettings(cmd)
        .name(cmd.name())

    if (cmd.description()) clone.description(cmd.description())
    if (cmd.version()) clone.version(cmd.version())
    if (cmd.alias()) clone.alias(cmd.alias())

    cmd.registeredArguments.forEach(clone.addArgument.bind(clone))
    cmd.options.filter(o => !['version'].includes(o.name())).forEach(clone.addOption.bind(clone))
    cmd.commands.forEach(c => clone.addCommand(cloneCommand(c)))

    clone._actionHandler = cmd._actionHandler
    clone._optionValueSources = cmd._optionValueSources

    return clone
}

and a bit of cleaning up after cloning:

const executable = cloneCommand(program)
        .showHelpAfterError(true)
        .showSuggestionAfterError(true)
executable._lifeCycleHooks = {}

Usage:

executable.exitOverride(err => { ... })
executable.configureOutput({ ... })
await executable.parseAsync(args, { from: 'user' })

This however, doesn't parse options properly and they remain set to default values (or undefined). Works good for arguments though.

So the question is: what am I doing wrong during cloning or what am I missing? Any better way or plans to add cloning support out-of-the-box?

Thanks!

[shadowspawn]

Support was added in Commander v13 for calling parse more than once: #2299

I suggest you see if that works for your use case, it may Just Work.

---

There is no plan to add cloning support.

The options get implemented by adding an event handler to the parent command. I am not sure why that isn't working from visual inspection of your example code.

commander.js/lib/command.js

Lines 721 to 724 in 395cf71

	this.on('option:' + oname, (val) => {
	const invalidValueMessage = `error: option '${option.flags}' argument '${val}' is invalid.`;
	handleOptionValue(val, invalidValueMessage, 'cli');
	});

[mdrozdovz]

Thanks for the quick reply!

Was using v12 previously. Calling parse on existing program Commander instance works like a charm. However, it's not quite feasible for me to use this approach due to life cycle hooks which are shared.
One way to deal with it is to set and restore hooks after command execution, but that brings potentially more issues than it resolves.

Anyways, thanks for the suggestion, will dig more into cloning and options parsing (original approach)!

[mdrozdovz]

More info:

program.command('test-some')
    .argument('<argument>')
    .option('-o, --option <option>', 'Some option')
    .action(testSome)

export const testSome = async (args, options, cmd) => {
    console.log({args, options})
}

when cloned yields the following:

// await clone.parseAsync([ 'test-some', '12312', '-o', 'Value' ], { from: 'user' })
{ args: '12312', options: {} }

[shadowspawn]

One problem with cloneCommand is the stored action handler is not just the callback function and includes context from the original command:

commander.js/lib/command.js

Line 557 in 395cf71

action(fn) {

So this line in cloneCommand is insufficient;

clone._actionHandler = cmd._actionHandler

I did a quick hack after the clone to set the action handler through the method, and then the option parsing did something:

clonedProgram.commands[0].action(testSome);

[shadowspawn]

I don't think this line should be needed in cloneCommand. The _optionValues and _optionValueSources will get modified from the empty initial values as options are added to the clones command, and as parsing occurs.

    clone._optionValueSources = cmd._optionValueSources

[shadowspawn]

For interest, what is the problem with the life cycle hooks and calling parse more than once? If it isn't too complicated to explain!

(The stored hooks are just the supplied callbacks, so I thought they might work ok.)

[mdrozdovz]

One problem with cloneCommand is the stored action handler is not just the callback function and includes context from the original command:

That was indeed the issue, I didn't notice at first, thank you! Now, since listener is an arrow function it's not possible to re-bind it to the cloned command. But that's fine, may end up using own fork of Commander for that purpose. Though requires minimal change:

const listener = function (args) {
    ...
};

Usage: clone._actionHandler = cmd._actionHandler?.bind(clone)

Are there some caveats I'm missing for using listener as arrow function specifically? If not maybe, we could get this merged (I'll prepare a PR)?

I don't think this line should be needed in cloneCommand

Indeed, removed.

For interest, what is the problem with the life cycle hooks and calling parse more than once? If it isn't too complicated to explain!

For a context: Application was started as a group of CLI commands for administration and business purposes. I have put resource cleanup/connections closing in the postAction hook, pretty common practice. Later on it, as app was growing, web server was attached to it, and now I'd like to move some of the administration commands from exclusively CLI to a web server.

However, running resource cleanup during web server lifetime, spinned up via Commander as well, is definitely not desirable behavior, but still needed for a CLI. So I clear hooks manually for cloned commands exposed to a web server.

I wouldn't say my case is common, so using workarounds is totally fine.

[shadowspawn]

I don't like the code accessing or changing the implementation of the private _actionHandler. My thought is storing the original callback and reapplying it. Like:

class MyCommand extends Command {
  #actionCallback = null; // keep track of the supplied action handler
  
  createCommand(name) {
      const cmd = new MyCommand(name);
      return cmd;
  }

  action(handler) {
      this.#actionCallback = handler;
      return super.action(handler);
  }

  cloneCommand() {
    const clone = new MyCommand()
      .copyInheritedSettings(this)
      .name(this.name())

    if (this.description()) clone.description(this.description())
    if (this.version()) clone.version(this.version())
    if (this.alias()) clone.alias(this.alias())

    this.registeredArguments.forEach(clone.addArgument.bind(clone))
    this.options.filter(o => !['version'].includes(o.name())).forEach(clone.addOption.bind(clone))
    this.commands.forEach(c => clone.addCommand(c.cloneCommand()));

    if (this.#actionCallback) clone.action(this.#actionCallback);  // <--- define action handler using public method
    return clone;
  }
}

[mdrozdovz]

I don't like the code accessing or changing the implementation of the private _actionHandler.

Agree, I don't like touching private state either. Totally forgot Command is a class which can be extended. Thanks for the suggestion and your time! Now closing issue for real 😄