fix: reduce progress bar flickering (#30349)

Also goes back to showing a single line instead of 3 lines.

Author: dsherret

cli/factory.rs

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

cli/graph_util.rs

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

cli/module_loader.rs

@@ -183,7 +183,7 @@ impl ModuleLoadPreparer {
       allow_unknown_media_types,
       skip_graph_roots_validation,
     } = 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
       .module_graph_builder
@@ -273,7 +273,7 @@ impl ModuleLoadPreparer {
         .collect::<Vec<_>>()
         .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
       .module_graph_builder

cli/tools/pm/cache_deps.rs

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

cli/tools/pm/deps.rs

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

cli/tools/pm/outdated/mod.rs

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

cli/tools/run/mod.rs

@@ -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 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
         .ensure_top_level_package_json_install()
         .await?;

cli/tools/task.rs

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

cli/util/progress_bar/mod.rs

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