Skip to content

Export the InferOptions and InferArguments type helpers - #62

Closed
nipunn1313 wants to merge 1 commit into
commander-js:developfrom
nipunn1313:nipunn/export_infer
Closed

nipunn1313 wants to merge 1 commit into
commander-js:developfrom
nipunn1313:nipunn/export_infer

Conversation

@nipunn1313

Copy link
Copy Markdown

This allows callers to write helper methods that manipulate commands (eg to add options to a preexisting command)

This is technically already possible if you rely on type inference, but I've run into:

error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.

Pull Request

Problem

It is not possible (I believe) to type methods like this

export function addMyCustomOptions<
  Args extends any[],
  Opts extends OptionValues
>(command: Command<Args, Opts>, action: string) {
  return command.option("--custom-thing");
}

If ts can infer the return type, things are fine, but sometimes it gives:
error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.

In this case, I believe we would need InferOptions which is private

Solution

Make InferOptions type exported.

ChangeLog

This allows callers to write helper methods that manipulate commands (eg
to add options to a preexisting command)

This is technically already possible if you rely on type inference, but
I've run into:

error TS7056: The inferred type of this node exceeds the maximum length
the compiler will serialize. An explicit type annotation is needed.
@nipunn1313

Copy link
Copy Markdown
Author

I am open to other solutions. These exported types would work, but don't seem ideal.

@shadowspawn

Copy link
Copy Markdown
Contributor

If ts can infer the return type, things are fine, but sometimes it gives:

error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.

Did you see this error on a branch where you had added this to the action handler?

We saw some similar problems in some different approaches to the type inference:

@nipunn1313

Copy link
Copy Markdown
Author

It wasn't a this it was just a long chain of

export function addMyOptions<...>(command: Command<Args, Opts>) {
  command
    .addOption(new Option())
    .addOption(new Option())
    // .{about 5-7 of these, some including .conflicts()}
}

The output type of that function was not inferable - and tsc gave that error.

I actually found a workaround which was splitting the function into two halves and having all the callsites call both of the methods. The workaround kinda sucks. You can't write a helper to call the two helpers because it overloads tsc in this way.

@shadowspawn

Copy link
Copy Markdown
Contributor

Somewhat related issue: #32

@nipunn1313

Copy link
Copy Markdown
Author

yeah - perhaps there's some way to to reduce complexity on typescript via some kind of optimization to mitigate this.

I was running into it with just 5-7 options which feels... too quick.

Seems like typescript has some limit at 50 recursion depth https://github.com/bluepichu/ctf-challenges/blob/d83b02dffe2c7d2404c3fa0c27da886d61ff4c98/plaidctf-2020/thats_a_lot_of_fish/handout/tsc.js.diff

@shadowspawn

Copy link
Copy Markdown
Contributor

I have not got my head around the issue yet. I found a mention of that error message with a fix from a Typescript guru but not sure if it is relevant. Posting for future reading!

@shadowspawn

Copy link
Copy Markdown
Contributor

Did you see this error on a branch where you had added this to the action handler?

That was a misguided question sorry. A few PR were opened in short time frame and I assumed they were all related!

@shadowspawn

Copy link
Copy Markdown
Contributor

Have you got a reproduction? What version of Typescript? I am up to 20 options in a toy program and not had an error yet. I am interested in reproducing, and wondering whether it is a particular method call that is the problem, or basically just lots and lots of type inference.

function addMyOptions<Args extends any[] = [], Opts extends OptionValues = {}>(command: Command<Args, Opts>) {
    return command
        .addOption(new Option('-a, --aaa'))
        ...
        .addOption(new Option('--A <value>').preset('A').default('D'))
        ...
);

const program = new Command()
   .option('--port <number>', 'port number', parseFloat, 80)
   .argument('[rest...]');

const fullProgram = addMyOptions(program);

@nipunn1313

Copy link
Copy Markdown
Author

I have a non-minimal repro in our codebase. Let me see if I can extract it.

@shadowspawn

Copy link
Copy Markdown
Contributor

Was addMyOptions being included in a TypeScript definition file? I notice that the error in microsoft/TypeScript#43817 only occurs when generating a definition file.

@shadowspawn

shadowspawn commented Mar 16, 2024 •

Copy link
Copy Markdown
Contributor

It is technically easy to export more types, but I don't want to do so without understanding the problem it is solving. I will close this for now, but if you want to keep working on this @nipunn1313 then I can reopen.

Feel free to open a new issue if it comes up again, with new information and renewed interest.

Thank you for your contributions.

@mateussilva92

Copy link
Copy Markdown
Contributor

Maybe I have an example that would actually require this types to be exported. Building a class that extends from the Command class, if I'm overriding methods like addOption I need to type InferOptions to defined the return type.

There using type inference would simple not work.

If there's a solution that would work for this kind of example I would love to know about it.

@shadowspawn

shadowspawn commented Jan 20, 2025 •

Copy link
Copy Markdown
Contributor

@mateussilva92

There may be a case for exporting all the inference functions used directly in the class definitions even just for easy copy-paste from base class definitions. I have been leaning on return type inference in experiments to date.

I was able to use type inference for the addOption return type like this. Is your use case fundamentally different?

class MyCommand extends Command {
    createCommand(name?: string) {
        return new Command(name);
    }

    addOption<
        Usage extends string,
        PresetT,
        DefaultT,
        CoerceT,
        Mandatory extends boolean,
        ChoicesT,
  >(option: Option<Usage, PresetT, DefaultT, CoerceT, Mandatory, ChoicesT>) {
        const inferredCommand = super.addOption(option);
        return inferredCommand;
    }
}

@mateussilva92

Copy link
Copy Markdown
Contributor

@shadowspawn

Sorry for the late reply.

On your example the thing is that I can only return the type of the super class which is Command.

I can give a partial example of the class that I'm creating and where inference will not work.

In the code bellow I'm allowing the addOption method to receive an ExtendedOption instance which has extra properties.
The problem comes when I try to return the class instance to allow chaining like we can do with the Command class.

There I want to narrow the type of the Opts generic parameter to include the new option that was added.
For that to happen I need to InferOptions type which is not currently exported for the library.

export class ExtendedCommand<
  Args extends any[] = [],
  Opts extends OptionValues = {},
  GlobalOpts extends OptionValues = {},
> extends Command<Args, Opts, GlobalOpts> {
  constructor(name?: string) {
    super(name);
  }

  addOption<
    Usage extends string,
    PresetT,
    DefaultT,
    CoerceT,
    Mandatory extends boolean,
    ChoicesT,
  >(
    option:
      | ExtendedOption<Usage, PresetT, DefaultT, CoerceT, Mandatory, ChoicesT>
      | Option<Usage, PresetT, DefaultT, CoerceT, Mandatory, ChoicesT>
  ): ExtendedCommand<
    Args,
    InferOptions<Opts, Usage, DefaultT, CoerceT, Mandatory, PresetT, ChoicesT>,
    GlobalOpts
  > {
    super.addOption(option);

    return this as unknown as ExtendedCommand<
      Args,
      InferOptions<
        Opts,
        Usage,
        DefaultT,
        CoerceT,
        Mandatory,
        PresetT,
        ChoicesT
      >,
      GlobalOpts
    >;
  }
}

@shadowspawn

Copy link
Copy Markdown
Contributor

@shadowspawn

Copy link
Copy Markdown
Contributor

Added in v14.0.0: export InferArgument, InferArguments, InferCommandArguments, and InferOptions

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

Successfully merging this pull request may close these issues.

3 participants