From 881eab624f9d5cfc9ff3563945e1bc8fe51370a4 Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Fri, 3 Jul 2026 00:47:01 +0000 Subject: [PATCH 1/2] [Performance] Memoize terminalSupportsPrompting Memoize the terminalSupportsPrompting function to avoid redundant environment variable lookups and TTY checks. This function checks if the process is running in CI and if both stdin and stdout are TTYs, values that are constant during the process lifetime. Added an exported _resetTerminalSupportsPromptingCache function and corresponding unit tests to verify memoization and cache reset. --- .../cli-kit/src/public/node/system.test.ts | 77 ++++++++++++++++++- packages/cli-kit/src/public/node/system.ts | 19 ++++- 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/packages/cli-kit/src/public/node/system.test.ts b/packages/cli-kit/src/public/node/system.test.ts index ed12516ba40..31bd30281f8 100644 --- a/packages/cli-kit/src/public/node/system.test.ts +++ b/packages/cli-kit/src/public/node/system.test.ts @@ -1,6 +1,6 @@ import * as system from './system.js' import {execa} from 'execa' -import {describe, expect, test, vi} from 'vitest' +import {describe, expect, test, vi, beforeEach} from 'vitest' import which from 'which' import {Readable} from 'stream' @@ -353,6 +353,81 @@ describe('isStdinPiped', () => { }) }) +describe('terminalSupportsPrompting', () => { + beforeEach(() => { + system._resetTerminalSupportsPromptingCache() + }) + + test('returns true when not in CI and both stdin/stdout are TTY', () => { + // Given + vi.stubEnv('CI', '') + Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true}) + Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true}) + + // When + const got = system.terminalSupportsPrompting() + + // Then + expect(got).toBe(true) + }) + + test('returns false when in CI', () => { + // Given + vi.stubEnv('CI', 'true') + Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true}) + Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true}) + + // When + const got = system.terminalSupportsPrompting() + + // Then + expect(got).toBe(false) + }) + + test('memoizes the result', () => { + // Given + vi.stubEnv('CI', '') + let calls = 0 + Object.defineProperty(process.stdin, 'isTTY', { + get: () => { + calls++ + return true + }, + configurable: true, + }) + Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true}) + + // When + system.terminalSupportsPrompting() + system.terminalSupportsPrompting() + + // Then + expect(calls).toBe(1) + }) + + test('reset function clears the cache', () => { + // Given + vi.stubEnv('CI', '') + let calls = 0 + Object.defineProperty(process.stdin, 'isTTY', { + get: () => { + calls++ + return true + }, + configurable: true, + }) + Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true}) + + // When + system.terminalSupportsPrompting() + system._resetTerminalSupportsPromptingCache() + system.terminalSupportsPrompting() + + // Then + expect(calls).toBe(2) + }) +}) + describe('readStdinString', () => { test('returns undefined when stdin is not piped', async () => { // Given diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index 3f449f8ff34..e50df19803b 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -333,16 +333,27 @@ export function terminalSupportsHyperlinks(): boolean { return supportsHyperlinks.stdout } +/** + * Memoized value for the terminal prompting support check. + */ +let memoizedTerminalSupportsPrompting: boolean | undefined + /** * Check if the standard input and output streams support prompting. * * @returns True if the standard input and output streams support prompting. */ export function terminalSupportsPrompting(): boolean { - if (isTruthy(process.env.CI)) { - return false - } - return Boolean(process.stdin.isTTY && process.stdout.isTTY) + return (memoizedTerminalSupportsPrompting ??= + !isTruthy(process.env.CI) && Boolean(process.stdin.isTTY && process.stdout.isTTY)) +} + +/** + * Resets the memoized value for the terminal prompting support check. + * This is used for testing purposes. + */ +export function _resetTerminalSupportsPromptingCache(): void { + memoizedTerminalSupportsPrompting = undefined } /** From 72276d73c32201766552d670d7bd44d969f87d95 Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Fri, 3 Jul 2026 01:59:01 +0000 Subject: [PATCH 2/2] [Performance] Memoize terminalSupportsPrompting Memoize the terminalSupportsPrompting function to avoid redundant environment variable lookups and TTY checks. Key changes: - Introduced memoizedTerminalSupportsPrompting variable in `system.ts`. - Updated terminalSupportsPrompting to use the cached value. - Added `_resetTerminalSupportsPromptingCache` for test isolation. - Added unit tests for memoization and reset logic. - Updated `base-command.test.ts` to reset the cache when simulating different TTY/CI environments. --- packages/cli-kit/src/public/node/base-command.test.ts | 3 +++ packages/cli-kit/src/public/node/system.ts | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/cli-kit/src/public/node/base-command.test.ts b/packages/cli-kit/src/public/node/base-command.test.ts index 31203ef5d24..8729894c87e 100644 --- a/packages/cli-kit/src/public/node/base-command.test.ts +++ b/packages/cli-kit/src/public/node/base-command.test.ts @@ -6,6 +6,7 @@ import {inTemporaryDirectory, mkdir, writeFile} from './fs.js' import {joinPath, resolvePath, cwd} from './path.js' import {mockAndCaptureOutput} from './testing/output.js' import {unstyled} from './output.js' +import {_resetTerminalSupportsPromptingCache} from './system.js' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' import {Flags} from '@oclif/core' @@ -20,6 +21,7 @@ beforeEach(() => { Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true}) Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true}) vi.stubEnv('CI', '') + _resetTerminalSupportsPromptingCache() }) afterEach(() => { @@ -441,6 +443,7 @@ describe('applying environments', async () => { runTestInTmpDir('throws in non-TTY mode when a non-TTY required argument is missing', async (tmpDir: string) => { // Given — simulate non-interactive (CI) environment vi.stubEnv('CI', 'true') + _resetTerminalSupportsPromptingCache() // When await MockCommandWithRequiredFlagInNonTTY.run(['--path', tmpDir]) diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index e50df19803b..e162ecd5a9c 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -344,8 +344,13 @@ let memoizedTerminalSupportsPrompting: boolean | undefined * @returns True if the standard input and output streams support prompting. */ export function terminalSupportsPrompting(): boolean { - return (memoizedTerminalSupportsPrompting ??= - !isTruthy(process.env.CI) && Boolean(process.stdin.isTTY && process.stdout.isTTY)) + if (memoizedTerminalSupportsPrompting === undefined) { + const isCi = isTruthy(process.env.CI) + const isStdinTTY = Boolean(process.stdin.isTTY) + const isStdoutTTY = Boolean(process.stdout.isTTY) + memoizedTerminalSupportsPrompting = !isCi && isStdinTTY && isStdoutTTY + } + return memoizedTerminalSupportsPrompting } /**