fix: include all jsr deps in lockfile (#15)

denoland · Feb 5, 2024 · 2b95dcd · 2b95dcd
1 parent 12b73d5
commit 2b95dcd
Show file tree

Hide file tree

Showing 15 changed files with 97 additions and 31 deletions.
diff --git a/src/graphs.rs b/src/graphs.rs
@@ -32,19 +32,21 @@ impl LockfileNpmPackageId {
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 struct LockfilePkgReq(String);
 
+#[derive(Debug)]
 enum LockfileGraphPackage {
   Jsr(LockfileJsrGraphPackage),
   Npm(LockfileNpmGraphPackage),
 }
 
+#[derive(Debug)]
 struct LockfileNpmGraphPackage {
   /// Root ids that transitively reference this package.
   root_ids: HashSet<LockfilePkgId>,
   integrity: String,
   dependencies: BTreeMap<String, LockfileNpmPackageId>,
 }
 
-#[derive(Default)]
+#[derive(Debug, Default)]
 struct LockfileJsrGraphPackage {
   /// Root ids that transitively reference this package.
   root_ids: HashSet<LockfilePkgId>,
@@ -286,21 +288,19 @@ impl<FNvToJsrUrl: Fn(&str) -> Option<String>>
     for (id, package) in self.packages {
       match package {
         LockfileGraphPackage::Jsr(package) => {
-          if !package.dependencies.is_empty() {
-            packages.jsr.insert(
-              match id {
-                LockfilePkgId::Jsr(nv) => nv.0,
-                LockfilePkgId::Npm(_) => unreachable!(),
-              },
-              crate::JsrPackageInfo {
-                dependencies: package
-                  .dependencies
-                  .into_iter()
-                  .map(|req| req.0)
-                  .collect(),
-              },
-            );
-          }
+          packages.jsr.insert(
+            match id {
+              LockfilePkgId::Jsr(nv) => nv.0,
+              LockfilePkgId::Npm(_) => unreachable!(),
+            },
+            crate::JsrPackageInfo {
+              dependencies: package
+                .dependencies
+                .into_iter()
+                .map(|req| req.0)
+                .collect(),
+            },
+          );
         }
         LockfileGraphPackage::Npm(package) => {
           packages.npm.insert(

diff --git a/src/lib.rs b/src/lib.rs
@@ -165,7 +165,7 @@ pub struct PackagesContent {
 
 impl PackagesContent {
   fn is_empty(&self) -> bool {
-    self.specifiers.is_empty() && self.npm.is_empty()
+    self.specifiers.is_empty() && self.npm.is_empty() && self.jsr.is_empty()
   }
 }
 

diff --git a/tests/integration_test.rs b/tests/integration_test.rs
@@ -144,9 +144,8 @@ fn verify_packages_content(packages: &PackagesContent) {
   for id in packages.specifiers.values() {
     if let Some(npm_id) = id.strip_prefix("npm:") {
       assert!(packages.npm.contains_key(npm_id), "Missing: {}", id);
-    } else if id.strip_prefix("jsr:").is_some() {
-      // ignore jsr packages because they won't be in the lockfile when they don't have dependencies
-      // todo(dsherret): actually include them here because we need to lock the manifest version
+    } else if let Some(jsr_id) = id.strip_prefix("jsr:") {
+      assert!(packages.jsr.contains_key(jsr_id), "Missing: {}", id);
     } else {
       panic!("Invalid package id: {}", id);
     }
@@ -161,6 +160,31 @@ fn verify_packages_content(packages: &PackagesContent) {
       );
     }
   }
+  for (pkg_id, package) in &packages.jsr {
+    for req in &package.dependencies {
+      let dep_id = match packages.specifiers.get(req) {
+        Some(dep_id) => dep_id,
+        None => panic!("Missing specifier for '{}' in '{}'", req, pkg_id),
+      };
+      if let Some(npm_id) = dep_id.strip_prefix("npm:") {
+        assert!(
+          packages.npm.contains_key(npm_id),
+          "Missing: '{}' dep in '{}'",
+          dep_id,
+          pkg_id,
+        );
+      } else if let Some(jsr_id) = dep_id.strip_prefix("jsr:") {
+        assert!(
+          packages.jsr.contains_key(jsr_id),
+          "Missing: '{}' dep in '{}'",
+          dep_id,
+          pkg_id,
+        );
+      } else {
+        panic!("Invalid package id: {}", dep_id);
+      }
+    }
+  }
 }
 
 #[test]

diff --git a/tests/specs/config_changes/CircularJsrDep02.txt b/tests/specs/config_changes/CircularJsrDep02.txt
@@ -5,7 +5,7 @@
     "specifiers": {
       "jsr:@scope/package_a": "jsr:@scope/[email protected]",
       "jsr:@scope/package_b": "jsr:@scope/[email protected]",
-      "jsr:@scope/package_c": "jsr:@scope/package_b@0.0.1"
+      "jsr:@scope/package_c": "jsr:@scope/package_c@0.0.1"
     },
     "jsr": {
       "@scope/[email protected]": {

diff --git a/tests/specs/config_changes/NoConfigMaintains.txt b/tests/specs/config_changes/NoConfigMaintains.txt
@@ -24,7 +24,8 @@
         "dependencies": [
           "jsr:@scope/package_a"
         ]
-      }
+      },
+      "@scope/[email protected]": {}
     },
     "npm": {
       "@example/orphan": {
@@ -225,7 +226,8 @@
         "dependencies": [
           "jsr:@scope/package_a"
         ]
-      }
+      },
+      "@scope/[email protected]": {}
     },
     "npm": {
       "@example/orphan": {

diff --git a/tests/specs/config_changes/NoConfigWorkspaceMaintains.txt b/tests/specs/config_changes/NoConfigWorkspaceMaintains.txt
@@ -24,7 +24,8 @@
         "dependencies": [
           "jsr:@scope/package_a"
         ]
-      }
+      },
+      "@scope/[email protected]": {}
     },
     "npm": {
       "@example/orphan": {
@@ -229,7 +230,8 @@
         "dependencies": [
           "jsr:@scope/package_a"
         ]
-      }
+      },
+      "@scope/[email protected]": {}
     },
     "npm": {
       "@example/orphan": {

diff --git a/tests/specs/config_changes/NoNpmMaintainsNpm.txt b/tests/specs/config_changes/NoNpmMaintainsNpm.txt
@@ -207,6 +207,9 @@
       "jsr:@scope/package_orphan": "jsr:@scope/[email protected]",
       "npm:ts-morph": "npm:[email protected]"
     },
+    "jsr": {
+      "@scope/[email protected]": {}
+    },
     "npm": {
       "@example/orphan": {
         "integrity": "sha512-vq24Bq3ym5HEQm2NKCr3yXDwjc7vTsEThRDnkp2DK9p1uqLR+DHurm/NOTo0KG7HYHU7eppKZj3MyqYuMBf62g==",

diff --git a/tests/specs/config_changes/NoNpmWorkspaceMaintainsNpm.txt b/tests/specs/config_changes/NoNpmWorkspaceMaintainsNpm.txt
@@ -24,7 +24,8 @@
         "dependencies": [
           "jsr:@scope/package_a"
         ]
-      }
+      },
+      "@scope/[email protected]": {}
     },
     "npm": {
       "@example/orphan": {
@@ -236,7 +237,8 @@
         "dependencies": [
           "jsr:@scope/package_a"
         ]
-      }
+      },
+      "@scope/[email protected]": {}
     },
     "npm": {
       "@example/orphan": {
@@ -422,6 +424,9 @@
     "specifiers": {
       "jsr:@scope/package_orphan": "jsr:@scope/[email protected]"
     },
+    "jsr": {
+      "@scope/[email protected]": {}
+    },
     "npm": {
       "@example/orphan": {
         "integrity": "sha512-vq24Bq3ym5HEQm2NKCr3yXDwjc7vTsEThRDnkp2DK9p1uqLR+DHurm/NOTo0KG7HYHU7eppKZj3MyqYuMBf62g==",

diff --git a/tests/specs/config_changes/RemovedConfigRemoves.txt b/tests/specs/config_changes/RemovedConfigRemoves.txt
@@ -206,6 +206,9 @@
     "specifiers": {
       "jsr:@scope/package_orphan": "jsr:@scope/[email protected]"
     },
+    "jsr": {
+      "@scope/[email protected]": {}
+    },
     "npm": {
       "@example/orphan": {
         "integrity": "sha512-vq24Bq3ym5HEQm2NKCr3yXDwjc7vTsEThRDnkp2DK9p1uqLR+DHurm/NOTo0KG7HYHU7eppKZj3MyqYuMBf62g==",

diff --git a/tests/specs/config_changes/RemovedDenoJsonRemoves.txt b/tests/specs/config_changes/RemovedDenoJsonRemoves.txt
@@ -212,6 +212,9 @@
       "jsr:@scope/package_orphan": "jsr:@scope/[email protected]",
       "npm:ts-morph": "npm:[email protected]"
     },
+    "jsr": {
+      "@scope/[email protected]": {}
+    },
     "npm": {
       "@example/orphan": {
         "integrity": "sha512-vq24Bq3ym5HEQm2NKCr3yXDwjc7vTsEThRDnkp2DK9p1uqLR+DHurm/NOTo0KG7HYHU7eppKZj3MyqYuMBf62g==",

diff --git a/tests/specs/config_changes/RemovedPackageJsonRemoves.txt b/tests/specs/config_changes/RemovedPackageJsonRemoves.txt
@@ -227,7 +227,8 @@
         "dependencies": [
           "jsr:@scope/package_a"
         ]
-      }
+      },
+      "@scope/[email protected]": {}
     },
     "npm": {
       "@example/orphan": {

diff --git a/tests/specs/config_changes/RemovingOakThenDax.txt b/tests/specs/config_changes/RemovingOakThenDax.txt
@@ -407,6 +407,10 @@
           "jsr:@std/[email protected]"
         ]
       },
+      "@dsherret/[email protected]": {},
+      "@std/[email protected]": {},
+      "@std/[email protected]": {},
+      "@std/[email protected]": {},
       "@std/[email protected]": {
         "dependencies": [
           "jsr:@std/assert@^0.210.0",
@@ -431,6 +435,8 @@
           "jsr:@std/io@^0.210.0"
         ]
       },
+      "@zome_unreferenced/[email protected]": {},
+      "@zome_unreferenced/[email protected]": {},
       "jsr:@zome_unreferenced/[email protected]": {
         "dependencies": [
           "jsr:@zome_unreferenced/package_b"
@@ -653,6 +659,8 @@
       "jsr:@zome_unreferenced/package_b": "jsr:@zome_unreferenced/[email protected]"
     },
     "jsr": {
+      "@zome_unreferenced/[email protected]": {},
+      "@zome_unreferenced/[email protected]": {},
       "jsr:@zome_unreferenced/[email protected]": {
         "dependencies": [
           "jsr:@zome_unreferenced/package_b"

diff --git a/tests/specs/config_changes/RemovingWorkspaceDep.txt b/tests/specs/config_changes/RemovingWorkspaceDep.txt
@@ -419,6 +419,10 @@
           "jsr:@std/[email protected]"
         ]
       },
+      "@dsherret/[email protected]": {},
+      "@std/[email protected]": {},
+      "@std/[email protected]": {},
+      "@std/[email protected]": {},
       "@std/[email protected]": {
         "dependencies": [
           "jsr:@std/assert@^0.210.0",
@@ -443,6 +447,8 @@
           "jsr:@std/io@^0.210.0"
         ]
       },
+      "@zome_unreferenced/[email protected]": {},
+      "@zome_unreferenced/[email protected]": {},
       "jsr:@zome_unreferenced/[email protected]": {
         "dependencies": [
           "jsr:@zome_unreferenced/package_b"

diff --git a/tests/specs/config_changes/RemovingWorkspaceMember.txt b/tests/specs/config_changes/RemovingWorkspaceMember.txt
@@ -413,6 +413,10 @@
           "jsr:@std/[email protected]"
         ]
       },
+      "@dsherret/[email protected]": {},
+      "@std/[email protected]": {},
+      "@std/[email protected]": {},
+      "@std/[email protected]": {},
       "@std/[email protected]": {
         "dependencies": [
           "jsr:@std/assert@^0.210.0",
@@ -437,6 +441,8 @@
           "jsr:@std/io@^0.210.0"
         ]
       },
+      "@zome_unreferenced/[email protected]": {},
+      "@zome_unreferenced/[email protected]": {},
       "jsr:@zome_unreferenced/[email protected]": {
         "dependencies": [
           "jsr:@zome_unreferenced/package_b"

diff --git a/tests/specs/config_changes/ShiftingDepsDifferentWorkspaceMember.txt b/tests/specs/config_changes/ShiftingDepsDifferentWorkspaceMember.txt
@@ -11,7 +11,8 @@
         "dependencies": [
           "jsr:@dsherret/[email protected]"
         ]
-      }
+      },
+      "@dsherret/[email protected]": {}
     }
   },
   "remote": {
@@ -49,7 +50,8 @@
         "dependencies": [
           "jsr:@dsherret/[email protected]"
         ]
-      }
+      },
+      "@dsherret/[email protected]": {}
     }
   },
   "remote": {
@@ -91,7 +93,8 @@
         "dependencies": [
           "jsr:@dsherret/[email protected]"
         ]
-      }
+      },
+      "@dsherret/[email protected]": {}
     }
   },
   "remote": {