refactor(cli): move snapshot_from_lockfile function to deno_npm

Cargo.toml

@@ -47,9 +47,9 @@ deno_runtime = { version = "0.122.0", path = "./runtime" }
 napi_sym = { version = "0.44.0", path = "./cli/napi/sym" }
 deno_bench_util = { version = "0.108.0", path = "./bench_util" }
 test_util = { path = "./test_util" }
-deno_lockfile = "0.14.1"
+deno_lockfile = "0.15.0"
 deno_media_type = { version = "0.1.1", features = ["module_specifier"] }
-deno_npm = "0.10.1"
+deno_npm = "0.11.0"
 deno_semver = "0.3.0"
 
 # exts

cli/Cargo.toml

@@ -56,7 +56,7 @@ deno_npm.workspace = true
 deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] }
 deno_semver.workspace = true
 deno_task_shell = "=0.13.1"
-eszip = "=0.48.0"
+eszip = "=0.49.0"
 napi_sym.workspace = true
 
 async-trait.workspace = true

cli/args/lockfile.rs

@@ -1,25 +1,14 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
-use std::collections::HashMap;
 use std::path::PathBuf;
 use std::sync::Arc;
 
-use deno_core::anyhow::bail;
-use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
-use deno_core::futures::stream::FuturesOrdered;
-use deno_core::futures::StreamExt;
 use deno_core::parking_lot::Mutex;
 use deno_npm::registry::NpmRegistryApi;
-use deno_npm::resolution::SerializedNpmResolutionSnapshot;
-use deno_npm::resolution::SerializedNpmResolutionSnapshotPackage;
 use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot;
-use deno_npm::NpmPackageId;
-use deno_npm::NpmResolutionPackageSystemInfo;
-use deno_semver::npm::NpmPackageReq;
 
 use crate::args::ConfigFile;
-use crate::npm::CliNpmRegistryApi;
 use crate::Flags;
 
 use super::DenoSubcommand;
