fix: support node 14 again (#741)

* fix: support node 14 again

* fix: support reading from stdin in node 14

* fix: avoid spread operator on context

* fix: add token to context

* test: ut for last

* chore: code review

---------

Co-authored-by: mshanemc <shane.mclaughlin@salesforce.com>

.github/workflows/test.yml

@@ -13,6 +13,23 @@ jobs:
   windows-unit-tests:
     needs: linux-unit-tests
     uses: salesforcecli/github-workflows/.github/workflows/unitTestsWindows.yml@main
+  node14:
+    needs: linux-unit-tests
+    strategy:
+      matrix:
+        os: ["ubuntu-latest", "windows-latest"]
+        node_version: [14.x]
+      fail-fast: false
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.node_version }}
+          cache: yarn
+      - run: yarn install
+      - run: yarn build
+      - run: yarn test
   e2e:
     needs: linux-unit-tests
     strategy:

src/interfaces/parser.ts

@@ -287,7 +287,7 @@ export type BooleanFlag<T> = FlagProps & BooleanFlagProps & {
    * specifying a default of false is the same as not specifying a default
    */
   default?: FlagDefault<boolean>;
-  parse: (input: boolean, context: Command, opts: FlagProps & BooleanFlagProps) => Promise<T>
+  parse: (input: boolean, context: FlagParserContext, opts: FlagProps & BooleanFlagProps) => Promise<T>
 }
 
 export type OptionFlagDefaults<T, P = CustomOptions, M = false> = FlagProps & OptionFlagProps & {
@@ -325,7 +325,7 @@ export type Input<TFlags extends FlagOutput, BFlags extends FlagOutput, AFlags e
   baseFlags?: FlagInput<BFlags>;
   args?: ArgInput<AFlags>;
   strict?: boolean;
-  context?: Command;
+  context?: ParserContext;
   '--'?: boolean;
 }
 
@@ -334,10 +334,14 @@ export type ParserInput = {
   flags: FlagInput<any>;
   args: ArgInput<any>;
   strict: boolean;
-  context: Command | undefined;
+  context: ParserContext | undefined;
   '--'?: boolean;
 }
 
