Fix loading bar cleanup when tasks log messages

.changeset/fix-loading-bar-logging.md

@@ -0,0 +1,5 @@
+---
+'@shopify/cli': patch
+---
+
+Fix loading bars remaining on screen when a task prints log messages.

packages/app/src/cli/commands/app/doctor/submit.integration.test.ts

@@ -15,6 +15,8 @@ import {readStdinString, terminalSupportsPrompting} from '@shopify/cli-kit/node/
 import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
 import {describe, expect, test, vi} from 'vitest'
 import {mkdir, readFile, readdir, writeFile} from 'node:fs/promises'
+// eslint-disable-next-line n/prefer-global/console
+import {Console} from 'node:console'
 import type {
   DeveloperPlatformClient,
   SourceScanCreateSchema,
@@ -97,6 +99,8 @@ async function runCommand(argv: string[]) {
     stderr += chunk.toString()
     return true
   })
+  // Vitest intercepts console.warn; use Node's console to exercise the captured streams.
+  const warn = vi.spyOn(console, 'warn').mockImplementation(new Console(process.stdout, process.stderr).warn)
   // Observe the real Oclif error handler's requested exit without terminating the test worker.
   const exit = vi.spyOn(process, 'exit').mockImplementation((code) => {
     process.exitCode = code ?? 0
@@ -109,6 +113,7 @@ async function runCommand(argv: string[]) {
     await DoctorSubmit.run(argv, config)
     return {stdout, stderr, exitCode: process.exitCode, exits: exit.mock.calls.map(([code]) => code)}
   } finally {
+    warn.mockRestore()
     out.mockRestore()
     err.mockRestore()
     exit.mockRestore()

packages/cli-kit/src/private/node/output.test.tsx

@@ -0,0 +1,84 @@
+import {InkLifecycleRoot} from './ui.js'
+import {SingleTask} from './ui/components/SingleTask.js'
+import {outputInfo, outputWarn, outputResult, TokenizedString, unstyled} from '../../public/node/output.js'
+import React from 'react'
+import {render} from 'ink'
+import ansiEscapes from 'ansi-escapes'
+import {describe, expect, test, vi} from 'vitest'
+import {PassThrough} from 'stream'
+// Vitest's console omits the constructor used by Ink's console patch.
+// eslint-disable-next-line n/prefer-global/console
+import {Console} from 'console'
+
+// Ink detects CI when imported; exercise terminal rendering on CI runners too.
+vi.hoisted(() => {
+  vi.stubEnv('CI', 'false')
+  vi.stubEnv('CONTINUOUS_INTEGRATION', 'false')
+})
+vi.mock('../../public/node/context/local.js', async (importOriginal) => ({
+  ...(await importOriginal<typeof import('../../public/node/context/local.js')>()),
+  isUnitTest: () => false,
+}))
+
+describe('logging during an Ink task', () => {
+  test.each([outputInfo, outputWarn])('clears and redraws the task around %o', async (log) => {
+    vi.stubGlobal('console', {...console, Console})
+    const writes: string[] = []
+    const terminal = Object.assign(new PassThrough(), {isTTY: true, columns: 80, rows: 24})
+    terminal.on('data', (data: Buffer) => writes.push(data.toString()))
+    let finishTask!: () => void
+    const taskResult = new Promise<void>((resolve) => {
+      finishTask = resolve
+    })
+    const stdoutWrite = vi.spyOn(process.stdout, 'write').mockReturnValue(true)
+    const stderrWrite = vi.spyOn(process.stderr, 'write').mockReturnValue(true)
+    const instance = render(
+      <InkLifecycleRoot>
+        <SingleTask title={new TokenizedString('Preparing the result')} task={() => taskResult} />
+      </InkLifecycleRoot>,
+      {
+        stdout: terminal as unknown as NodeJS.WriteStream,
+        stderr: terminal as unknown as NodeJS.WriteStream,
+        patchConsole: true,
+        exitOnCtrlC: false,
+      },
+    )
+
+    try {
+      await vi.waitFor(() => expect(writes.join('')).toContain('Preparing the result'))
+      expect(writes.join('')).toContain('▀')
+      const beforeLog = writes.length
+
+      log('Prepared an item')
+
+      // Clear both UI lines and their trailing newline before writing the log.
+      const logWrites = writes.slice(beforeLog)
+      const messageIndex = logWrites.findIndex((write) => write.includes('Prepared an item'))
+      expect(messageIndex).toBeGreaterThan(0)
+      expect(logWrites[messageIndex - 1]).toBe(ansiEscapes.eraseLines(3))
+      expect(unstyled(logWrites[messageIndex]!)).toBe('Prepared an item\n')
+      expect(logWrites[messageIndex + 1]).toContain('Preparing the result')
+
+      outputResult('{"items":1}')
+      expect(stdoutWrite).toHaveBeenCalledWith('{"items":1}\n')
+      expect(stderrWrite).not.toHaveBeenCalled()
+
+      const afterLog = writes.length
+      finishTask()
+      await instance.waitUntilExit()
+
+      const cleanup = writes.slice(afterLog).join('')
+      expect(cleanup).toContain(ansiEscapes.eraseLines(3))
+      expect(unstyled(cleanup).trim()).toBe('')
+    } finally {
+      finishTask()
+      instance.unmount()
+      instance.cleanup()
+      stdoutWrite.mockRestore()
+      stderrWrite.mockRestore()
+      vi.unstubAllGlobals()
+      vi.unstubAllEnvs()
+      terminal.destroy()
+    }
+  })
+})

packages/cli-kit/src/private/node/output.ts

@@ -39,7 +39,9 @@ export function consoleLog(message: string): void {
  * @param message - The message to print.
  */
 export function consoleWarn(message: string): void {
-  process.stderr.write(`${withOrWithoutStyle(message)}\n`)
+  // Ink intercepts console calls to preserve its active UI around log messages.
+  // eslint-disable-next-line no-console
+  console.warn(withOrWithoutStyle(message))
 }
 
 /**

packages/store/src/cli/services/store/auth/result.test.ts

@@ -1,6 +1,8 @@
 import {createStoreAuthPresenter} from './result.js'
 import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
 import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
+// eslint-disable-next-line n/prefer-global/console
+import {Console} from 'node:console'
 
 function captureStandardStreams() {
   const stdout: string[] = []
@@ -14,11 +16,14 @@ function captureStandardStreams() {
     stderr.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8'))
     return true
   }) as typeof process.stderr.write)
+  // Vitest intercepts console.warn; use Node's console to exercise the captured streams.
+  const warnSpy = vi.spyOn(console, 'warn').mockImplementation(new Console(process.stdout, process.stderr).warn)
 
   return {
     stdout: () => stdout.join(''),
     stderr: () => stderr.join(''),
     restore: () => {
+      warnSpy.mockRestore()
       stdoutSpy.mockRestore()
       stderrSpy.mockRestore()
     },