diff --git a/.changeset/render-tasks-to-stderr.md b/.changeset/render-tasks-to-stderr.md new file mode 100644 index 0000000000..a4b2853089 --- /dev/null +++ b/.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 diff --git a/packages/cli-kit/src/private/node/ui/components/LoadingBar.test.tsx b/packages/cli-kit/src/private/node/ui/components/LoadingBar.test.tsx index e4ca307890..b318b02c02 100644 --- a/packages/cli-kit/src/private/node/ui/components/LoadingBar.test.tsx +++ b/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,15 +25,30 @@ 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() + const {lastFrame} = renderWithTTY() - // Then expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀ Loading content ..." @@ -41,13 +56,8 @@ describe('LoadingBar', () => { }) test('renders loading bar with hill pattern when noColor prop is true', async () => { - // Given - const title = 'Processing files' - - // When - const {lastFrame} = render() + const {lastFrame} = renderWithTTY() - // Then expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅ Processing files ..." @@ -55,14 +65,9 @@ describe('LoadingBar', () => { }) 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() - // When - const {lastFrame} = render() - - // Then expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅ Downloading packages ..." @@ -70,18 +75,9 @@ describe('LoadingBar', () => { }) 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() - - // Then + vi.mocked(useLayout).mockReturnValue({twoThirds: 20, oneThird: 10, fullWidth: 30}) + const {lastFrame} = renderWithTTY() + expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀ Building app ..." @@ -89,18 +85,9 @@ describe('LoadingBar', () => { }) 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() - - // Then + vi.mocked(useLayout).mockReturnValue({twoThirds: 15, oneThird: 8, fullWidth: 23}) + const {lastFrame} = renderWithTTY() + expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇ Installing ..." @@ -108,18 +95,9 @@ describe('LoadingBar', () => { }) 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() - - // Then + vi.mocked(useLayout).mockReturnValue({twoThirds: 5, oneThird: 3, fullWidth: 8}) + const {lastFrame} = renderWithTTY() + expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▁▁▁▂▂ Wait ..." @@ -127,18 +105,9 @@ describe('LoadingBar', () => { }) 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() - - // Then + vi.mocked(useLayout).mockReturnValue({twoThirds: 100, oneThird: 50, fullWidth: 150}) + const {lastFrame} = renderWithTTY() + expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀ Synchronizing data ..." @@ -146,18 +115,9 @@ describe('LoadingBar', () => { }) 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() - - // Then + vi.mocked(useLayout).mockReturnValue({twoThirds: 90, oneThird: 45, fullWidth: 135}) + const {lastFrame} = renderWithTTY() + expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁ Analyzing dependencies ..." @@ -165,13 +125,8 @@ describe('LoadingBar', () => { }) test('renders correctly with empty title', async () => { - // Given - const title = '' - - // When - const {lastFrame} = render() + const {lastFrame} = renderWithTTY() - // Then expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀ ..." @@ -179,14 +134,9 @@ describe('LoadingBar', () => { }) test('noColor prop overrides shouldDisplayColors when both would show colors', async () => { - // Given vi.mocked(shouldDisplayColors).mockReturnValue(true) - const title = 'Testing override' + const {lastFrame} = renderWithTTY() - // When - const {lastFrame} = render() - - // Then expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(` "▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅▄▄▃▃▂▂▁▁▁▁▂▂▃▃▄▄▅▅▆▆▇▇██▇▇▆▆▅▅ Testing override ..." @@ -194,27 +144,32 @@ describe('LoadingBar', () => { }) test('renders consistently with same props', async () => { - // Given - const title = 'Consistent test' - const props = {title, noColor: false} - - // When - const {lastFrame: frame1} = render() - const {lastFrame: frame2} = render() + const props = {title: 'Consistent test', noColor: false} + const {lastFrame: frame1} = renderWithTTY() + const {lastFrame: frame2} = renderWithTTY() - // 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() + const {lastFrame} = renderWithTTY() - // 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() + + expect(unstyled(lastFrame()!)).toMatchInlineSnapshot(`"Installing dependencies ..."`) + }) + + test('shows animated progress bar when output stream is a TTY', async () => { + const {lastFrame} = renderWithTTY() + + const frame = unstyled(lastFrame()!) + expect(frame).toContain('▀') + expect(frame).toContain('Uploading theme ...') + }) }) diff --git a/packages/cli-kit/src/private/node/ui/components/LoadingBar.tsx b/packages/cli-kit/src/private/node/ui/components/LoadingBar.tsx index 42278012be..0091cb6da4 100644 --- a/packages/cli-kit/src/private/node/ui/components/LoadingBar.tsx +++ b/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,6 +16,15 @@ interface LoadingBarProps { const LoadingBar = ({title, noColor, noProgressBar}: React.PropsWithChildren) => { 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).isTTY) + let loadingBar = new Array(twoThirds).fill(loadingBarChar).join('') if (noColor ?? !shouldDisplayColors()) { loadingBar = hillString.repeat(Math.ceil(twoThirds / hillString.length)) @@ -23,7 +32,7 @@ const LoadingBar = ({title, noColor, noProgressBar}: React.PropsWithChildren - {!noProgressBar && } + {isTTY && !noProgressBar && } {title} ... ) diff --git a/packages/cli-kit/src/public/node/ui.tsx b/packages/cli-kit/src/public/node/ui.tsx index a6fdb7797c..35c6bab6a7 100644 --- a/packages/cli-kit/src/public/node/ui.tsx +++ b/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( noProgressBar={noProgressBar} />, { + stdout: process.stderr as unknown as NodeJS.WriteStream, ...renderOptions, exitOnCtrlC: false, }, @@ -519,7 +519,6 @@ export interface RenderSingleTaskOptions { * @param options.renderOptions - Optional render configuration * @returns The result of the task * @example - * ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀ * Loading app ... */ export async function renderSingleTask({ @@ -539,6 +538,7 @@ export async function renderSingleTask({ onAbort={onAbort} />, { + stdout: process.stderr as unknown as NodeJS.WriteStream, ...renderOptions, exitOnCtrlC: false, },