Skip to content

Commit

Permalink
(fix): don't mutate process.argv with arr.splice
Browse files Browse the repository at this point in the history
- 1.7.1 introduced some arr.splice's into the code, which caused
  mutations in process.argv, affecting downstream code

- set arr to a clone of process.argv instead so it can be freely
  mutated after
- explicitly call the parameter processArgv so code is written more
  carefully when dealing with it
- add a test to ensure process.argv is not mutated
  • Loading branch information
agilgur5 committed Feb 16, 2020
1 parent ff2ed46 commit 8a6a568
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ class Sade {
return this;
}

parse(arr, opts={}) {
parse(processArgv, opts={}) {
let arr = processArgv.slice(); // DON'T mutate process.argv, clone first
let offset=2, tmp, idx, isVoid, cmd;
let alias = { h:'help', v:'version' };
let argv = mri(arr.slice(offset), { alias });
Expand Down
14 changes: 14 additions & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,20 @@ test('prog.action (multi optional)', t => {
(c=true) && run(); // +4 tests
});

test('prog.parse', t => {
t.plan(1);

let ctx = sade('foo')
.command('build')
.action(() => {});

let args = ['', '', 'build'];
let argsCopy = args.slice();
ctx.parse(argsCopy);

t.deepEqual(argsCopy, args, '~> process.argv is not mutated');
});

test('prog.parse :: lazy', t => {
t.plan(14);

Expand Down

0 comments on commit 8a6a568

Please sign in to comment.