Missing this type for action

[matthyk]

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.

commander.js/typings/index.d.ts

Line 525 in 83c3f4e

action(fn: (...args: any[]) => void | Promise<void>): this;

Happy to provide a PR for this!

[shadowspawn]

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]

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]

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

[shadowspawn]

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]

Absolutely!