From 5becfd6381889287ff16a064128021f87c8dfcb6 Mon Sep 17 00:00:00 2001
From: David Sherret <dsherret@users.noreply.github.com>
Date: Tue, 21 Feb 2023 15:19:09 -0500
Subject: [PATCH] fix(npm): filter out duplicate packages names in resolution
 (#17857)

---
 cli/npm/resolution/graph.rs    | 54 +++++++++++++++++-----------------
 cli/npm/resolution/mod.rs      | 18 +++++++++++-
 cli/npm/resolution/snapshot.rs | 27 ++++++++++++++++-
 3 files changed, 70 insertions(+), 29 deletions(-)

diff --git a/cli/npm/resolution/graph.rs b/cli/npm/resolution/graph.rs
index 76ec2e62cc..966e1f0106 100644
--- a/cli/npm/resolution/graph.rs
+++ b/cli/npm/resolution/graph.rs
@@ -584,14 +584,14 @@ impl Graph {
         .nodes_by_package_name
         .into_iter()
         .map(|(name, ids)| {
-          (
-            name,
-            ids
-              .into_iter()
-              .filter(|id| traversed_node_ids.contains(id))
-              .map(|id| packages_to_resolved_id.get(&id).unwrap().clone())
-              .collect(),
-          )
+          let mut ids = ids
+            .into_iter()
+            .filter(|id| traversed_node_ids.contains(id))
+            .map(|id| packages_to_resolved_id.get(&id).unwrap().clone())
+            .collect::<Vec<_>>();
+          ids.sort();
+          ids.dedup();
+          (name, ids)
         })
         .collect(),
       packages,
@@ -3586,25 +3586,25 @@ mod test {
     let snapshot = graph.into_snapshot(&api).await.unwrap();
 
     {
-      // let new_snapshot = Graph::from_snapshot(snapshot.clone())
-      //   .unwrap()
-      //   .into_snapshot(&api)
-      //   .await
-      //   .unwrap();
-      // assert_eq!(
-      //   snapshot, new_snapshot,
-      //   "recreated snapshot should be the same"
-      // );
-      // // create one again from the new snapshot
-      // let new_snapshot2 = Graph::from_snapshot(new_snapshot.clone())
-      //   .unwrap()
-      //   .into_snapshot(&api)
-      //   .await
-      //   .unwrap();
-      // assert_eq!(
-      //   snapshot, new_snapshot2,
-      //   "second recreated snapshot should be the same"
-      // );
+      let new_snapshot = Graph::from_snapshot(snapshot.clone())
+        .unwrap()
+        .into_snapshot(&api)
+        .await
+        .unwrap();
+      assert_eq!(
+        snapshot, new_snapshot,
+        "recreated snapshot should be the same"
+      );
+      // create one again from the new snapshot
+      let new_snapshot2 = Graph::from_snapshot(new_snapshot.clone())
+        .unwrap()
+        .into_snapshot(&api)
+        .await
+        .unwrap();
+      assert_eq!(
+        snapshot, new_snapshot2,
+        "second recreated snapshot should be the same"
+      );
     }
 
     let mut packages = snapshot.all_packages();
diff --git a/cli/npm/resolution/mod.rs b/cli/npm/resolution/mod.rs
index 90bf16c432..c95124b61b 100644
--- a/cli/npm/resolution/mod.rs
+++ b/cli/npm/resolution/mod.rs
@@ -1,6 +1,7 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
 use std::cmp::Ordering;
+use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::collections::HashSet;
 
@@ -191,7 +192,7 @@ impl PartialOrd for NpmPackageId {
   }
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
+#[derive(Clone, Serialize, Deserialize, PartialEq, Eq)]
 pub struct NpmResolutionPackage {
   pub pkg_id: NpmPackageId,
   /// The peer dependency resolution can differ for the same
@@ -205,6 +206,21 @@ pub struct NpmResolutionPackage {
   pub dependencies: HashMap<String, NpmPackageId>,
 }
 
+impl std::fmt::Debug for NpmResolutionPackage {
+  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    // custom debug implementation for deterministic output in the tests
+    f.debug_struct("NpmResolutionPackage")
+      .field("pkg_id", &self.pkg_id)
+      .field("copy_index", &self.copy_index)
+      .field("dist", &self.dist)
+      .field(
+        "dependencies",
+        &self.dependencies.iter().collect::<BTreeMap<_, _>>(),
+      )
+      .finish()
+  }
+}
+
 impl NpmResolutionPackage {
   pub fn get_package_cache_folder_id(&self) -> NpmPackageCacheFolderId {
     NpmPackageCacheFolderId {
diff --git a/cli/npm/resolution/snapshot.rs b/cli/npm/resolution/snapshot.rs
index bdc204ce8b..3fc82cbb8e 100644
--- a/cli/npm/resolution/snapshot.rs
+++ b/cli/npm/resolution/snapshot.rs
@@ -1,5 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
+use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::sync::Arc;
@@ -42,7 +43,7 @@ impl NpmPackagesPartitioned {
   }
 }
 
-#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)]
+#[derive(Clone, Default, Serialize, Deserialize, PartialEq, Eq)]
 pub struct NpmResolutionSnapshot {
   /// The unique package requirements map to a single npm package name and version.
   #[serde(with = "map_to_vec")]
@@ -55,6 +56,30 @@ pub struct NpmResolutionSnapshot {
   pub(super) packages: HashMap<NpmPackageId, NpmResolutionPackage>,
 }
 
+impl std::fmt::Debug for NpmResolutionSnapshot {
+  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    // do a custom debug implementation that creates deterministic output for the tests
+    f.debug_struct("NpmResolutionSnapshot")
+      .field(
+        "package_reqs",
+        &self.package_reqs.iter().collect::<BTreeMap<_, _>>(),
+      )
+      .field(
+        "root_packages",
+        &self.root_packages.iter().collect::<BTreeMap<_, _>>(),
+      )
+      .field(
+        "packages_by_name",
+        &self.packages_by_name.iter().collect::<BTreeMap<_, _>>(),
+      )
+      .field(
+        "packages",
+        &self.packages.iter().collect::<BTreeMap<_, _>>(),
+      )
+      .finish()
+  }
+}
+
 // This is done so the maps with non-string keys get serialized and deserialized as vectors.
 // Adapted from: https://github.com/serde-rs/serde/issues/936#issuecomment-302281792
 mod map_to_vec {