Repository: oclif/core
Pull request #751 (2023-07-26 21:38:55)

feat: deprecate scopedEnvVarKey for scopedEnvVarKeys which accounts for binAliases
@W-13825184@


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f6756d1..5e11222 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,12 @@
+## [2.10.1](https://github.com/oclif/core/compare/2.10.0...2.10.1) (2023-07-28)
+
+
+### Bug Fixes
+
+* support node 14 again ([#741](https://github.com/oclif/core/issues/741)) ([a80c4fd](https://github.com/oclif/core/commit/a80c4fd74c7c5dc7b8426e6f78fc60689c82eab7))
+
+
+
 # [2.10.0](https://github.com/oclif/core/compare/2.9.5...2.10.0) (2023-07-25)
 
 
diff --git a/package.json b/package.json
index 341f436..8d8e9f5 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@oclif/core",
   "description": "base library for oclif CLIs",
-  "version": "2.10.0",
+  "version": "2.10.1",
   "author": "Salesforce",
   "bugs": "https://github.com/oclif/core/issues",
   "dependencies": {
diff --git a/src/command.ts b/src/command.ts
index fe44263..a57c3ed 100644
--- a/src/command.ts
+++ b/src/command.ts
@@ -227,7 +227,7 @@ export abstract class Command {
     let result
     try {
       // remove redirected env var to allow subsessions to run autoupdated client
-      delete process.env[this.config.scopedEnvVarKey('REDIRECTED')]
+      this.config.scopedEnvVarKeys('REDIRECTED').map(key => delete process.env[key])
       await this.init()
       result = await this.run()
     } catch (error: any) {
diff --git a/src/config/config.ts b/src/config/config.ts
index 42bc294..6b0413e 100644
--- a/src/config/config.ts
+++ b/src/config/config.ts
@@ -396,14 +396,19 @@ export class Config implements IConfig {
   }
 
   public scopedEnvVar(k: string): string | undefined {
-    return process.env[this.scopedEnvVarKey(k)]
+    return process.env[this.scopedEnvVarKeys(k).find(k => process.env[k]) as string]
   }
 
   public scopedEnvVarTrue(k: string): boolean {
-    const v = process.env[this.scopedEnvVarKey(k)]
+    const v = process.env[this.scopedEnvVarKeys(k).find(k => process.env[k]) as string]
     return v === '1' || v === 'true'
   }
 
+  /**
+   * this DOES NOT account for bin aliases, use scopedEnvVarKeys instead which will account for bin aliases
+   * @param {string} k, the unscoped key you want to get the value for
+   * @returns {string} returns the env var key
+   */
   public scopedEnvVarKey(k: string): string {
     return [this.bin, k]
     .map(p => p.replace(/@/g, '').replace(/[/-]/g, '_'))
@@ -411,6 +416,16 @@ export class Config implements IConfig {
     .toUpperCase()
   }
 
+  /**
+   * gets the scoped env var keys for a given key, including bin aliases
+   * @param {string} k, the env key e.g. 'debug'
+   * @returns {string[]} e.g. ['SF_DEBUG', 'SFDX_DEBUG']
+   */
+  public scopedEnvVarKeys(k: string): string[] {
+    return [this.bin, ...this.binAliases ?? []].map(alias =>
+      [alias.replace(/@/g, '').replace(/[/-]/g, '_'), k].join('_').toUpperCase())
+  }
+
   public findCommand(id: string, opts: { must: true }): Command.Loadable
 
   public findCommand(id: string, opts?: { must: boolean }): Command.Loadable | undefined
diff --git a/src/interfaces/config.ts b/src/interfaces/config.ts
index 9636aac..28ec10f 100644
--- a/src/interfaces/config.ts
+++ b/src/interfaces/config.ts
@@ -127,6 +127,7 @@ export interface Config {
   findMatches(id: string, argv: string[]): Command.Loadable[];
   scopedEnvVar(key: string): string | undefined;
   scopedEnvVarKey(key: string): string;
+  scopedEnvVarKeys(key: string): string[];
   scopedEnvVarTrue(key: string): boolean;
   s3Url(key: string): string;
   s3Key(type: 'versioned' | 'unversioned', ext: '.tar.gz' | '.tar.xz', options?: Config.s3Key.Options): string;
diff --git a/src/interfaces/parser.ts b/src/interfaces/parser.ts
index 58abb38..631b4ed 100644
--- a/src/interfaces/parser.ts
+++ b/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 };
diff --git a/src/parser/parse.ts b/src/parser/parse.ts
index b3aa0c1..9187e38 100644
--- a/src/parser/parse.ts
+++ b/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,19 +328,38 @@ 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),
+            ),
+          }
         }
       }
 
@@ -276,11 +367,21 @@ export class Parser<T extends ParserInput, TFlags extends OutputFlags<T['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)
diff --git a/src/util.ts b/src/util.ts
index b3b4cdb..2240890 100644
--- a/src/util.ts
+++ b/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 patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 16cae5f..d5263d1 100644
--- a/.github/workflows/test.yml
+++ b/.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:
diff --git a/test/config/config.test.ts b/test/config/config.test.ts
index 43f7ee5..7de416f 100644
--- a/test/config/config.test.ts
+++ b/test/config/config.test.ts
@@ -22,7 +22,7 @@ const pjson = {
   files: [],
   commands: {},
   oclif: {
-    binAliases: ['foo', 'bar'],
+    binAliases: ['bar', 'baz'],
     topics: {
       t1: {
         description: 'desc for t1',
@@ -103,7 +103,55 @@ describe('Config', () => {
   describe('binAliases', () => {
     testConfig({pjson})
     .it('will have binAliases set', config => {
-      expect(config.binAliases).to.deep.equal(['foo', 'bar'])
+      expect(config.binAliases).to.deep.equal(['bar', 'baz'])
+    })
+
+    testConfig({pjson}).it('will get scoped env vars with bin aliases', config => {
+      expect(config.scopedEnvVarKeys('abc')).to.deep.equal(['FOO_ABC', 'BAR_ABC', 'BAZ_ABC'])
+    })
+
+    testConfig({pjson}).it('will get scoped env vars', config => {
+      expect(config.scopedEnvVarKey('abc')).to.equal('FOO_ABC')
+    })
+
+    testConfig({pjson}).it('will get scopedEnvVar', config => {
+      process.env.FOO_ABC = 'find me'
+      expect(config.scopedEnvVar('abc')).to.deep.equal('find me')
+      delete process.env.FOO_ABC
+    })
+
+    testConfig({pjson}).it('will get scopedEnvVar via alias', config => {
+      process.env.BAZ_ABC = 'find me'
+      expect(config.scopedEnvVar('abc')).to.deep.equal('find me')
+      delete process.env.BAZ_ABC
+    })
+
+    testConfig({pjson}).it('will get scoped env vars', config => {
+      expect(config.scopedEnvVarKey('abc')).to.equal('FOO_ABC')
+    })
+
+    testConfig({pjson}).it('will get scopedEnvVarTrue', config => {
+      process.env.FOO_ABC = 'true'
+      expect(config.scopedEnvVarTrue('abc')).to.equal(true)
+      delete process.env.FOO_ABC
+    })
+
+    testConfig({pjson}).it('will get scopedEnvVarTrue via alias', config => {
+      process.env.BAR_ABC = 'true'
+      expect(config.scopedEnvVarTrue('abc')).to.equal(true)
+      delete process.env.BAR_ABC
+    })
+
+    testConfig({pjson}).it('will get scopedEnvVarTrue=1', config => {
+      process.env.FOO_ABC = '1'
+      expect(config.scopedEnvVarTrue('abc')).to.equal(true)
+      delete process.env.FOO_ABC
+    })
+
+    testConfig({pjson}).it('will get scopedEnvVarTrue=1 via alias', config => {
+      process.env.BAR_ABC = '1'
+      expect(config.scopedEnvVarTrue('abc')).to.equal(true)
+      delete process.env.BAR_ABC
     })
   })
 
diff --git a/test/util.test.ts b/test/util.test.ts
index 90b4faa..42a42e0 100644
--- a/test/util.test.ts
+++ b/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 = [