fix!: calling parse multiple times now appropriately maintains state (#1137) (#1369)

mleguen · Benjamin E. Coe · commit 026b1514d47d · 2019-07-28T19:35:59.000-07:00
BREAKING CHANGE: previously to this fix methods like `yargs.getOptions()` contained the state of the last command to execute.
diff --git a/lib/command.js b/lib/command.js
@@ -186,24 +186,17 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
       // a function can be provided, which builds
       // up a yargs chain and possibly returns it.
       innerYargs = commandHandler.builder(yargs.reset(parsed.aliases))
-      // if the builder function did not yet parse argv with reset yargs
-      // and did not explicitly set a usage() string, then apply the
-      // original command string as usage() for consistent behavior with
-      // options object below.
-      if (yargs.parsed === false) {
-        if (shouldUpdateUsage(yargs)) {
-          yargs.getUsageInstance().usage(
-            usageFromParentCommandsCommandHandler(parentCommands, commandHandler),
-            commandHandler.description
-          )
-        }
-        innerArgv = innerYargs ? innerYargs._parseArgs(null, null, true, commandIndex) : yargs._parseArgs(null, null, true, commandIndex)
-      } else {
-        innerArgv = yargs.parsed.argv
+      if (!innerYargs || (typeof innerYargs._parseArgs !== 'function')) {
+        innerYargs = yargs
       }
-
-      if (innerYargs && yargs.parsed === false) aliases = innerYargs.parsed.aliases
-      else aliases = yargs.parsed.aliases
+      if (shouldUpdateUsage(innerYargs)) {
+        innerYargs.getUsageInstance().usage(
+          usageFromParentCommandsCommandHandler(parentCommands, commandHandler),
+          commandHandler.description
+        )
+      }
+      innerArgv = innerYargs._parseArgs(null, null, true, commandIndex)
+      aliases = innerYargs.parsed.aliases
     } else if (typeof commandHandler.builder === 'object') {
       // as a short hand, an object can instead be provided, specifying
       // the options that a command takes.
@@ -419,18 +412,19 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
   // the state of commands such that
   // we can apply .parse() multiple times
   // with the same yargs instance.
-  let frozen
+  let frozens = []
   self.freeze = () => {
-    frozen = {}
+    let frozen = {}
+    frozens.push(frozen)
     frozen.handlers = handlers
     frozen.aliasMap = aliasMap
     frozen.defaultCommand = defaultCommand
   }
   self.unfreeze = () => {
+    let frozen = frozens.pop()
     handlers = frozen.handlers
     aliasMap = frozen.aliasMap
     defaultCommand = frozen.defaultCommand
-    frozen = undefined
   }
 
   return self
diff --git a/lib/usage.js b/lib/usage.js
@@ -512,9 +512,10 @@ module.exports = function usage (yargs, y18n) {
     return self
   }
 
-  let frozen
+  let frozens = []
   self.freeze = function freeze () {
-    frozen = {}
+    let frozen = {}
+    frozens.push(frozen)
     frozen.failMessage = failMessage
     frozen.failureOutput = failureOutput
     frozen.usages = usages
@@ -525,6 +526,7 @@ module.exports = function usage (yargs, y18n) {
     frozen.descriptions = descriptions
   }
   self.unfreeze = function unfreeze () {
+    let frozen = frozens.pop()
     failMessage = frozen.failMessage
     failureOutput = frozen.failureOutput
     usages = frozen.usages
@@ -533,7 +535,6 @@ module.exports = function usage (yargs, y18n) {
     examples = frozen.examples
     commands = frozen.commands
     descriptions = frozen.descriptions
-    frozen = undefined
   }
 
   return self
diff --git a/lib/validation.js b/lib/validation.js
@@ -323,18 +323,19 @@ module.exports = function validation (yargs, usage, y18n) {
     return self
   }
 
-  let frozen
+  let frozens = []
   self.freeze = function freeze () {
-    frozen = {}
+    let frozen = {}
+    frozens.push(frozen)
     frozen.implied = implied
     frozen.checks = checks
     frozen.conflicting = conflicting
   }
   self.unfreeze = function unfreeze () {
+    let frozen = frozens.pop()
     implied = frozen.implied
     checks = frozen.checks
     conflicting = frozen.conflicting
-    frozen = undefined
   }
 
   return self
diff --git a/test/yargs.js b/test/yargs.js
@@ -835,6 +835,29 @@ describe('yargs dsl tests', () => {
 
       r.logs[0].should.match(/Commands:[\s\S]*blerg command/)
     })
+
+    it('can be called multiple times with the same behavior', () => {
+      let counter = { foobar: 0 }
+      yargs(['test', 'foobar'])
+        .command(
+          'test <name>',
+          'increases counter',
+          yargs => yargs.positional('name', {
+            aliases: 'n',
+            describe: 'a name',
+            choices: ['foobar'],
+            type: 'string'
+          }),
+          argv => { counter[argv.name]++ }
+        )
+        .fail((msg) => {
+          expect.fail(undefined, undefined, msg)
+        })
+      yargs.parse()
+      yargs.parse()
+      yargs.parse()
+      expect(counter.foobar).to.equal(3)
+    })
   })
 
   describe('parsed', () => {
@@ -2060,17 +2083,23 @@ describe('yargs dsl tests', () => {
     })
 
     it('allows a defaultDescription to be set', () => {
-      yargs('cmd')
+      const r = checkOutput(() => yargs('cmd --help').wrap(null)
         .command('cmd [heroes...]', 'a command', (yargs) => {
           yargs.positional('heroes', {
             default: ['batman', 'Iron Man'],
             defaultDescription: 'batman and Iron Man'
           })
         }).parse()
-
-      yargs.getOptions().defaultDescription.should.deep.equal({
-        heroes: 'batman and Iron Man'
-      })
+      )
+      r.logs.join('\n').split(/\n+/).should.deep.equal([
+        'usage cmd [heroes...]',
+        'a command',
+        'Positionals:',
+        '  heroes  [array] [default: batman and Iron Man]',
+        'Options:',
+        '  --help     Show help  [boolean]',
+        '  --version  Show version number  [boolean]'
+      ])
     })
 
     it('allows an implied argument to be specified', (done) => {
diff --git a/yargs.js b/yargs.js
@@ -144,9 +144,10 @@ function Yargs (processArgs, cwd, parentRequire) {
   self.resetOptions()
 
   // temporary hack: allow "freezing" of reset-able state for parse(msg, cb)
-  let frozen
+  let frozens = []
   function freeze () {
-    frozen = {}
+    let frozen = {}
+    frozens.push(frozen)
     frozen.options = options
     frozen.configObjects = options.configObjects.slice(0)
     frozen.exitProcess = exitProcess
@@ -160,8 +161,11 @@ function Yargs (processArgs, cwd, parentRequire) {
     frozen.exitError = exitError
     frozen.hasOutput = hasOutput
     frozen.parsed = self.parsed
+    frozen.parseFn = parseFn
+    frozen.parseContext = parseContext
   }
   function unfreeze () {
+    let frozen = frozens.pop()
     options = frozen.options
     options.configObjects = frozen.configObjects
     exitProcess = frozen.exitProcess
@@ -175,9 +179,8 @@ function Yargs (processArgs, cwd, parentRequire) {
     command.unfreeze()
     strict = frozen.strict
     completionCommand = frozen.completionCommand
-    parseFn = null
-    parseContext = null
-    frozen = undefined
+    parseFn = frozen.parseFn
+    parseContext = frozen.parseContext
   }
 
   self.boolean = function (keys) {
@@ -538,8 +541,11 @@ function Yargs (processArgs, cwd, parentRequire) {
   let parseContext = null
   self.parse = function parse (args, shortCircuit, _parseFn) {
     argsert('[string|array] [function|boolean|object] [function]', [args, shortCircuit, _parseFn], arguments.length)
+    freeze()
     if (typeof args === 'undefined') {
-      return self._parseArgs(processArgs)
+      const parsed = self._parseArgs(processArgs)
+      unfreeze()
+      return parsed
     }
 
     // a context object can optionally be provided, this allows
@@ -560,7 +566,6 @@ function Yargs (processArgs, cwd, parentRequire) {
     // skipping validation, etc.
     if (!shortCircuit) processArgs = args
 
-    freeze()
     if (parseFn) exitProcess = false
 
     const parsed = self._parseArgs(args, shortCircuit)
