Merge pull request #7295 from Shopify/ac/hide-loading-bar-non-tty

Render task progress bars to stderr by default

Author: amcaplan

.changeset/render-tasks-to-stderr.md

@@ -0,0 +1,5 @@
+---
+'@shopify/cli-kit': patch
+---
+
+Render task progress bars to stderr to reduce output noise in non-TTY environments

packages/cli-kit/src/private/node/ui/components/LoadingBar.test.tsx

@@ -1,4 +1,5 @@
 import {LoadingBar} from './LoadingBar.js'
+import {Stdout} from '../../ui.js'
 import {render} from '../../testing/ui.js'
 import {shouldDisplayColors, unstyled} from '../../../../public/node/output.js'
 import useLayout from '../hooks/use-layout.js'
@@ -16,7 +17,6 @@ vi.mock('../../../../public/node/output.js', async () => {
 })
 
 beforeEach(() => {
-  // Default terminal width
   vi.mocked(useLayout).mockReturnValue({
     twoThirds: 53,
     oneThird: 27,
@@ -25,196 +25,151 @@ beforeEach(() => {
   vi.mocked(shouldDisplayColors).mockReturnValue(true)
 })
 
+/**
+ * Creates a Stdout test double simulating a TTY stream.
+ * On real Node streams, isTTY is only present as an own property when the
+ * stream IS a TTY.
+ */
+function createTTYStdout(columns = 100) {
+  const stdout = new Stdout({columns}) as Stdout & {isTTY: boolean}
+  stdout.isTTY = true
+  return stdout
+}
+
+/**
+ * Renders LoadingBar with a TTY stdout so the animated progress bar renders.
+ */
+function renderWithTTY(element: React.ReactElement) {
+  const stdout = createTTYStdout()
+  const instance = render(element, {stdout})
+  return {lastFrame: stdout.lastFrame, unmount: instance.unmount}
+}
+
 describe('LoadingBar', () => {
   test('renders loading bar with default colored characters', async () => {
-    // Given
-    const title = 'Loading content'
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} />)
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Loading content" />)
 
-    // Then
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
       Loading content ..."
     `)
   })
 
   test('renders loading bar with hill pattern when noColor prop is true', async () => {
-    // Given
-    const title = 'Processing files'
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} noColor />)
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Processing files" noColor />)
 
-    // Then
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅
       Processing files ..."
     `)
   })
 
   test('renders loading bar with hill pattern when shouldDisplayColors returns false', async () => {
-    // Given
     vi.mocked(shouldDisplayColors).mockReturnValue(false)
-    const title = 'Downloading packages'
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Downloading packages" />)
 
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} />)
-
-    // Then
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅
       Downloading packages ..."
     `)
   })
 
   test('handles narrow terminal width correctly', async () => {
-    // Given
-    vi.mocked(useLayout).mockReturnValue({
-      twoThirds: 20,
-      oneThird: 10,
-      fullWidth: 30,
-    })
-    const title = 'Building app'
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} />)
-
-    // Then
+    vi.mocked(useLayout).mockReturnValue({twoThirds: 20, oneThird: 10, fullWidth: 30})
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Building app" />)
+
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
       Building app ..."
     `)
   })
 
   test('handles narrow terminal width correctly in no-color mode', async () => {
-    // Given
-    vi.mocked(useLayout).mockReturnValue({
-      twoThirds: 15,
-      oneThird: 8,
-      fullWidth: 23,
-    })
-    const title = 'Installing'
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} noColor />)
-
-    // Then
+    vi.mocked(useLayout).mockReturnValue({twoThirds: 15, oneThird: 8, fullWidth: 23})
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Installing" noColor />)
+
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇
       Installing ..."
     `)
   })
 
   test('handles very narrow terminal width in no-color mode', async () => {
-    // Given
-    vi.mocked(useLayout).mockReturnValue({
-      twoThirds: 5,
-      oneThird: 3,
-      fullWidth: 8,
-    })
-    const title = 'Wait'
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} noColor />)
-
-    // Then
+    vi.mocked(useLayout).mockReturnValue({twoThirds: 5, oneThird: 3, fullWidth: 8})
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Wait" noColor />)
+
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▁▁▁▂▂
       Wait ..."
     `)
   })
 
   test('handles wide terminal width correctly', async () => {
-    // Given
-    vi.mocked(useLayout).mockReturnValue({
-      twoThirds: 100,
-      oneThird: 50,
-      fullWidth: 150,
-    })
-    const title = 'Synchronizing data'
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} />)
-
-    // Then
+    vi.mocked(useLayout).mockReturnValue({twoThirds: 100, oneThird: 50, fullWidth: 150})
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Synchronizing data" />)
+
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
       Synchronizing data ..."
     `)
   })
 
   test('handles wide terminal width correctly in no-color mode with pattern repetition', async () => {
-    // Given
-    vi.mocked(useLayout).mockReturnValue({
-      twoThirds: 90,
-      oneThird: 45,
-      fullWidth: 135,
-    })
-    const title = 'Analyzing dependencies'
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} noColor />)
-
-    // Then
+    vi.mocked(useLayout).mockReturnValue({twoThirds: 90, oneThird: 45, fullWidth: 135})
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Analyzing dependencies" noColor />)
+
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁
       Analyzing dependencies ..."
     `)
   })
 
   test('renders correctly with empty title', async () => {
-    // Given
-    const title = ''
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} />)
+    const {lastFrame} = renderWithTTY(<LoadingBar title="" />)
 
