KEMBAR78
fix: progress bar code cleanup by wraithgar · Pull Request #8633 · npm/cli · GitHub
Skip to content

Conversation

@wraithgar
Copy link
Member

@wraithgar wraithgar commented Oct 2, 2025

  • inline this.#render logic. The initial setTimeout is only used during load(), so run it there instead.
  • .unref() the progress bar's recurring timeout object
  • reuse the progress bar's timeout object with .refresh() instead of making a new one every frame
  • Minor comment syntax cleanup and remove stale comments
  • skip clearing the line on the first frame when resuming the spinner
  • removed the test from fix: preserve npm run output without trailing newline #8631 as it was only testing that the very first frame of the spinner didn't clear the line, not any subsequent frames

This is built off of #8631, moving the "skip clearing the line..." logic into a flag that's passed during .resume()

@wraithgar wraithgar requested a review from a team as a code owner October 2, 2025 16:19
cp90-pixel and others added 2 commits October 2, 2025 09:20
 - inline this.#render logic.  The initial setTimeout is only used during load(), so run it there instead.
 - .unref() the progress bar's recurring timeout object
 - reuse the progress bar's timeout object with .refresh() instead of making a new one every frame
 - consolidate comments into one line and remove stale comments
 - skip clearing the line on the first frame when resuming the spinner
 - removed the test from $8631 as it was only testing that the very first frame of the spinner didn't clear the line, not any subsequent frames

This is built off of #8631, moving the "skip clearing the line..." logic into a flag that's passed during .resume()
@wraithgar wraithgar merged commit c54d1e9 into latest Oct 6, 2025
38 of 39 checks passed
@wraithgar wraithgar deleted the gar/pr8631 branch October 6, 2025 16:58
@github-actions github-actions bot mentioned this pull request Oct 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants