fix(parser): cache stdin value in global scope (#935)

Currently, flags and args read from stdin are lost whenever Parser.parse is called multiple times. This PR caches the value read from stdin so that Parser.parse can be called multiple times.

This PR also:

* decreases timeout for reading stdin (from 100ms to 10ms)
* Flags with allowStdin: 'only' are no longer required to have - provided as the value (it's assumed)

src/parser/parse.ts

@@ -36,6 +36,20 @@ try {
   }
 }
 
+declare global {
+  /**
+   * Cache the stdin so that it can be read multiple times.
+   *
+   * This fixes a bug where the stdin would be read multiple times (because Parser.parse() was called more than once)
+   * but only the first read would be successful - all other reads would return null.
+   *
+   * Storing in global is necessary because we want the cache to be shared across all versions of @oclif/core in
+   * in the dependency tree. Storing in a variable would only share the cache within the same version of @oclif/core.
+   */
+  // eslint-disable-next-line no-var
+  var oclif: {stdinCache?: string}
+}
+
 export const readStdin = async (): Promise<null | string> => {
   const {stdin, stdout} = process
 
@@ -47,11 +61,16 @@ export const readStdin = async (): Promise<null | string> => {
 
   if (stdin.isTTY) return null
 
+  if (global.oclif?.stdinCache) {
+    debug('resolved stdin from global cache', global.oclif.stdinCache)
+    return global.oclif.stdinCache
+  }
+
   return new Promise((resolve) => {
     let result = ''
     const ac = new AbortController()
     const {signal} = ac
-    const timeout = setTimeout(() => ac.abort(), 100)
+    const timeout = setTimeout(() => ac.abort(), 10)
 
     const rl = createInterface({
       input: stdin,
@@ -66,6 +85,7 @@ export const readStdin = async (): Promise<null | string> => {
     rl.once('close', () => {
       clearTimeout(timeout)
       debug('resolved from stdin', result)
+      global.oclif = {...global.oclif, stdinCache: result}
       resolve(result)
     })
 
@@ -123,6 +143,7 @@ export class Parser<
   public async parse(): Promise<ParserOutput<TFlags, BFlags, TArgs>> {
     this._debugInput()
 
+    // eslint-disable-next-line complexity
     const parseFlag = async (arg: string): Promise<boolean> => {
       const {isLong, name} = this.findFlag(arg)
       if (!name) {
@@ -151,22 +172,25 @@ export class Parser<
 
         this.currentFlag = flag
         let input = isLong || arg.length < 3 ? this.argv.shift() : arg.slice(arg[2] === '=' ? 3 : 2)
-        // if the value ends up being one of the command's flags, the user didn't provide an input
-        if (typeof input !== 'string' || this.findFlag(input).name) {
-          throw new CLIError(`Flag --${name} expects a value`)
-        }
 
-        if (flag.allowStdin === 'only' && input !== '-') {
-          throw new CLIError(`Flag --${name} can only be read from stdin. The value must be "-".`)
+        if (flag.allowStdin === 'only' && input !== '-' && input !== undefined) {
+          throw new CLIError(
+            `Flag --${name} can only be read from stdin. The value must be "-" or not provided at all.`,
+          )
         }
 
-        if (flag.allowStdin && input === '-') {
+        if ((flag.allowStdin && input === '-') || flag.allowStdin === 'only') {
           const stdin = await readStdin()
           if (stdin) {
             input = stdin.trim()
           }
         }
 
+        // if the value ends up being one of the command's flags, the user didn't provide an input
+        if (typeof input !== 'string' || this.findFlag(input).name) {
+          throw new CLIError(`Flag --${name} expects a value`)
+        }
+
         this.raw.push({flag: flag.name, input, type: 'flag'})
       } else {
         this.raw.push({flag: flag.name, input: arg, type: 'flag'})

test/parser/parse.test.ts

@@ -1908,7 +1908,19 @@ describe('allowStdin', () => {
     expect(out.raw[0].input).to.equal('x')
   })
 
-  it('should throw if allowStdin is "only" but value is not "-"', async () => {
+  it('should read stdin as input for flag when allowStdin is "only" and no value is given', async () => {
+    sandbox.stub(parser, 'readStdin').returns(stdinPromise)
+    const out = await parse(['--myflag'], {
+      flags: {
+        myflag: Flags.string({allowStdin: 'only'}),
+      },
+    })
+
+    expect(out.flags.myflag).to.equals(stdinValue)
+    expect(out.raw[0].input).to.equal('x')
+  })
+
+  it('should throw if allowStdin is "only" but value is not "-" or undefined', async () => {
     sandbox.stub(parser, 'readStdin').returns(stdinPromise)
     try {
       await parse(['--myflag', 'INVALID'], {
@@ -1919,7 +1931,9 @@ describe('allowStdin', () => {
       expect.fail('Should have thrown an error')
     } catch (error) {
       if (error instanceof CLIError) {
-        expect(error.message).to.equal('Flag --myflag can only be read from stdin. The value must be "-".')
+        expect(error.message).to.equal(
+          'Flag --myflag can only be read from stdin. The value must be "-" or not provided at all.',
+        )
       } else {
         expect.fail('Should have thrown a CLIError')
       }