-    // Then
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
        ..."
     `)
   })
 
   test('noColor prop overrides shouldDisplayColors when both would show colors', async () => {
-    // Given
     vi.mocked(shouldDisplayColors).mockReturnValue(true)
-    const title = 'Testing override'
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Testing override" noColor />)
 
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} noColor />)
-
-    // Then
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`
       "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅
       Testing override ..."
     `)
   })
 
   test('renders consistently with same props', async () => {
-    // Given
-    const title = 'Consistent test'
-    const props = {title, noColor: false}
-
-    // When
-    const {lastFrame: frame1} = render(<LoadingBar {...props} />)
-    const {lastFrame: frame2} = render(<LoadingBar {...props} />)
+    const props = {title: 'Consistent test', noColor: false}
+    const {lastFrame: frame1} = renderWithTTY(<LoadingBar {...props} />)
+    const {lastFrame: frame2} = renderWithTTY(<LoadingBar {...props} />)
 
-    // Then
     expect(frame1()).toBe(frame2())
   })
 
   test('hides progress bar when noProgressBar is true', async () => {
-    // Given
     vi.mocked(shouldDisplayColors).mockReturnValue(true)
-    const title = 'task 1'
-
-    // When
-    const {lastFrame} = render(<LoadingBar title={title} noProgressBar />)
+    const {lastFrame} = renderWithTTY(<LoadingBar title="task 1" noProgressBar />)
 
-    // Then
     expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`"task 1 ..."`)
   })
+
+  test('shows only static title text when output stream is not a TTY', async () => {
+    // Default test Stdout has no isTTY property, simulating a non-TTY stream
+    const {lastFrame} = render(<LoadingBar title="Installing dependencies" />)
+
+    expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`"Installing dependencies ..."`)
+  })
+
+  test('shows animated progress bar when output stream is a TTY', async () => {
+    const {lastFrame} = renderWithTTY(<LoadingBar title="Uploading theme" />)
+
+    const frame = unstyled(lastFrame()!)
+    expect(frame).toContain('▀')
+    expect(frame).toContain('Uploading theme ...')
+  })
 })

packages/cli-kit/src/private/node/ui/components/LoadingBar.tsx

@@ -3,7 +3,7 @@ import useLayout from '../hooks/use-layout.js'
 import {shouldDisplayColors} from '../../../../public/node/output.js'
 import React from 'react'
 
-import {Box, Text} from 'ink'
+import {Box, Text, useStdout} from 'ink'
 
 const loadingBarChar = '▀'
 const hillString = '▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁'
@@ -16,14 +16,23 @@ interface LoadingBarProps {
 
 const LoadingBar = ({title, noColor, noProgressBar}: React.PropsWithChildren<LoadingBarProps>) => {
   const {twoThirds} = useLayout()
+  const {stdout} = useStdout()
+
+  // On real Node streams, isTTY is only present as an own property when the
+  // stream IS a TTY.  When Ink's output stream is not a TTY (e.g. AI agents
+  // capturing stderr via 2>&1), the animated progress bar can't overwrite
+  // previous frames and would flood the output.  Show only the static title
+  // in that case.
+  const isTTY = Boolean((stdout as unknown as Record<string, unknown>).isTTY)
+
   let loadingBar = new Array(twoThirds).fill(loadingBarChar).join('')
   if (noColor ?? !shouldDisplayColors()) {
     loadingBar = hillString.repeat(Math.ceil(twoThirds / hillString.length))
   }
 
   return (
     <Box flexDirection="column">
-      {!noProgressBar && <TextAnimation text={loadingBar} maxWidth={twoThirds} />}
+      {isTTY && !noProgressBar && <TextAnimation text={loadingBar} maxWidth={twoThirds} />}
       <Text>{title} ...</Text>
     </Box>
   )

packages/cli-kit/src/public/node/ui.tsx

@@ -479,7 +479,6 @@ interface RenderTasksOptions {
 /**
  * Runs async tasks and displays their progress to the console.
  * @example
- * ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
  * Installing dependencies ...
  */
 
@@ -497,6 +496,7 @@ export async function renderTasks<TContext>(
       noProgressBar={noProgressBar}
     />,
     {
+      stdout: process.stderr as unknown as NodeJS.WriteStream,
       ...renderOptions,
       exitOnCtrlC: false,
     },
@@ -519,7 +519,6 @@ export interface RenderSingleTaskOptions<T> {
  * @param options.renderOptions - Optional render configuration
  * @returns The result of the task
  * @example
- * ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
  * Loading app ...
  */
 export async function renderSingleTask<T>({
@@ -539,6 +538,7 @@ export async function renderSingleTask<T>({
       onAbort={onAbort}
     />,
     {
+      stdout: process.stderr as unknown as NodeJS.WriteStream,
       ...renderOptions,
       exitOnCtrlC: false,
     },