Conversation
Also replace browser download progress logger with our own
1f08b2c to
f1206ba
Compare
Robdel12
approved these changes
Jun 18, 2021
Contributor
Robdel12
left a comment
There was a problem hiding this comment.
Drops a dependency and makes the UX better? You’re on a roll 🏁
samarsault
pushed a commit
that referenced
this pull request
Mar 3, 2023
Bumps [eslint](https://github.com/eslint/eslint) from 7.29.0 to 7.30.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v7.29.0...v7.30.0) --- updated-dependencies: - dependency-name: eslint dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
There are two logs in particular that would be nice to have some progress associated with them. The processing snapshot queue log and the snapshot uploading queue log.
When SDKs such as Storybook or the snapshot command quickly take and process snapshots via asset discovery, the end result could be waiting for hundreds of snapshots to upload. To prevent clogging up processes with hundreds of in-flight requests at once, uploads are queued just as asset discovery is.
This results in a single log at the end of snapshot processing, before finalizing the build, when snapshots are uploading:
Waiting for X snapshots to finish uploading. With #369, the message was changed to be more generic:Uploading snapshots.... Which is arguably less helpful, but in either case the CLI could be waiting for up to a minute while snapshots are uploaded. Meanwhile there is no feedback from the user's perspective.Approach
During the refactor, while updating the CLI build plugin to work with client changes, I noticed that the code we use to log progress for the wait command was fairly simple and straightforward; and could be adapted to the above use case. The core pieces of progress logging code was moved into a new
logger.progress()method and exposed for loggers. This method will not capture message to the internal log store since it is meant for visual purposes and not for debugging.Where the wait command would always clear the line and not log progress before finished or failed logs were printed, we can take a slightly different approach to achieve the same effect. Rather than always clear the entire line, we can clear the line to the right of the cursor after printing a new message over the existing line. This helps to prevent some flicker between when the line was cleared and when new logs are printed.
For the snapshot queue logs, snapshots can continue to be taken once the queue is awaited one. The build will only be finalized once both the snapshot and upload queues have completely emptied. Because other logs can be printed while waiting on the snapshot queue, a
persistoption can be provided which will re-print the progress log after other logs clear it.I also updated the install script progress logger to use our new method. Likewise with the build command that inspired the method, and even the browser revision script.
Preview
preview.mp4
(sped up a little for brevity)