Skip to content

Commit

Permalink
fix(npm): show a progress bar when initializing the node_modules fold…
Browse files Browse the repository at this point in the history
…er (denoland#18136)

Creating the node_modules folder when the packages are already
downloaded can take a bit of time and not knowing what is going on can
be confusing. It's better to show a progress bar.
  • Loading branch information
dsherret committed Mar 13, 2023
1 parent e8f22c0 commit 44b0d4c
Show file tree
Hide file tree
Showing 20 changed files with 257 additions and 166 deletions.
4 changes: 3 additions & 1 deletion cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,11 +345,12 @@ fn create_lsp_structs(
registry_url.clone(),
npm_cache.clone(),
http_client,
progress_bar,
progress_bar.clone(),
);
let resolution = NpmResolution::new(api.clone(), None, None);
let fs_resolver = create_npm_fs_resolver(
npm_cache.clone(),
&progress_bar,
registry_url.clone(),
resolution.clone(),
None,
Expand Down Expand Up @@ -609,6 +610,7 @@ impl Inner {
resolution.clone(),
create_npm_fs_resolver(
self.npm_cache.clone(),
&ProgressBar::new(ProgressBarStyle::TextOnly),
self.npm_api.base_url().clone(),
resolution,
None,
Expand Down
38 changes: 24 additions & 14 deletions cli/npm/resolvers/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use std::path::PathBuf;

use crate::util::fs::symlink_dir;
use crate::util::fs::LaxSingleProcessFsFlag;
use crate::util::progress_bar::ProgressBar;
use crate::util::progress_bar::ProgressMessagePrompt;
use async_trait::async_trait;
use deno_ast::ModuleSpecifier;
use deno_core::anyhow::bail;
Expand Down Expand Up @@ -42,6 +44,7 @@ use super::common::NpmPackageFsResolver;
#[derive(Debug, Clone)]
pub struct LocalNpmPackageResolver {
cache: NpmCache,
progress_bar: ProgressBar,
resolution: NpmResolution,
registry_url: Url,
root_node_modules_path: PathBuf,
Expand All @@ -51,12 +54,14 @@ pub struct LocalNpmPackageResolver {
impl LocalNpmPackageResolver {
pub fn new(
cache: NpmCache,
progress_bar: ProgressBar,
registry_url: Url,
node_modules_folder: PathBuf,
resolution: NpmResolution,
) -> Self {
Self {
cache,
progress_bar,
resolution,
registry_url,
root_node_modules_url: Url::from_directory_path(&node_modules_folder)
Expand Down Expand Up @@ -200,8 +205,14 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
}

async fn cache_packages(&self) -> Result<(), AnyError> {
sync_resolver_with_fs(self).await?;
Ok(())
sync_resolution_with_fs(
&self.resolution.snapshot(),
&self.cache,
&self.progress_bar,
&self.registry_url,
&self.root_node_modules_path,
)
.await
}

fn ensure_read_permission(
Expand All @@ -217,22 +228,11 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
}
}

async fn sync_resolver_with_fs(
resolver: &LocalNpmPackageResolver,
) -> Result<(), AnyError> {
sync_resolution_with_fs(
&resolver.resolution.snapshot(),
&resolver.cache,
&resolver.registry_url,
&resolver.root_node_modules_path,
)
.await
}

/// Creates a pnpm style folder structure.
async fn sync_resolution_with_fs(
snapshot: &NpmResolutionSnapshot,
cache: &NpmCache,
progress_bar: &ProgressBar,
registry_url: &Url,
root_node_modules_dir_path: &Path,
) -> Result<(), AnyError> {
Expand All @@ -252,6 +252,8 @@ async fn sync_resolution_with_fs(
)
.await;

let pb_clear_guard = progress_bar.clear_guard(); // prevent flickering

// 1. Write all the packages out the .deno directory.
//
// Copy (hardlink in future) <global_registry_cache>/<package_id>/ to
Expand All @@ -277,13 +279,18 @@ async fn sync_resolution_with_fs(
.should_use_for_npm_package(&package.pkg_id.nv.name)
|| !initialized_file.exists()
{
let pb = progress_bar.clone();
let cache = cache.clone();
let registry_url = registry_url.clone();
let package = package.clone();
let handle = tokio::task::spawn(async move {
cache
.ensure_package(&package.pkg_id.nv, &package.dist, &registry_url)
.await?;
let pb_guard = pb.update_with_prompt(
ProgressMessagePrompt::Initialize,
&package.pkg_id.nv.to_string(),
);
let sub_node_modules = folder_path.join("node_modules");
let package_path =
join_package_name(&sub_node_modules, &package.pkg_id.nv.name);
Expand All @@ -297,6 +304,8 @@ async fn sync_resolution_with_fs(
copy_dir_recursive(&cache_folder, &package_path)?;
// write out a file that indicates this folder has been initialized
fs::write(initialized_file, "")?;
// finally stop showing the progress bar
drop(pb_guard); // explicit for clarity
Ok(())
});
if sync_download {
Expand Down Expand Up @@ -411,6 +420,7 @@ async fn sync_resolution_with_fs(
}

drop(single_process_lock);
drop(pb_clear_guard);

Ok(())
}
Expand Down
3 changes: 3 additions & 0 deletions cli/npm/resolvers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use std::sync::Arc;

use crate::args::Lockfile;
use crate::util::fs::canonicalize_path_maybe_not_exists;
use crate::util::progress_bar::ProgressBar;

use self::common::NpmPackageFsResolver;
use self::local::LocalNpmPackageResolver;
Expand Down Expand Up @@ -276,13 +277,15 @@ impl RequireNpmResolver for NpmPackageResolver {

pub fn create_npm_fs_resolver(
cache: NpmCache,
progress_bar: &ProgressBar,
registry_url: Url,
resolution: NpmResolution,
maybe_node_modules_path: Option<PathBuf>,
) -> Arc<dyn NpmPackageFsResolver> {
match maybe_node_modules_path {
Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new(
cache,
progress_bar.clone(),
registry_url,
node_modules_folder,
resolution,
Expand Down
1 change: 1 addition & 0 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ impl ProcState {
);
let npm_fs_resolver = create_npm_fs_resolver(
npm_cache,
&progress_bar,
npm_registry_url,
npm_resolution.clone(),
cli_options.node_modules_dir_path(),
Expand Down
Loading

0 comments on commit 44b0d4c

Please sign in to comment.