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

Missing this type for action #2145

Open
matthyk opened this issue Feb 5, 2024 · 5 comments
Open

Missing this type for action #2145

matthyk opened this issue Feb 5, 2024 · 5 comments
Labels
enhancement pending release Merged into a branch for a future release, but not released yet

Comments

@matthyk
Copy link

matthyk commented Feb 5, 2024

commander.js/lib/command.js

Lines 511 to 527 in 83c3f4e

action(fn) {
const listener = (args) => {
// The .action callback takes an extra parameter which is the command or options.
const expectedArgsCount = this.registeredArguments.length;
const actionArgs = args.slice(0, expectedArgsCount);
if (this._storeOptionsAsProperties) {
actionArgs[expectedArgsCount] = this; // backwards compatible "options"
} else {
actionArgs[expectedArgsCount] = this.opts();
}
actionArgs.push(this);
return fn.apply(this, actionArgs);
};
this._actionHandler = listener;
return this;
}

The passed action function fn receives the current command as this context in line 523. However, this context is missing in the current typings.
action(fn: (...args: any[]) => void | Promise<void>): this;

Happy to provide a PR for this!

@shadowspawn
Copy link
Collaborator

shadowspawn commented Feb 5, 2024

I had seen this recommended in the Effective TypeScript book, but not acted on it as I was not familiar with the pattern (Item 49: Provide a Type for this in Callbacks).

@shadowspawn
Copy link
Collaborator

shadowspawn commented Feb 5, 2024

We have more advanced typings and try out some typing improvements in: https://github.com/commander-js/extra-typings

If you open a PR there, we can make it available there first before bringing it back to Commander.

@matthyk
Copy link
Author

matthyk commented Feb 6, 2024

Done with commander-js/extra-typings#59 🎉

@shadowspawn
Copy link
Collaborator

shadowspawn commented May 19, 2024

The change has been out for a couple of months in https://github.com/commander-js/extra-typings without causing reported issues.

Would you still like to make a PR here @matthyk ?

@matthyk
Copy link
Author

matthyk commented May 19, 2024

Absolutely!

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pending release Merged into a branch for a future release, but not released yet
Projects
None yet
Development

No branches or pull requests

2 participants