Skip to content

Commit

Permalink
Do not mutate program.args when calling action handler (#1048)
Browse files Browse the repository at this point in the history
  • Loading branch information
shadowspawn committed Sep 21, 2019
1 parent 5cac9d6 commit cc4a525
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 8 deletions.
16 changes: 8 additions & 8 deletions index.js
Expand Up @@ -346,16 +346,16 @@ Command.prototype.action = function(fn) {
}
});

// Always append ourselves to the end of the arguments,
// to make sure we match the number of arguments the user
// expects
if (self._args.length) {
args[self._args.length] = self;
} else {
args.push(self);
// The .action callback takes an extra parameter which is the command itself.
var expectedArgsCount = self._args.length;
var actionArgs = args.slice(0, expectedArgsCount);
actionArgs[expectedArgsCount] = self;
// Add the extra arguments so available too.
if (args.length > expectedArgsCount) {
actionArgs.push(args.slice(expectedArgsCount));
}

fn.apply(self, args);
fn.apply(self, actionArgs);
};
var parent = this.parent || this;
var name = parent === this ? '*' : this._name;
Expand Down
36 changes: 36 additions & 0 deletions tests/command.action.test.js
@@ -0,0 +1,36 @@
const commander = require('../');

// Test some behaviours of .action not covered in more specific tests.

test('when .action called then command passed to action', () => {
const actionMock = jest.fn();
const program = new commander.Command();
const cmd = program
.command('info')
.action(actionMock);
program.parse(['node', 'test', 'info']);
expect(actionMock).toHaveBeenCalledWith(cmd);
});

test('when .action called then program.args only contains args', () => {
// At one time program.args was being modified to contain the same args as the call to .action
// and so included the command as an extra and unexpected complex item in array.
const program = new commander.Command();
program
.command('info <file>')
.action(() => {});
program.parse(['node', 'test', 'info', 'my-file']);
expect(program.args).toEqual(['my-file']);
});

test('when .action called with extra arguments then extras also passed to action', () => {
// This is a new and undocumented behaviour for now.
// Might make this an error by default in future.
const actionMock = jest.fn();
const program = new commander.Command();
const cmd = program
.command('info <file>')
.action(actionMock);
program.parse(['node', 'test', 'info', 'my-file', 'a']);
expect(actionMock).toHaveBeenCalledWith('my-file', cmd, ['a']);
});

0 comments on commit cc4a525

Please sign in to comment.