fix(npm): store npm binary command resolution in lockfile (#19219)

Part of #19038

Closes #19034 (eliminates the time spent re-resolving)

3 files changed, 35 insertions, 21 deletions

diff --git a/cli/npm/installer.rs b/cli/npm/installer.rs
index 43f79d8f0..8590feb9c 100644
--- a/cli/npm/installer.rs
+++ b/cli/npm/installer.rs
@@ -25,23 +25,22 @@ struct PackageJsonDepsInstallerInner {
 }
 
 impl PackageJsonDepsInstallerInner {
-  pub fn reqs_with_info_futures(
+  pub fn reqs_with_info_futures<'a>(
     &self,
+    reqs: &'a [&'a NpmPackageReq],
   ) -> FuturesOrdered<
     impl Future<
       Output = Result<
-        (&NpmPackageReq, Arc<deno_npm::registry::NpmPackageInfo>),
+        (&'a NpmPackageReq, Arc<deno_npm::registry::NpmPackageInfo>),
         NpmRegistryPackageInfoLoadError,
       >,
     >,
   > {
-    let package_reqs = self.deps_provider.reqs();
-
-    FuturesOrdered::from_iter(package_reqs.into_iter().map(|req| {
+    FuturesOrdered::from_iter(reqs.iter().map(|req| {
       let api = self.npm_registry_api.clone();
       async move {
         let info = api.package_info(&req.name).await?;
-        Ok::<_, NpmRegistryPackageInfoLoadError>((req, info))
+        Ok::<_, NpmRegistryPackageInfoLoadError>((*req, info))
       }
     }))
   }
@@ -77,7 +76,24 @@ impl PackageJsonDepsInstaller {
       return Ok(()); // already installed by something else
     }
 
-    let mut reqs_with_info_futures = inner.reqs_with_info_futures();
+    let package_reqs = inner.deps_provider.reqs();
+
+    // check if something needs resolving before bothering to load all
+    // the package information (which is slow)
+    if package_reqs.iter().all(|req| {
+      inner
+        .npm_resolution
+        .resolve_pkg_id_from_pkg_req(req)
+        .is_ok()
+    }) {
+      log::debug!(
+        "All package.json deps resolvable. Skipping top level install."
+      );
+      return Ok(()); // everything is already resolvable
+    }
+
+    let mut reqs_with_info_futures =
+      inner.reqs_with_info_futures(&package_reqs);
 
     while let Some(result) = reqs_with_info_futures.next().await {
       let (req, info) = result?;
@@ -88,7 +104,7 @@ impl PackageJsonDepsInstaller {
         if inner.npm_registry_api.mark_force_reload() {
           log::debug!("Failed to resolve package. Retrying. Error: {err:#}");
           // re-initialize
-          reqs_with_info_futures = inner.reqs_with_info_futures();
+          reqs_with_info_futures = inner.reqs_with_info_futures(&package_reqs);
         } else {
           return Err(err.into());
         }
diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs
index 3e9438ffa..eba4069e7 100644
--- a/cli/npm/resolution.rs
+++ b/cli/npm/resolution.rs
@@ -89,7 +89,7 @@ impl NpmResolution {
 
   pub async fn add_package_reqs(
     &self,
-    package_reqs: Vec<NpmPackageReq>,
+    package_reqs: &[NpmPackageReq],
   ) -> Result<(), AnyError> {
     // only allow one thread in here at a time
     let _permit = self.update_queue.acquire().await;
@@ -107,12 +107,12 @@ impl NpmResolution {
 
   pub async fn set_package_reqs(
     &self,
-    package_reqs: Vec<NpmPackageReq>,
+    package_reqs: &[NpmPackageReq],
   ) -> Result<(), AnyError> {
     // only allow one thread in here at a time
     let _permit = self.update_queue.acquire().await;
 
-    let reqs_set = package_reqs.iter().cloned().collect::<HashSet<_>>();
+    let reqs_set = package_reqs.iter().collect::<HashSet<_>>();
     let snapshot = add_package_reqs_to_snapshot(
       &self.api,
       package_reqs,
@@ -144,7 +144,7 @@ impl NpmResolution {
 
     let snapshot = add_package_reqs_to_snapshot(
       &self.api,
-      Vec::new(),
+      &Vec::new(),
       self.maybe_lockfile.clone(),
       || self.snapshot.read().clone(),
     )
@@ -275,10 +275,7 @@ impl NpmResolution {
 
 async fn add_package_reqs_to_snapshot(
   api: &CliNpmRegistryApi,
-  // todo(18079): it should be possible to pass &[NpmPackageReq] in here
-  // and avoid all these clones, but the LSP complains because of its
-  // `Send` requirement
-  package_reqs: Vec<NpmPackageReq>,
+  package_reqs: &[NpmPackageReq],
   maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
   get_new_snapshot: impl Fn() -> NpmResolutionSnapshot,
 ) -> Result<NpmResolutionSnapshot, AnyError> {
@@ -288,10 +285,11 @@ async fn add_package_reqs_to_snapshot(
       .iter()
       .all(|req| snapshot.package_reqs().contains_key(req))
   {
-    return Ok(snapshot); // already up to date
+    log::debug!("Snapshot already up to date. Skipping pending resolution.");
+    return Ok(snapshot);
   }
 
-  let result = snapshot.resolve_pending(package_reqs.clone()).await;
+  let result = snapshot.resolve_pending(package_reqs).await;
   api.clear_memory_cache();
   let snapshot = match result {
     Ok(snapshot) => snapshot,
diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs
index 0f123c382..168786407 100644
--- a/cli/npm/resolvers/mod.rs
+++ b/cli/npm/resolvers/mod.rs
@@ -159,7 +159,7 @@ impl CliNpmResolver {
   /// Adds package requirements to the resolver and ensures everything is setup.
   pub async fn add_package_reqs(
     &self,
-    packages: Vec<NpmPackageReq>,
+    packages: &[NpmPackageReq],
   ) -> Result<(), AnyError> {
     if packages.is_empty() {
       return Ok(());
@@ -182,7 +182,7 @@ impl CliNpmResolver {
   /// This will retrieve and resolve package information, but not cache any package files.
   pub async fn set_package_reqs(
     &self,
-    packages: Vec<NpmPackageReq>,
+    packages: &[NpmPackageReq],
   ) -> Result<(), AnyError> {
     self.resolution.set_package_reqs(packages).await
   }
@@ -212,7 +212,7 @@ impl CliNpmResolver {
   ) -> Result<(), AnyError> {
     // add and ensure this isn't added to the lockfile
     let package_reqs = vec![NpmPackageReq::from_str("@types/node").unwrap()];
-    self.resolution.add_package_reqs(package_reqs).await?;
+    self.resolution.add_package_reqs(&package_reqs).await?;
     self.fs_resolver.cache_packages().await?;
 
     Ok(())