+export type ParserContext = Command & {
+  token?: FlagToken | ArgToken;
+}
+
 export type CompletionContext = {
   args?: { [name: string]: string };
   flags?: { [name: string]: string };

src/parser/parse.ts

@@ -1,8 +1,24 @@
 /* eslint-disable no-await-in-loop */
 import {ArgInvalidOptionError, CLIError, FlagInvalidOptionError} from './errors'
-import {ArgToken, BooleanFlag, CompletableFlag, FlagToken, Metadata, MetadataFlag, OptionFlag, OutputArgs, OutputFlags, ParserInput, ParserOutput, ParsingToken} from '../interfaces/parser'
+import {
+  ArgParserContext,
+  ArgToken,
+  BooleanFlag,
+  CompletableFlag,
+  FlagParserContext,
+  FlagToken,
+  Metadata,
+  MetadataFlag,
+  OptionFlag,
+  OutputArgs,
+  OutputFlags,
+  ParserContext,
+  ParserInput,
+  ParserOutput,
+  ParsingToken,
+} from '../interfaces/parser'
 import * as readline from 'readline'
-import {isTruthy, pickBy} from '../util'
+import {isTruthy, last, pickBy} from '../util'
 
 let debug: any
 try {
@@ -12,16 +28,44 @@ try {
   debug = () => {}
 }
 
-const readStdin = async (): Promise<string | null> => {
-  const {stdin, stdout} = process
-  debug('stdin.isTTY', stdin.isTTY)
+/**
+ * Support reading from stdin in Node 14 and older.
+ *
+ * This generally works for Node 14 and older EXCEPT when it's being
+ * run from another process, in which case it will hang indefinitely. Because
+ * of that issue we updated this to use AbortController but since AbortController
+ * is only available in Node 16 and newer, we have to keep this legacy version.
+ *
+ * See these issues for more details on the hanging indefinitely bug:
+ * https://github.com/oclif/core/issues/330
+ * https://github.com/oclif/core/pull/363
+ *
+ * @returns Promise<string | null>
+ */
+const readStdinLegacy = async (): Promise<string | null> => {
+  const {stdin} = process
+  let result
   if (stdin.isTTY) return null
+  result = ''
+  stdin.setEncoding('utf8')
+  for await (const chunk of stdin) {
+    result += chunk
+  }
+
+  return result
+}
+
+const readStdinWithTimeout = async (): Promise<string | null> => {
+  const {stdin, stdout} = process
 
   // process.stdin.isTTY is true whenever it's running in a terminal.
   // process.stdin.isTTY is undefined when it's running in a pipe, e.g. echo 'foo' | my-cli command
   // process.stdin.isTTY is undefined when it's running in a spawned process, even if there's no pipe.
   // This means that reading from stdin could hang indefinitely while waiting for a non-existent pipe.
   // Because of this, we have to set a timeout to prevent the process from hanging.
+
+  if (stdin.isTTY) return null
+
   return new Promise(resolve => {
     let result = ''
     const ac = new AbortController()
@@ -53,6 +97,16 @@ const readStdin = async (): Promise<string | null> => {
   })
 }
 
+const readStdin = async (): Promise<string | null> => {
+  const {stdin, version} = process
+  debug('stdin.isTTY', stdin.isTTY)
+
+  const nodeMajorVersion = Number(version.split('.')[0].replace(/^v/, ''))
+  debug('node version', nodeMajorVersion)
+
+  return nodeMajorVersion >= 14 ? readStdinLegacy() : readStdinWithTimeout()
+}
+
 function isNegativeNumber(input: string): boolean {
   return /^-\d/g.test(input)
 }
@@ -65,12 +119,12 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
   private readonly booleanFlags: { [k: string]: BooleanFlag<any> }
   private readonly flagAliases: { [k: string]: BooleanFlag<any> | OptionFlag<any> }
 
-  private readonly context: any
+  private readonly context: ParserContext
 
   private currentFlag?: OptionFlag<any>
 
   constructor(private readonly input: T) {
-    this.context = input.context || {}
+    this.context = input.context ?? {} as ParserContext
     this.argv = [...input.argv]
     this._setNames()
     this.booleanFlags = pickBy(input.flags, f => f.type === 'boolean') as any
@@ -222,15 +276,25 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
       return input
     }
 
-    const parseFlagOrThrowError = async (input: any, flag: BooleanFlag<any> | OptionFlag<any>, token?: FlagToken, context: typeof this.context = {}) => {
+    const parseFlagOrThrowError = async (input: any, flag: BooleanFlag<any> | OptionFlag<any>, context: ParserContext | undefined, token?: FlagToken) => {
       if (!flag.parse) return input
 
+      const ctx = {
+        ...context,
+        token,
+        error: context?.error,
+        exit: context?.exit,
+        log: context?.log,
+        logToStderr: context?.logToStderr,
+        warn: context?.warn,
+      } as FlagParserContext
+
       try {
         if (flag.type === 'boolean') {
-          return await flag.parse(input, {...context, token}, flag)
+          return await flag.parse(input, ctx, flag)
         }
 
-        return await flag.parse(input, {...context, token}, flag)
+        return await flag.parse(input, ctx, flag)
       } catch (error: any) {
         error.message = `Parsing --${flag.name} \n\t${error.message}\nSee more help with --help`
         throw error
@@ -245,8 +309,16 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
       // user provided some input
       if (tokenLength) {
         // boolean
-        if (fws.inputFlag.flag.type === 'boolean' && fws.tokens?.at(-1)?.input) {
-          return {...fws, valueFunction: async (i: FlagWithStrategy) => parseFlagOrThrowError(i.tokens?.at(-1)?.input !== `--no-${i.inputFlag.name}`, i.inputFlag.flag, i.tokens?.at(-1), this.context)}
+        if (fws.inputFlag.flag.type === 'boolean' && last(fws.tokens)?.input) {
+          return {
+            ...fws,
+            valueFunction: async (i: FlagWithStrategy) => parseFlagOrThrowError(
+              last(i.tokens)?.input !== `--no-${i.inputFlag.name}`,
+              i.inputFlag.flag,
+              this.context,
+              last(i.tokens),
+            ),
+          }
         }
 
         // multiple with custom delimiter
@@ -256,31 +328,60 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
               ((i.tokens ?? []).flatMap(token => (token.input as string).split(i.inputFlag.flag.delimiter as string)))
               // trim, and remove surrounding doubleQuotes (which would hav been needed if the elements contain spaces)
               .map(v => v.trim().replace(/^"(.*)"$/, '$1').replace(/^'(.*)'$/, '$1'))
-              .map(async v => parseFlagOrThrowError(v, i.inputFlag.flag, {...i.tokens?.at(-1) as FlagToken, input: v}, this.context)),
+              .map(async v => parseFlagOrThrowError(v, i.inputFlag.flag, this.context, {...last(i.tokens) as FlagToken, input: v})),
             )).map(v => validateOptions(i.inputFlag.flag as OptionFlag<any>, v)),
           }
         }
 
         // multiple in the oclif-core style
         if (fws.inputFlag.flag.type === 'option' && fws.inputFlag.flag.multiple) {
-          return {...fws, valueFunction: async (i: FlagWithStrategy) => Promise.all((fws.tokens ?? []).map(token => parseFlagOrThrowError(validateOptions(i.inputFlag.flag as OptionFlag<any>, token.input as string), i.inputFlag.flag, token, this.context)))}
+          return {
+            ...fws,
+            valueFunction: async (i: FlagWithStrategy) =>
+              Promise.all(
+                (fws.tokens ?? []).map(token => parseFlagOrThrowError(
+                  validateOptions(i.inputFlag.flag as OptionFlag<any>, token.input as string),
+                  i.inputFlag.flag,
+                  this.context,
+                  token,
+                )),
+              ),
+          }
         }
 
         // simple option flag
         if (fws.inputFlag.flag.type === 'option') {
-          return {...fws, valueFunction: async (i: FlagWithStrategy) => parseFlagOrThrowError(validateOptions(i.inputFlag.flag as OptionFlag<any>, fws.tokens?.at(-1)?.input as string), i.inputFlag.flag, fws.tokens?.at(-1), this.context)}
+          return {
+            ...fws,
+            valueFunction: async (i: FlagWithStrategy) => parseFlagOrThrowError(
+              validateOptions(i.inputFlag.flag as OptionFlag<any>, last(fws.tokens)?.input as string),
+              i.inputFlag.flag,
+              this.context,
+              last(fws.tokens),
+            ),
+          }
         }
       }
 
       // no input: env flags
       if (fws.inputFlag.flag.env && process.env[fws.inputFlag.flag.env]) {
         const valueFromEnv = process.env[fws.inputFlag.flag.env]
         if (fws.inputFlag.flag.type === 'option' && valueFromEnv) {
-          return {...fws, valueFunction: async (i: FlagWithStrategy) => parseFlagOrThrowError(validateOptions(i.inputFlag.flag as OptionFlag<any>, valueFromEnv), i.inputFlag.flag, this.context)}
+          return {
+            ...fws,
+            valueFunction: async (i: FlagWithStrategy) => parseFlagOrThrowError(
+              validateOptions(i.inputFlag.flag as OptionFlag<any>, valueFromEnv),
+              i.inputFlag.flag,
+              this.context,
+            ),
+          }
         }
 
         if (fws.inputFlag.flag.type === 'boolean') {
-          return {...fws, valueFunction: async (i: FlagWithStrategy) => Promise.resolve(isTruthy(process.env[i.inputFlag.flag.env as string] ?? 'false'))}
+          return {
+            ...fws,
+            valueFunction: async (i: FlagWithStrategy) => Promise.resolve(isTruthy(process.env[i.inputFlag.flag.env as string] ?? 'false')),
+          }
         }
       }
 
@@ -381,11 +482,12 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['flags']
     const args = {} as Record<string, unknown>
     const tokens = this._argTokens
     let stdinRead = false
-    const ctx = this.context
+    const ctx = this.context as ArgParserContext
 
     for (const [name, arg] of Object.entries(this.input.args)) {
       const token = tokens.find(t => t.arg === name)
-      ctx.token = token
+      ctx.token = token!
+
       if (token) {
         if (arg.options && !arg.options.includes(token.input)) {
           throw new ArgInvalidOptionError(arg, token.input)

src/util.ts

@@ -22,6 +22,11 @@ export function uniqBy<T>(arr: T[], fn: (cur: T) => any): T[] {
   })
 }
 
+export function last<T>(arr?: T[]): T | undefined {
+  if (!arr) return
+  return arr.slice(-1)[0]
+}
+
 type SortTypes = string | number | undefined | boolean
 
 export function sortBy<T>(arr: T[], fn: (i: T) => SortTypes | SortTypes[]): T[] {

test/util.test.ts

@@ -1,5 +1,5 @@
 import {expect} from 'chai'
-import {maxBy, sumBy, capitalize, ensureArgObject} from '../src/util'
+import {maxBy, sumBy, capitalize, ensureArgObject, last} from '../src/util'
 
 describe('capitalize', () => {
   it('capitalizes the string', () => {
@@ -34,6 +34,22 @@ describe('maxBy', () => {
   })
 })
 
+describe('last', () => {
+  it('returns undefined for empty array', () => {
+    expect(last([])).to.be.undefined
+  })
+  it('returns undefined for undefined', () => {
+    expect(last()).to.be.undefined
+  })
+  it('returns last value in the array', () => {
+    const arr: Item[] = [{x: 1}, {x: 3}, {x: 2}]
+    expect(last(arr)).to.equal(arr[2])
+  })
+  it('returns only item in array', () => {
+    expect(last([6])).to.equal(6)
+  })
+})
+
 describe('ensureArgObject', () => {
   it('should convert array of arguments to an object', () => {
     const args = [