Fix printing phase durations for non-tty terminals #85002
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||
| import ora from 'next/dist/compiled/ora' | ||||||
| import * as Log from './output/log' | ||||||
| import { hrtimeBigIntDurationToString } from './duration-to-string' | ||||||
|
|
||||||
| const dotsSpinner = { | ||||||
| frames: ['.', '..', '...'], | ||||||
|
|
@@ -11,12 +12,11 @@ export default function createSpinner( | |||||
| options: ora.Options = {}, | ||||||
| logFn: (...data: any[]) => void = console.log | ||||||
| ) { | ||||||
| let prefixText = ` ${Log.prefixes.info} ${text} ` | ||||||
| const progressStart = process.hrtime.bigint() | ||||||
|
|
||||||
| if (process.stdout.isTTY) { | ||||||
| let spinner = ora({ | ||||||
| text: undefined, | ||||||
| prefixText, | ||||||
| spinner: dotsSpinner, | ||||||
|
|
@@ -35,16 +35,16 @@ export default function createSpinner( | |||||
| const logHandle = (method: any, args: any[]) => { | ||||||
| // Enter a new line before logging new message, to avoid | ||||||
| // the new message shows up right after the spinner in the same line. | ||||||
| const isInProgress = spinner.isSpinning | ||||||
| if (isInProgress) { | ||||||
| // Reset the current running spinner to empty line by `\r` | ||||||
| spinner.prefixText = '\r' | ||||||
| spinner.text = '\r' | ||||||
| spinner.clear() | ||||||
| origStop() | ||||||
| } | ||||||
| method(...args) | ||||||
| if (isInProgress) { | ||||||
| spinner.start() | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -61,29 +61,46 @@ export default function createSpinner( | |||||
| spinner.setText = (newText) => { | ||||||
| text = newText | ||||||
| prefixText = ` ${Log.prefixes.info} ${newText} ` | ||||||
| spinner.prefixText = prefixText | ||||||
| return spinner | ||||||
| } | ||||||
| spinner.stop = () => { | ||||||
| origStop() | ||||||
| resetLog() | ||||||
| return spinner | ||||||
| } | ||||||
| spinner.stopAndPersist = () => { | ||||||
| const duration = process.hrtime.bigint() - progressStart | ||||||
| text = `${text} in ${hrtimeBigIntDurationToString(duration)}` | ||||||
| prefixText = ` ${Log.prefixes.info} ${text} ` | ||||||
|
There was a problem hiding this comment.
Suggested change
The TTY spinner's View DetailsAnalysisIncorrect prefix in spinner stopAndPersist() shows space instead of checkmarkWhat fails: The TTY spinner's How to reproduce: The spinner is used during the build process when collecting build traces. When the build spinner completes, the message should display with a green checkmark prefix. Currently it displays with a space prefix instead. This is verified by the test expectations in Result: The final spinner message displays Expected: When Fix applied: Changed line 75 in |
||||||
| spinner.prefixText = prefixText | ||||||
| origStopAndPersist() | ||||||
| resetLog() | ||||||
mischnic marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| return spinner | ||||||
| } | ||||||
| return spinner | ||||||
| } else { | ||||||
| text = ` ${Log.prefixes.info} ${text} ` | ||||||
| logFn(text) | ||||||
|
There was a problem hiding this comment.
Suggested change
The non-TTY spinner output is missing the "..." suffix that indicates the operation is in progress, causing the output format to not match the test expectations and PR description. View DetailsAnalysisMissing "..." suffix in non-TTY spinner outputWhat fails: The non-TTY spinner mode in How to reproduce: import createSpinner from 'packages/next/src/build/spinner'
const outputs: string[] = [];
const spinner = createSpinner('Collecting build traces', {}, (text) => {
outputs.push(text);
});
// In non-TTY environment:
// Expected: " ℹ Collecting build traces ..."
// Actual: " ℹ Collecting build traces "Root cause: Commit 74df2f6 (fixup) removed the Why this is a bug: In TTY mode, the spinner uses animated frames ['.', '..', '...'] to indicate progress. In non-TTY mode, there is no animation, so the static output should include the dots to maintain visual consistency and clearly indicate the operation is in progress. The original implementation (commit 79f9651) intentionally included the dots for this reason. Expected behavior: Non-TTY output should display "Collecting build traces ..." with the dots appended to indicate the operation is in progress, matching the original intent and TTY mode behavior. Fix applied: Append |
||||||
|
|
||||||
| // @ts-ignore | ||||||
| let spinner = { | ||||||
| isSpinning: false, | ||||||
| prefixText: '', | ||||||
| text: '', | ||||||
| clear: '', | ||||||
| setText(newText: string) { | ||||||
| text = ` ${Log.prefixes.info} ${newText}` | ||||||
| logFn(text) | ||||||
| }, | ||||||
| stop: () => spinner, | ||||||
| stopAndPersist: () => { | ||||||
| const duration = process.hrtime.bigint() - progressStart | ||||||
| logFn(`${text} in ${hrtimeBigIntDurationToString(duration)}`) | ||||||
| return spinner! | ||||||
| }, | ||||||
| } as ora.Ora & { setText: (text: string) => void } | ||||||
|
|
||||||
| return spinner | ||||||
| } | ||||||
| } | ||||||
Uh oh!
There was an error while loading. Please reload this page.