fix: reduce progress bar flickering (#30349)

Also goes back to showing a single line instead of 3 lines.
This commit is contained in:
David Sherret 2025-08-07 17:13:29 +02:00 committed by GitHub
parent e282367f67
commit a17c53cfe4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 116 additions and 51 deletions

View file

@ -736,6 +736,7 @@ impl CliFactory {
self.npm_installer_if_managed().await?.cloned(), self.npm_installer_if_managed().await?.cloned(),
self.npm_resolver().await?.clone(), self.npm_resolver().await?.clone(),
self.resolver_factory()?.parsed_source_cache().clone(), self.resolver_factory()?.parsed_source_cache().clone(),
self.text_only_progress_bar().clone(),
self.resolver().await?.clone(), self.resolver().await?.clone(),
self.root_permissions_container()?.clone(), self.root_permissions_container()?.clone(),
self.sys(), self.sys(),

View file

@ -67,6 +67,7 @@ use crate::type_checker::CheckOptions;
use crate::type_checker::TypeChecker; use crate::type_checker::TypeChecker;
use crate::util::file_watcher::WatcherCommunicator; use crate::util::file_watcher::WatcherCommunicator;
use crate::util::fs::canonicalize_path; use crate::util::fs::canonicalize_path;
use crate::util::progress_bar::ProgressBar;
#[derive(Clone)] #[derive(Clone)]
pub struct GraphValidOptions<'a> { pub struct GraphValidOptions<'a> {
@ -609,6 +610,7 @@ pub struct ModuleGraphBuilder {
npm_installer: Option<Arc<CliNpmInstaller>>, npm_installer: Option<Arc<CliNpmInstaller>>,
npm_resolver: CliNpmResolver, npm_resolver: CliNpmResolver,
parsed_source_cache: Arc<ParsedSourceCache>, parsed_source_cache: Arc<ParsedSourceCache>,
progress_bar: ProgressBar,
resolver: Arc<CliResolver>, resolver: Arc<CliResolver>,
root_permissions_container: PermissionsContainer, root_permissions_container: PermissionsContainer,
sys: CliSys, sys: CliSys,
@ -631,6 +633,7 @@ impl ModuleGraphBuilder {
npm_installer: Option<Arc<CliNpmInstaller>>, npm_installer: Option<Arc<CliNpmInstaller>>,
npm_resolver: CliNpmResolver, npm_resolver: CliNpmResolver,
parsed_source_cache: Arc<ParsedSourceCache>, parsed_source_cache: Arc<ParsedSourceCache>,
progress_bar: ProgressBar,
resolver: Arc<CliResolver>, resolver: Arc<CliResolver>,
root_permissions_container: PermissionsContainer, root_permissions_container: PermissionsContainer,
sys: CliSys, sys: CliSys,
@ -650,6 +653,7 @@ impl ModuleGraphBuilder {
npm_installer, npm_installer,
npm_resolver, npm_resolver,
parsed_source_cache, parsed_source_cache,
progress_bar,
resolver, resolver,
root_permissions_container, root_permissions_container,
sys, sys,
@ -676,6 +680,7 @@ impl ModuleGraphBuilder {
} }
} }
let _clear_guard = self.progress_bar.deferred_keep_initialize_alive();
let analyzer = self.module_info_cache.as_module_analyzer(); let analyzer = self.module_info_cache.as_module_analyzer();
let mut loader = match options.loader { let mut loader = match options.loader {
Some(loader) => MutLoaderRef::Borrowed(loader), Some(loader) => MutLoaderRef::Borrowed(loader),

View file

@ -183,7 +183,7 @@ impl ModuleLoadPreparer {
allow_unknown_media_types, allow_unknown_media_types,
skip_graph_roots_validation, skip_graph_roots_validation,
} = options; } = options;
let _pb_clear_guard = self.progress_bar.clear_guard(); let _pb_clear_guard = self.progress_bar.deferred_keep_initialize_alive();
let mut loader = self let mut loader = self
.module_graph_builder .module_graph_builder
@ -273,7 +273,7 @@ impl ModuleLoadPreparer {
.collect::<Vec<_>>() .collect::<Vec<_>>()
.join(", ") .join(", ")
); );
let _pb_clear_guard = self.progress_bar.clear_guard(); let _pb_clear_guard = self.progress_bar.deferred_keep_initialize_alive();
let mut loader = self let mut loader = self
.module_graph_builder .module_graph_builder

View file

@ -25,6 +25,9 @@ pub async fn cache_top_level_deps(
factory: &CliFactory, factory: &CliFactory,
jsr_resolver: Option<Arc<crate::jsr::JsrFetchResolver>>, jsr_resolver: Option<Arc<crate::jsr::JsrFetchResolver>>,
) -> Result<(), AnyError> { ) -> Result<(), AnyError> {
let _clear_guard = factory
.text_only_progress_bar()
.deferred_keep_initialize_alive();
let npm_installer = factory.npm_installer().await?; let npm_installer = factory.npm_installer().await?;
npm_installer npm_installer
.ensure_top_level_package_json_install() .ensure_top_level_package_json_install()

View file

@ -45,6 +45,7 @@ use crate::module_loader::ModuleLoadPreparer;
use crate::npm::CliNpmInstaller; use crate::npm::CliNpmInstaller;
use crate::npm::CliNpmResolver; use crate::npm::CliNpmResolver;
use crate::npm::NpmFetchResolver; use crate::npm::NpmFetchResolver;
use crate::util::progress_bar::ProgressBar;
use crate::util::sync::AtomicFlag; use crate::util::sync::AtomicFlag;
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
@ -464,6 +465,7 @@ pub struct DepManager {
npm_resolver: CliNpmResolver, npm_resolver: CliNpmResolver,
npm_installer: Arc<CliNpmInstaller>, npm_installer: Arc<CliNpmInstaller>,
permissions_container: PermissionsContainer, permissions_container: PermissionsContainer,
progress_bar: ProgressBar,
main_module_graph_container: Arc<MainModuleGraphContainer>, main_module_graph_container: Arc<MainModuleGraphContainer>,
lockfile: Option<Arc<CliLockfile>>, lockfile: Option<Arc<CliLockfile>>,
} }
@ -475,6 +477,7 @@ pub struct DepManagerArgs {
pub npm_installer: Arc<CliNpmInstaller>, pub npm_installer: Arc<CliNpmInstaller>,
pub npm_resolver: CliNpmResolver, pub npm_resolver: CliNpmResolver,
pub permissions_container: PermissionsContainer, pub permissions_container: PermissionsContainer,
pub progress_bar: ProgressBar,
pub main_module_graph_container: Arc<MainModuleGraphContainer>, pub main_module_graph_container: Arc<MainModuleGraphContainer>,
pub lockfile: Option<Arc<CliLockfile>>, pub lockfile: Option<Arc<CliLockfile>>,
} }
@ -485,6 +488,7 @@ impl DepManager {
new.latest_versions = self.latest_versions; new.latest_versions = self.latest_versions;
new new
} }
fn with_deps_args(deps: Vec<Dep>, args: DepManagerArgs) -> Self { fn with_deps_args(deps: Vec<Dep>, args: DepManagerArgs) -> Self {
let DepManagerArgs { let DepManagerArgs {
module_load_preparer, module_load_preparer,
@ -492,6 +496,7 @@ impl DepManager {
npm_fetch_resolver, npm_fetch_resolver,
npm_installer, npm_installer,
npm_resolver, npm_resolver,
progress_bar,
permissions_container, permissions_container,
main_module_graph_container, main_module_graph_container,
lockfile, lockfile,
@ -506,6 +511,7 @@ impl DepManager {
npm_fetch_resolver, npm_fetch_resolver,
npm_installer, npm_installer,
npm_resolver, npm_resolver,
progress_bar,
permissions_container, permissions_container,
main_module_graph_container, main_module_graph_container,
lockfile, lockfile,
@ -546,6 +552,7 @@ impl DepManager {
if self.dependencies_resolved.is_raised() { if self.dependencies_resolved.is_raised() {
return Ok(()); return Ok(());
} }
let _clear_guard = self.progress_bar.deferred_keep_initialize_alive();
let mut graph_permit = self let mut graph_permit = self
.main_module_graph_container .main_module_graph_container

View file

@ -514,6 +514,7 @@ async fn dep_manager_args(
npm_fetch_resolver, npm_fetch_resolver,
npm_resolver: factory.npm_resolver().await?.clone(), npm_resolver: factory.npm_resolver().await?.clone(),
npm_installer: factory.npm_installer().await?.clone(), npm_installer: factory.npm_installer().await?.clone(),
progress_bar: factory.text_only_progress_bar().clone(),
permissions_container: factory.root_permissions_container()?.clone(), permissions_container: factory.root_permissions_container()?.clone(),
main_module_graph_container: factory main_module_graph_container: factory
.main_module_graph_container() .main_module_graph_container()

View file

@ -258,6 +258,9 @@ pub async fn maybe_npm_install(factory: &CliFactory) -> Result<(), AnyError> {
if cli_options.specified_node_modules_dir()? == Some(NodeModulesDirMode::Auto) if cli_options.specified_node_modules_dir()? == Some(NodeModulesDirMode::Auto)
{ {
if let Some(npm_installer) = factory.npm_installer_if_managed().await? { if let Some(npm_installer) = factory.npm_installer_if_managed().await? {
let _clear_guard = factory
.text_only_progress_bar()
.deferred_keep_initialize_alive();
let already_done = npm_installer let already_done = npm_installer
.ensure_top_level_package_json_install() .ensure_top_level_package_json_install()
.await?; .await?;

View file

@ -46,6 +46,7 @@ use crate::npm::CliNpmResolver;
use crate::task_runner; use crate::task_runner;
use crate::task_runner::run_future_forwarding_signals; use crate::task_runner::run_future_forwarding_signals;
use crate::util::fs::canonicalize_path; use crate::util::fs::canonicalize_path;
use crate::util::progress_bar::ProgressBar;
#[derive(Debug)] #[derive(Debug)]
struct PackageTaskInfo { struct PackageTaskInfo {
@ -178,6 +179,7 @@ pub async fn execute_script(
let npm_installer = factory.npm_installer_if_managed().await?; let npm_installer = factory.npm_installer_if_managed().await?;
let npm_resolver = factory.npm_resolver().await?; let npm_resolver = factory.npm_resolver().await?;
let node_resolver = factory.node_resolver().await?; let node_resolver = factory.node_resolver().await?;
let progress_bar = factory.text_only_progress_bar();
let mut env_vars = task_runner::real_env_vars(); let mut env_vars = task_runner::real_env_vars();
if let Some(connected) = &flags.connected { if let Some(connected) = &flags.connected {
@ -196,6 +198,7 @@ pub async fn execute_script(
npm_installer: npm_installer.map(|n| n.as_ref()), npm_installer: npm_installer.map(|n| n.as_ref()),
npm_resolver, npm_resolver,
node_resolver: node_resolver.as_ref(), node_resolver: node_resolver.as_ref(),
progress_bar,
env_vars, env_vars,
cli_options, cli_options,
maybe_lockfile, maybe_lockfile,
@ -250,6 +253,7 @@ struct TaskRunner<'a> {
npm_installer: Option<&'a CliNpmInstaller>, npm_installer: Option<&'a CliNpmInstaller>,
npm_resolver: &'a CliNpmResolver, npm_resolver: &'a CliNpmResolver,
node_resolver: &'a CliNodeResolver, node_resolver: &'a CliNodeResolver,
progress_bar: &'a ProgressBar,
env_vars: HashMap<OsString, OsString>, env_vars: HashMap<OsString, OsString>,
cli_options: &'a CliOptions, cli_options: &'a CliOptions,
maybe_lockfile: Option<Arc<CliLockfile>>, maybe_lockfile: Option<Arc<CliLockfile>>,
@ -567,6 +571,7 @@ impl<'a> TaskRunner<'a> {
async fn maybe_npm_install(&self) -> Result<(), AnyError> { async fn maybe_npm_install(&self) -> Result<(), AnyError> {
if let Some(npm_installer) = self.npm_installer { if let Some(npm_installer) = self.npm_installer {
self.progress_bar.deferred_keep_initialize_alive();
npm_installer npm_installer
.ensure_top_level_package_json_install() .ensure_top_level_package_json_install()
.await?; .await?;

View file

@ -74,7 +74,7 @@ pub enum ProgressBarStyle {
/// Shows a progress bar with human readable download size /// Shows a progress bar with human readable download size
DownloadBars, DownloadBars,
/// Shows a progress bar with numeric progres count /// Shows a progress bar with numeric progress count
ProgressBars, ProgressBars,
/// Shows a list of currently downloaded files. /// Shows a list of currently downloaded files.
@ -134,6 +134,7 @@ struct InternalState {
keep_alive_count: usize, keep_alive_count: usize,
total_entries: usize, total_entries: usize,
entries: Vec<Arc<ProgressBarEntry>>, entries: Vec<Arc<ProgressBarEntry>>,
is_deferring_display: bool,
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
@ -151,17 +152,42 @@ impl ProgressBarInner {
keep_alive_count: 0, keep_alive_count: 0,
total_entries: 0, total_entries: 0,
entries: Vec::new(), entries: Vec::new(),
is_deferring_display: false,
})), })),
renderer, renderer,
} }
} }
/// A deferred entry will only be shown once another entry
/// is added to the progress bar.
pub fn add_deferred_entry(
&self,
kind: ProgressMessagePrompt,
message: String,
) -> Arc<ProgressBarEntry> {
let mut internal_state = self.state.lock();
if internal_state.entries.is_empty() {
internal_state.is_deferring_display = true;
}
self.add_entry_internal(&mut internal_state, kind, message)
}
pub fn add_entry( pub fn add_entry(
&self, &self,
kind: ProgressMessagePrompt, kind: ProgressMessagePrompt,
message: String, message: String,
) -> Arc<ProgressBarEntry> { ) -> Arc<ProgressBarEntry> {
let mut internal_state = self.state.lock(); let mut internal_state = self.state.lock();
internal_state.is_deferring_display = false;
self.add_entry_internal(&mut internal_state, kind, message)
}
fn add_entry_internal(
&self,
internal_state: &mut InternalState,
kind: ProgressMessagePrompt,
message: String,
) -> Arc<ProgressBarEntry> {
let id = internal_state.total_entries; let id = internal_state.total_entries;
let entry = Arc::new(ProgressBarEntry { let entry = Arc::new(ProgressBarEntry {
id, id,
@ -175,7 +201,7 @@ impl ProgressBarInner {
internal_state.total_entries += 1; internal_state.total_entries += 1;
internal_state.keep_alive_count += 1; internal_state.keep_alive_count += 1;
self.maybe_start_draw_thread(&mut internal_state); self.maybe_start_draw_thread(internal_state);
entry entry
} }
@ -188,20 +214,13 @@ impl ProgressBarInner {
.binary_search_by(|e| e.id.cmp(&entry_id)) .binary_search_by(|e| e.id.cmp(&entry_id))
{ {
internal_state.entries.remove(index); internal_state.entries.remove(index);
if internal_state.entries.is_empty() {
internal_state.is_deferring_display = false;
}
self.decrement_keep_alive(&mut internal_state); self.decrement_keep_alive(&mut internal_state);
} }
} }
pub fn increment_clear(&self) {
let mut internal_state = self.state.lock();
internal_state.keep_alive_count += 1;
}
pub fn decrement_clear(&self) {
let mut internal_state = self.state.lock();
self.decrement_keep_alive(&mut internal_state);
}
fn decrement_keep_alive(&self, state: &mut InternalState) { fn decrement_keep_alive(&self, state: &mut InternalState) {
state.keep_alive_count -= 1; state.keep_alive_count -= 1;
@ -226,7 +245,7 @@ impl DrawThreadRenderer for ProgressBarInner {
fn render(&self, size: &ConsoleSize) -> String { fn render(&self, size: &ConsoleSize) -> String {
let data = { let data = {
let state = self.state.lock(); let state = self.state.lock();
if state.entries.is_empty() { if state.entries.is_empty() || state.is_deferring_display {
return String::new(); return String::new();
} }
let display_entries = state let display_entries = state
@ -268,7 +287,7 @@ pub struct ProgressBar {
impl deno_npm_installer::Reporter for ProgressBar { impl deno_npm_installer::Reporter for ProgressBar {
type Guard = UpdateGuard; type Guard = UpdateGuard;
type ClearGuard = ClearGuard; type ClearGuard = UpdateGuard;
fn on_blocking(&self, message: &str) -> Self::Guard { fn on_blocking(&self, message: &str) -> Self::Guard {
self.update_with_prompt(ProgressMessagePrompt::Blocking, message) self.update_with_prompt(ProgressMessagePrompt::Blocking, message)
@ -279,7 +298,7 @@ impl deno_npm_installer::Reporter for ProgressBar {
} }
fn clear_guard(&self) -> Self::ClearGuard { fn clear_guard(&self) -> Self::ClearGuard {
self.clear_guard() self.deferred_keep_initialize_alive()
} }
} }
@ -334,22 +353,27 @@ impl ProgressBar {
} }
} }
pub fn clear_guard(&self) -> ClearGuard { pub fn deferred_keep_initialize_alive(&self) -> UpdateGuard {
self.inner.increment_clear(); self.deferred_update_with_prompt(ProgressMessagePrompt::Initialize, "")
ClearGuard { pb: self.clone() }
} }
fn decrement_clear(&self) { /// Add an entry to the progress bar that will only be shown
self.inner.decrement_clear(); /// once another entry has been added.
} pub fn deferred_update_with_prompt(
} &self,
kind: ProgressMessagePrompt,
pub struct ClearGuard { msg: &str,
pb: ProgressBar, ) -> UpdateGuard {
} // only check if progress bars are supported once we go
// to update so that we lazily initialize the progress bar
impl Drop for ClearGuard { if ProgressBar::are_supported() {
fn drop(&mut self) { let entry = self.inner.add_deferred_entry(kind, msg.to_string());
self.pb.decrement_clear(); UpdateGuard {
maybe_entry: Some(entry),
}
} else {
// do not display anything for a deferred update
UpdateGuard { maybe_entry: None }
}
} }
} }

View file

@ -143,22 +143,41 @@ impl Default for TextOnlyProgressBarRenderer {
} }
} }
const SPINNER_CHARS: [&str; 8] = ["", "", "", "", "", "", "", ""]; const SPINNER_CHARS: [&str; 13] = [
"▰▱▱▱▱▱",
"▰▰▱▱▱▱",
"▰▰▰▱▱▱",
"▰▰▰▰▱▱",
"▰▰▰▰▰▱",
"▰▰▰▰▰▰",
"▰▰▰▰▰▰",
"▱▰▰▰▰▰",
"▱▱▰▰▰▰",
"▱▱▱▰▰▰",
"▱▱▱▱▰▰",
"▱▱▱▱▱▰",
"▱▱▱▱▱▱",
];
impl ProgressBarRenderer for TextOnlyProgressBarRenderer { impl ProgressBarRenderer for TextOnlyProgressBarRenderer {
fn render(&self, data: ProgressData) -> String { fn render(&self, data: ProgressData) -> String {
let last_tick = { let last_tick = {
let last_tick = self.last_tick.load(Ordering::Relaxed); let last_tick = self.last_tick.load(Ordering::Relaxed);
let last_tick = (last_tick + 1) % 8; let last_tick = (last_tick + 1) % SPINNER_CHARS.len();
self.last_tick.store(last_tick, Ordering::Relaxed); self.last_tick.store(last_tick, Ordering::Relaxed);
last_tick last_tick
}; };
let current_time = std::time::Instant::now(); let current_time = std::time::Instant::now();
let mut display_str = format!( let non_empty_entry = data
"{} {} ", .display_entries
data.display_entries[0].prompt.as_text(), .iter()
SPINNER_CHARS[last_tick] .find(|d| !d.message.is_empty() || d.total_size != 0);
); let prompt = match non_empty_entry {
Some(entry) => entry.prompt,
None => data.display_entries[0].prompt,
};
let mut display_str =
format!("{} {} ", prompt.as_text(), SPINNER_CHARS[last_tick]);
let elapsed_time = current_time - self.start_time; let elapsed_time = current_time - self.start_time;
let fmt_elapsed_time = get_elapsed_text(elapsed_time); let fmt_elapsed_time = get_elapsed_text(elapsed_time);
@ -174,13 +193,7 @@ impl ProgressBarRenderer for TextOnlyProgressBarRenderer {
}; };
display_str.push_str(&format!("{}{}\n", fmt_elapsed_time, total_text)); display_str.push_str(&format!("{}{}\n", fmt_elapsed_time, total_text));
if let Some(display_entry) = non_empty_entry {
for i in 0..4 {
let Some(display_entry) = data.display_entries.get(i) else {
display_str.push('\n');
continue;
};
let bytes_text = { let bytes_text = {
let total_size = display_entry.total_size; let total_size = display_entry.total_size;
let pos = display_entry.position; let pos = display_entry.position;
@ -206,8 +219,11 @@ impl ProgressBarRenderer for TextOnlyProgressBarRenderer {
.replace("%2F", "/"); .replace("%2F", "/");
display_str.push_str( display_str.push_str(
&colors::gray(format!(" - {}{}\n", message, bytes_text)).to_string(), &colors::gray(format!(" {}{}\n", message, bytes_text)).to_string(),
); );
} else {
// prevent cursor from going up
display_str.push('\n');
} }
display_str display_str
@ -334,8 +350,8 @@ mod test {
}; };
let text = renderer.render(data.clone()); let text = renderer.render(data.clone());
let text = test_util::strip_ansi_codes(&text); let text = test_util::strip_ansi_codes(&text);
assert_contains!(text, "Blocking "); assert_contains!(text, "Blocking ▰▰▱▱▱▱");
assert_contains!(text, "2/3\n - data 0.00KiB/10.00KiB\n\n\n\n"); assert_contains!(text, "2/3\n data 0.00KiB/10.00KiB\n");
data.pending_entries = 0; data.pending_entries = 0;
data.total_entries = 1; data.total_entries = 1;
@ -343,7 +359,7 @@ mod test {
data.display_entries[0].total_size = 0; data.display_entries[0].total_size = 0;
let text = renderer.render(data); let text = renderer.render(data);
let text = test_util::strip_ansi_codes(&text); let text = test_util::strip_ansi_codes(&text);
assert_contains!(text, "Blocking "); assert_contains!(text, "Blocking ▰▰▰▱▱▱");
assert_contains!(text, "\n - data\n\n\n\n"); assert_contains!(text, "\n data\n");
} }
} }