fix: support multiple values after --ignore-files again (#1652)

The yargs upgrade from 6.6.0 to 13.2.1 in edebef4 broke the documented ability to pass multiple files to ignore after the `--ignore-files` (aka `-i`) parameter. This is because yargs 11 started to treat `requiresArg` as an implicit `nargs: 1`, i.e. to accept exactly one value only. This patch fixes the issue by dropping `requiresArg` and validating the number of parameters in the `execute` function instead.
mozilla · Jul 12, 2019 · ff0ffa5 · ff0ffa5
1 parent 626a180
commit ff0ffa5
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 1 deletion.
diff --git a/src/program.js b/src/program.js
@@ -166,6 +166,12 @@ export class Program {
 
     const argv = this.yargs.argv;
 
+    // Replacement for the "requiresArg: true" parameter until the following bug
+    // is fixed: https://github.com/yargs/yargs/issues/1098
+    if (argv.ignoreFiles && !argv.ignoreFiles.length) {
+      throw new UsageError('Not enough arguments following: ignore-files');
+    }
+
     const cmd = argv._[0];
 
     const version = getVersion(this.absolutePackageDir);
@@ -348,7 +354,10 @@ Example: $0 --help run.
                 'ignored. (Example: --ignore-files=path/to/first.js ' +
                 'path/to/second.js "**/*.log")',
       demandOption: false,
-      requiresArg: true,
+      // The following option prevents yargs>=11 from parsing multiple values,
+      // so the minimum value requirement is enforced in execute instead.
+      // Upstream bug: https://github.com/yargs/yargs/issues/1098
+      // requiresArg: true,
       type: 'array',
     },
     'no-input': {

diff --git a/tests/unit/test.program.js b/tests/unit/test.program.js
@@ -709,6 +709,30 @@ describe('program.main', () => {
 
     sinon.assert.called(logStream.makeVerbose);
   });
+
+  it('requires a parameter after --ignore-files', async () => {
+    const fakeCommands = fake(commands);
+    return execProgram(['build', '--ignore-files'], {commands: fakeCommands})
+      .then(makeSureItFails())
+      .catch((error) => {
+        assert.match(
+          error.message, /Not enough arguments following: ignore-files/);
+      });
+  });
+
+  it('supports multiple parameters after --ignore-files', async () => {
+    const fakeCommands = fake(commands, {
+      build: () => Promise.resolve(),
+    });
+    return execProgram(
+      ['build', '--ignore-files', 'f1', 'f2', '-a', 'xxx', '-i', 'f4', 'f3'],
+      {commands: fakeCommands})
+      .then(() => {
+        const options = fakeCommands.build.firstCall.args[0];
+        assert.deepEqual(options.ignoreFiles, ['f1', 'f2', 'f4', 'f3']);
+        assert.equal(options.artifactsDir, 'xxx');
+      });
+  });
 });
 
 describe('program.defaultVersionGetter', () => {