fix(npm): improve peer dependency resolution (#17835)

This PR fixes peer dependency resolution to only resolve peers based on
the current graph traversal path. Previously, it would resolve a peers
by looking at a graph node's ancestors, which is not correct because
graph nodes are shared by different resolutions.

It also stores more information about peer dependency resolution in the
lockfile.

1 files changed, 32 insertions, 29 deletions

diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs
index 0eb2c24ca..aa6233d61 100644
--- a/cli/npm/resolvers/local.rs
+++ b/cli/npm/resolvers/local.rs
@@ -32,9 +32,9 @@ use crate::npm::cache::NpmPackageCacheFolderId;
 use crate::npm::resolution::NpmResolution;
 use crate::npm::resolution::NpmResolutionSnapshot;
 use crate::npm::NpmCache;
-use crate::npm::NpmPackageNodeId;
+use crate::npm::NpmPackageId;
+use crate::npm::NpmRegistryApi;
 use crate::npm::NpmResolutionPackage;
-use crate::npm::RealNpmRegistryApi;
 use crate::util::fs::copy_dir_recursive;
 use crate::util::fs::hard_link_dir_recursive;
 
@@ -56,7 +56,7 @@ pub struct LocalNpmPackageResolver {
 impl LocalNpmPackageResolver {
   pub fn new(
     cache: NpmCache,
-    api: RealNpmRegistryApi,
+    api: NpmRegistryApi,
     node_modules_folder: PathBuf,
     initial_snapshot: Option<NpmResolutionSnapshot>,
   ) -> Self {
@@ -112,7 +112,7 @@ impl LocalNpmPackageResolver {
 
   fn get_package_id_folder(
     &self,
-    package_id: &NpmPackageNodeId,
+    package_id: &NpmPackageId,
   ) -> Result<PathBuf, AnyError> {
     match self.resolution.resolve_package_from_id(package_id) {
       Some(package) => Ok(self.get_package_id_folder_from_package(&package)),
@@ -136,7 +136,7 @@ impl LocalNpmPackageResolver {
         &package.get_package_cache_folder_id(),
       ))
       .join("node_modules")
-      .join(&package.id.name)
+      .join(&package.pkg_id.nv.name)
   }
 }
 
@@ -203,10 +203,7 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver {
     Ok(package_root_path)
   }
 
-  fn package_size(
-    &self,
-    package_id: &NpmPackageNodeId,
-  ) -> Result<u64, AnyError> {
+  fn package_size(&self, package_id: &NpmPackageId) -> Result<u64, AnyError> {
     let package_folder_path = self.get_package_id_folder(package_id)?;
 
     Ok(crate::util::fs::dir_size(&package_folder_path)?)
@@ -303,7 +300,9 @@ async fn sync_resolution_with_fs(
   if sync_download {
     // we're running the tests not with --quiet
     // and we want the output to be deterministic
-    package_partitions.packages.sort_by(|a, b| a.id.cmp(&b.id));
+    package_partitions
+      .packages
+      .sort_by(|a, b| a.pkg_id.cmp(&b.pkg_id));
   }
   let mut handles: Vec<JoinHandle<Result<(), AnyError>>> =
     Vec::with_capacity(package_partitions.packages.len());
@@ -314,7 +313,7 @@ async fn sync_resolution_with_fs(
     let initialized_file = folder_path.join(".initialized");
     if !cache
       .cache_setting()
-      .should_use_for_npm_package(&package.id.name)
+      .should_use_for_npm_package(&package.pkg_id.nv.name)
       || !initialized_file.exists()
     {
       let cache = cache.clone();
@@ -323,19 +322,19 @@ async fn sync_resolution_with_fs(
       let handle = tokio::task::spawn(async move {
         cache
           .ensure_package(
-            (&package.id.name, &package.id.version),
+            (&package.pkg_id.nv.name, &package.pkg_id.nv.version),
             &package.dist,
             &registry_url,
           )
           .await?;
         let sub_node_modules = folder_path.join("node_modules");
         let package_path =
-          join_package_name(&sub_node_modules, &package.id.name);
+          join_package_name(&sub_node_modules, &package.pkg_id.nv.name);
         fs::create_dir_all(&package_path)
           .with_context(|| format!("Creating '{}'", folder_path.display()))?;
         let cache_folder = cache.package_folder_for_name_and_version(
-          &package.id.name,
-          &package.id.version,
+          &package.pkg_id.nv.name,
+          &package.pkg_id.nv.version,
           &registry_url,
         );
         // for now copy, but in the future consider hard linking
@@ -365,7 +364,8 @@ async fn sync_resolution_with_fs(
     let initialized_file = destination_path.join(".initialized");
     if !initialized_file.exists() {
       let sub_node_modules = destination_path.join("node_modules");
-      let package_path = join_package_name(&sub_node_modules, &package.id.name);
+      let package_path =
+        join_package_name(&sub_node_modules, &package.pkg_id.nv.name);
       fs::create_dir_all(&package_path).with_context(|| {
         format!("Creating '{}'", destination_path.display())
       })?;
@@ -375,7 +375,7 @@ async fn sync_resolution_with_fs(
             &package_cache_folder_id.with_no_count(),
           ))
           .join("node_modules"),
-        &package.id.name,
+        &package.pkg_id.nv.name,
       );
       hard_link_dir_recursive(&source_path, &package_path)?;
       // write out a file that indicates this folder has been initialized
@@ -406,7 +406,7 @@ async fn sync_resolution_with_fs(
         &deno_local_registry_dir
           .join(dep_folder_name)
           .join("node_modules"),
-        &dep_id.name,
+        &dep_id.nv.name,
       );
       symlink_package_dir(
         &dep_folder_path,
@@ -428,10 +428,10 @@ async fn sync_resolution_with_fs(
       .map(|id| (id, true)),
   );
   while let Some((package_id, is_top_level)) = pending_packages.pop_front() {
-    let root_folder_name = if found_names.insert(package_id.name.clone()) {
-      package_id.name.clone()
+    let root_folder_name = if found_names.insert(package_id.nv.name.clone()) {
+      package_id.nv.name.clone()
     } else if is_top_level {
-      package_id.display()
+      format!("{}@{}", package_id.nv.name, package_id.nv.version)
     } else {
       continue; // skip, already handled
     };
@@ -442,7 +442,7 @@ async fn sync_resolution_with_fs(
           &package.get_package_cache_folder_id(),
         ))
         .join("node_modules"),
-      &package_id.name,
+      &package_id.nv.name,
     );
 
     symlink_package_dir(
@@ -457,18 +457,21 @@ async fn sync_resolution_with_fs(
   Ok(())
 }
 
-fn get_package_folder_id_folder_name(id: &NpmPackageCacheFolderId) -> String {
-  let copy_str = if id.copy_index == 0 {
+fn get_package_folder_id_folder_name(
+  folder_id: &NpmPackageCacheFolderId,
+) -> String {
+  let copy_str = if folder_id.copy_index == 0 {
     "".to_string()
   } else {
-    format!("_{}", id.copy_index)
+    format!("_{}", folder_id.copy_index)
   };
-  let name = if id.name.to_lowercase() == id.name {
-    Cow::Borrowed(&id.name)
+  let nv = &folder_id.nv;
+  let name = if nv.name.to_lowercase() == nv.name {
+    Cow::Borrowed(&nv.name)
   } else {
-    Cow::Owned(format!("_{}", mixed_case_package_name_encode(&id.name)))
+    Cow::Owned(format!("_{}", mixed_case_package_name_encode(&nv.name)))
   };
-  format!("{}@{}{}", name, id.version, copy_str).replace('/', "+")
+  format!("{}@{}{}", name, nv.version, copy_str).replace('/', "+")
 }
 
 fn symlink_package_dir(