@@ -63,96 +52,14 @@ pub fn discover(
 
 pub async fn snapshot_from_lockfile(
   lockfile: Arc<Mutex<Lockfile>>,
-  api: &CliNpmRegistryApi,
+  api: &dyn NpmRegistryApi,
 ) -> Result<ValidSerializedNpmResolutionSnapshot, AnyError> {
-  let (root_packages, mut packages) = {
-    let lockfile = lockfile.lock();
-
-    let mut root_packages =
-      HashMap::<NpmPackageReq, NpmPackageId>::with_capacity(
-        lockfile.content.npm.specifiers.len(),
-      );
-    // collect the specifiers to version mappings
-    for (key, value) in &lockfile.content.npm.specifiers {
-      let package_req = NpmPackageReq::from_str(key)
-        .with_context(|| format!("Unable to parse npm specifier: {key}"))?;
-      let package_id = NpmPackageId::from_serialized(value)?;
-      root_packages.insert(package_req, package_id.clone());
-    }
-
-    // now fill the packages except for the dist information
-    let mut packages = Vec::with_capacity(lockfile.content.npm.packages.len());
-    for (key, package) in &lockfile.content.npm.packages {
-      let id = NpmPackageId::from_serialized(key)?;
-
-      // collect the dependencies
-      let mut dependencies = HashMap::with_capacity(package.dependencies.len());
-      for (name, specifier) in &package.dependencies {
-        let dep_id = NpmPackageId::from_serialized(specifier)?;
-        dependencies.insert(name.clone(), dep_id);
-      }
-
-      packages.push(SerializedNpmResolutionSnapshotPackage {
-        id,
-        dependencies,
-        // temporarily empty
-        system: Default::default(),
-        dist: Default::default(),
-        optional_dependencies: Default::default(),
-      });
-    }
-    (root_packages, packages)
-  };
-
-  // now that the lockfile is dropped, fetch the package version information
-  let pkg_nvs = packages.iter().map(|p| p.id.nv.clone()).collect::<Vec<_>>();
-  let get_version_infos = || {
-    FuturesOrdered::from_iter(pkg_nvs.iter().map(|nv| async move {
-      let package_info = api.package_info(&nv.name).await?;
-      match package_info.version_info(nv) {
-        Ok(version_info) => Ok(version_info),
-        Err(err) => {
-          bail!("Could not find '{}' specified in the lockfile.", err.0);
-        }
-      }
-    }))
+  let incomplete_snapshot = {
+    let lock = lockfile.lock();
+    deno_npm::resolution::incomplete_snapshot_from_lockfile(&lock)?
   };
-  let mut version_infos = get_version_infos();
-  let mut i = 0;
-  while let Some(result) = version_infos.next().await {
-    match result {
-      Ok(version_info) => {
-        let package = &mut packages[i];
-        package.dist = version_info.dist;
-        package.system = NpmResolutionPackageSystemInfo {
-          cpu: version_info.cpu,
-          os: version_info.os,
-        };
-        package.optional_dependencies =
-          version_info.optional_dependencies.into_keys().collect();
-      }
-      Err(err) => {
-        if api.mark_force_reload() {
-          // reset and try again
-          version_infos = get_version_infos();
-          i = 0;
-          continue;
-        } else {
-          return Err(err);
-        }
-      }
-    }
-
-    i += 1;
-  }
-
-  // clear the memory cache to reduce memory usage
-  api.clear_memory_cache();
-
-  SerializedNpmResolutionSnapshot {
-    packages,
-    root_packages,
-  }
-  .into_valid()
-  .context("The lockfile is corrupt. You can recreate it with --lock-write")
+  let snapshot =
+    deno_npm::resolution::snapshot_from_lockfile(incomplete_snapshot, api)
+      .await?;
+  Ok(snapshot)
 }

cli/args/mod.rs

@@ -818,16 +818,17 @@ impl CliOptions {
 
     if let Some(lockfile) = self.maybe_lockfile() {
       if !lockfile.lock().overwrite {
-        return Ok(Some(
-          snapshot_from_lockfile(lockfile.clone(), api)
-            .await
-            .with_context(|| {
-              format!(
-                "failed reading lockfile '{}'",
-                lockfile.lock().filename.display()
-              )
-            })?,
-        ));
+        let snapshot = snapshot_from_lockfile(lockfile.clone(), api)
+          .await
+          .with_context(|| {
+            format!(
+              "failed reading lockfile '{}'",
+              lockfile.lock().filename.display()
+            )
+          })?;
+        // clear the memory cache to reduce memory usage
+        api.clear_memory_cache();
+        return Ok(Some(snapshot));
       }
     }
 

cli/lsp/language_server.rs

@@ -921,6 +921,25 @@ impl Inner {
     Ok(())
   }
 
+  async fn get_npm_snapshot(
+    &self,
+  ) -> Option<ValidSerializedNpmResolutionSnapshot> {
+    let lockfile = self.config.maybe_lockfile()?;
+    let snapshot =
+      match snapshot_from_lockfile(lockfile.clone(), &*self.npm.api).await {
+        Ok(snapshot) => snapshot,
+        Err(err) => {
+          lsp_warn!("Failed getting npm snapshot from lockfile: {}", err);
+          return None;
+        }
+      };
+
+    // clear the memory cache to reduce memory usage
+    self.npm.api.clear_memory_cache();
+
+    Some(snapshot)
+  }
+
   async fn recreate_npm_services_if_necessary(&mut self) {
     let deno_dir = match DenoDir::new(self.maybe_global_cache_path.clone()) {
       Ok(deno_dir) => deno_dir,
@@ -942,18 +961,7 @@ impl Inner {
       registry_url,
       &progress_bar,
     );
-    let maybe_snapshot = match self.config.maybe_lockfile() {
-      Some(lockfile) => {
-        match snapshot_from_lockfile(lockfile.clone(), &self.npm.api).await {
-          Ok(snapshot) => Some(snapshot),
-          Err(err) => {
-            lsp_warn!("Failed getting npm snapshot from lockfile: {}", err);
-            None
-          }
-        }
-      }
-      None => None,
-    };
+    let maybe_snapshot = self.get_npm_snapshot().await;
     (self.npm.resolver, self.npm.resolution) =
       create_npm_resolver_and_resolution(
         registry_url,

cli/npm/registry.rs

@@ -99,27 +99,6 @@ impl CliNpmRegistryApi {
     &self.inner().base_url
   }
 
-  /// Marks that new requests for package information should retrieve it
-  /// from the npm registry
-  ///
-  /// Returns true if it was successfully set for the first time.
-  pub fn mark_force_reload(&self) -> bool {
-    // never force reload the registry information if reloading
-    // is disabled or if we're already reloading
-    if matches!(
-      self.inner().cache.cache_setting(),
-      CacheSetting::Only | CacheSetting::ReloadAll
-    ) {
-      return false;
-    }
-    if self.inner().force_reload_flag.raise() {
-      self.clear_memory_cache(); // clear the memory cache to force reloading
-      true
-    } else {
-      false
-    }
-  }
-
   fn inner(&self) -> &Arc<CliNpmRegistryApiInner> {
     // this panicking indicates a bug in the code where this
     // wasn't initialized
@@ -154,6 +133,23 @@ impl NpmRegistryApi for CliNpmRegistryApi {
       }
     }
   }
+
+  fn mark_force_reload(&self) -> bool {
+    // never force reload the registry information if reloading
+    // is disabled or if we're already reloading
+    if matches!(
+      self.inner().cache.cache_setting(),
+      CacheSetting::Only | CacheSetting::ReloadAll
+    ) {
+      return false;
+    }
+    if self.inner().force_reload_flag.raise() {
+      self.clear_memory_cache(); // clear the cache to force reloading
+      true
+    } else {
+      false
+    }
+  }
 }
 
 type CacheItemPendingResult =
@@ -386,7 +382,7 @@ impl CliNpmRegistryApiInner {
     name_folder_path.join("registry.json")
   }
 
-  pub fn clear_memory_cache(&self) {
+  fn clear_memory_cache(&self) {
     self.mem_cache.lock().clear();
   }
 

cli/npm/resolution.rs

@@ -11,6 +11,7 @@ use deno_core::TaskQueue;
 use deno_lockfile::NpmPackageDependencyLockfileInfo;
 use deno_lockfile::NpmPackageLockfileInfo;
 use deno_npm::registry::NpmPackageInfo;
+use deno_npm::registry::NpmRegistryApi;
 use deno_npm::resolution::NpmPackageVersionResolutionError;
 use deno_npm::resolution::NpmPackagesPartitioned;
 use deno_npm::resolution::NpmResolutionError;

cli/tests/integration/npm_tests.rs

@@ -1915,7 +1915,8 @@ fn reload_info_not_found_cache_but_exists_remote() {
       "error: failed reading lockfile '[WILDCARD]deno.lock'\n",
       "\n",
       "Caused by:\n",
-      "    Could not find '@denotest/esm-basic@1.0.0' specified in the lockfile.\n"
+      "    0: Could not find '@denotest/esm-basic@1.0.0' specified in the lockfile.\n",
+      "    1: Could not find version '1.0.0' for npm package '@denotest/esm-basic'.\n",
     ));
     output.assert_exit_code(1);