fix: improve handling of linked jsr packages in lockfile (#30289)

Cargo.lock

@@ -2296,9 +2296,9 @@ dependencies = [
 
 [[package]]
 name = "deno_lockfile"
-version = "0.31.1"
+version = "0.31.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c2051c2aecfd7f287c2afdcee50d9e2ac1b8f1873bae2d3934d9a2e2762d7c66"
+checksum = "5afb20b44a44842178f41fa3b713c2b1516062590b5cca2ad792f2d6786bc74f"
 dependencies = [
  "async-trait",
  "deno_semver",
@@ -2696,6 +2696,7 @@ dependencies = [
  "async-trait",
  "base32",
  "boxed_error",
+ "capacity_builder",
  "dashmap",
  "deno_ast",
  "deno_cache_dir",

Cargo.toml

@@ -67,7 +67,7 @@ deno_doc = "=0.181.0"
 deno_error = "=0.7.0"
 deno_graph = { version = "=0.98.0", default-features = false }
 deno_lint = "=0.77.0"
-deno_lockfile = "=0.31.1"
+deno_lockfile = "=0.31.2"
 deno_media_type = { version = "=0.2.9", features = ["module_specifier"] }
 deno_native_certs = "0.3.0"
 deno_npm = "=0.36.0"

cli/lsp/jsr.rs

@@ -59,7 +59,7 @@ impl JsrCacheResolver {
     let info_by_nv = DashMap::new();
     let info_by_name = DashMap::new();
     let mut workspace_packages_by_name = HashMap::new();
-    for jsr_package in workspace_resolver.jsr_packages() {
+    for jsr_package in workspace_resolver.jsr_packages().iter() {
       let exports = deno_core::serde_json::json!(&jsr_package.exports);
       let version_info = Arc::new(JsrPackageVersionInfo {
         exports: exports.clone(),

cli/standalone/binary.rs

@@ -752,6 +752,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
         jsr_pkgs: self
           .workspace_resolver
           .jsr_packages()
+          .iter()
           .map(|pkg| SerializedResolverWorkspaceJsrPackage {
             relative_base: root_dir_url.specifier_key(&pkg.base).into_owned(),
             name: pkg.name.clone(),

cli/tools/pm/cache_deps.rs

@@ -70,6 +70,7 @@ pub async fn cache_top_level_deps(
           .and_then(|name| Some((name, pkg_json.version.as_deref()?)))
       })
       .collect::<HashMap<_, _>>();
+    let workspace_jsr_packages = resolver.jsr_packages();
 
     for entry in import_map.imports().entries().chain(
       import_map
@@ -83,49 +84,58 @@ pub async fn cache_top_level_deps(
       match specifier.scheme() {
         "jsr" => {
           let specifier_str = specifier.as_str();
-          if let Ok(req) = JsrPackageReqReference::from_str(specifier_str) {
+          let Ok(req_ref) = JsrPackageReqReference::from_str(specifier_str)
-            if let Some(sub_path) = req.sub_path() {
+          else {
-              if sub_path.ends_with('/') {
+            continue;
-                continue;
+          };
-              }
+          if workspace_jsr_packages
-              roots.push(specifier.clone());
+            .iter()
-              continue;
+            .any(|pkg| pkg.matches_req(req_ref.req()))
-            }
+          {
-            if !seen_reqs.insert(req.req().clone()) {
+            // do not install a workspace jsr package
-              continue;
+            continue;
-            }
-            let resolved_req = graph.packages.mappings().get(req.req());
-            let resolved_req = resolved_req.and_then(|nv| {
-              // the version might end up being upgraded to a newer version that's already in
-              // the graph (due to a reverted change), in which case our exports could end up
-              // being wrong. to avoid that, see if there's a newer version that matches the version
-              // req.
-              let versions =
-                graph.packages.versions_by_name(&req.req().name)?;
-              let mut best = nv;
-              for version in versions {
-                if version.version > best.version
-                  && req.req().version_req.matches(&version.version)
-                {
-                  best = version;
-                }
-              }
-              Some(best)
-            });
-
-            let jsr_resolver = jsr_resolver.clone();
-            info_futures.push(async move {
-              let nv = if let Some(req) = resolved_req {
-                Cow::Borrowed(req)
-              } else {
-                Cow::Owned(jsr_resolver.req_to_nv(req.req()).await?)
-              };
-              if let Some(info) = jsr_resolver.package_version_info(&nv).await {
-                return Some((specifier.clone(), info));
-              }
-              None
-            });
           }
+          if let Some(sub_path) = req_ref.sub_path() {
+            if sub_path.ends_with('/') {
+              continue;
+            }
+            roots.push(specifier.clone());
+            continue;
+          }
+          if !seen_reqs.insert(req_ref.req().clone()) {
+            continue;
+          }
+          let resolved_req = graph.packages.mappings().get(req_ref.req());
+          let resolved_req = resolved_req.and_then(|nv| {
+            // the version might end up being upgraded to a newer version that's already in
+            // the graph (due to a reverted change), in which case our exports could end up
+            // being wrong. to avoid that, see if there's a newer version that matches the version
+            // req.
+            let versions =
+              graph.packages.versions_by_name(&req_ref.req().name)?;
+            let mut best = nv;
+            for version in versions {
+              if version.version > best.version
+                && req_ref.req().version_req.matches(&version.version)
+              {
+                best = version;
+              }
+            }
+            Some(best)
+          });
+
+          let jsr_resolver = jsr_resolver.clone();
+          info_futures.push(async move {
+            let nv = if let Some(req) = resolved_req {
+              Cow::Borrowed(req)
+            } else {
+              Cow::Owned(jsr_resolver.req_to_nv(req_ref.req()).await?)
+            };
+            if let Some(info) = jsr_resolver.package_version_info(&nv).await {
+              return Some((specifier.clone(), info));
+            }
+            None
+          });
         }
         "npm" => {
           let Ok(req_ref) =

libs/config/workspace/mod.rs

@@ -79,6 +79,17 @@ pub struct ResolverWorkspaceJsrPackage {
   pub is_link: bool,
 }
 
+impl ResolverWorkspaceJsrPackage {
+  pub fn matches_req(&self, req: &PackageReq) -> bool {
+    self.name == req.name
+      && self
+        .version
+        .as_ref()
+        .map(|v| req.version_req.matches(v))
+        .unwrap_or(true)
+  }
+}
+
 #[derive(Debug, Clone)]
 pub struct JsrPackageConfig {
   /// The package name.

libs/resolver/Cargo.toml

@@ -24,6 +24,7 @@ async-once-cell.workspace = true
 async-trait.workspace = true
 base32.workspace = true
 boxed_error.workspace = true
+capacity_builder.workspace = true
 dashmap = { workspace = true, optional = true }
 deno_ast = { workspace = true, features = ["cjs"], optional = true }
 deno_cache_dir.workspace = true

libs/resolver/lockfile.rs

@@ -5,6 +5,7 @@ use std::path::PathBuf;
 
 use anyhow::Context;
 use anyhow::Error as AnyError;
+use capacity_builder::StringBuilder;
 use deno_config::workspace::Workspace;
 use deno_error::JsErrorBox;
 use deno_lockfile::Lockfile;
@@ -394,11 +395,16 @@ impl<TSys: LockfileSys> LockfileLock<TSys> {
               .unwrap_or_default()
           }
 
-          let key = format!(
+          let name = pkg_json.name.as_ref()?;
-            "npm:{}@{}",
+          let key = StringBuilder::<String>::build(|builder| {
-            pkg_json.name.as_ref()?,
+            builder.append("npm:");
-            pkg_json.version.as_ref()?
+            builder.append(name);
-          );
+            if let Some(version) = &pkg_json.version {
+              builder.append('@');
+              builder.append(version);
+            }
+          })
+          .unwrap();
           // anything that affects npm resolution should go here in order to bust
           // the npm resolution when it changes
           let value = deno_lockfile::LockfileLinkContent {
@@ -414,6 +420,24 @@ impl<TSys: LockfileSys> LockfileLock<TSys> {
           };
           Some((key, value))
         })
+        .chain(workspace.link_deno_jsons().filter_map(|deno_json| {
+          let name = deno_json.json.name.as_ref()?;
+          let key = StringBuilder::<String>::build(|builder| {
+            builder.append("jsr:");
+            builder.append(name);
+            if let Some(version) = &deno_json.json.version {
+              builder.append('@');
+              builder.append(version);
+            }
+          })
+          .unwrap();
+          let value = deno_lockfile::LockfileLinkContent {
+            dependencies: deno_json.dependencies(),
+            peer_dependencies: Default::default(),
+            peer_dependencies_meta: Default::default(),
+          };
+          Some((key, value))
+        }))
         .collect(),
     };
     lockfile.set_workspace_config(deno_lockfile::SetWorkspaceConfigOptions {

libs/resolver/workspace.rs

@@ -1120,6 +1120,7 @@ impl<TSys: FsMetadata + FsRead> WorkspaceResolver<TSys> {
       }),
       jsr_pkgs: self
         .jsr_packages()
+        .iter()
         .map(|pkg| SerializedResolverWorkspaceJsrPackage {
           relative_base: root_dir_url.make_relative_if_descendant(&pkg.base),
           name: Cow::Borrowed(&pkg.name),
@@ -1253,10 +1254,8 @@ impl<TSys: FsMetadata + FsRead> WorkspaceResolver<TSys> {
     self.pkg_jsons.values().map(|c| &c.pkg_json)
   }
 
-  pub fn jsr_packages(
+  pub fn jsr_packages(&self) -> &[ResolverWorkspaceJsrPackage] {
-    &self,
+    &self.jsr_pkgs
-  ) -> impl Iterator<Item = &ResolverWorkspaceJsrPackage> {
-    self.jsr_pkgs.iter()
   }
 
   pub fn diagnostics(&self) -> Vec<WorkspaceResolverDiagnostic<'_>> {

tests/specs/jsr/link_lockfile_changes/__test__.jsonc

@@ -0,0 +1,28 @@
+{
+  "tempDir": true,
+  "steps": [{
+    "args": "install",
+    "output": "[WILDCARD]"
+  }, {
+    "args": [
+      "eval",
+      "console.log(Deno.readTextFileSync('deno.lock').trim()); Deno.renameSync('deno1.json', 'deno.json')"
+    ],
+    "output": "lockfile1.out"
+  }, {
+    "args": "install",
+    "output": "[WILDCARD]"
+  }, {
+    "args": [
+      "eval",
+      "console.log(Deno.readTextFileSync('deno.lock').trim()); Deno.renameSync('deno2.json', 'deno.json')"
+    ],
+    "output": "lockfile2.out"
+  }, {
+    "args": "install",
+    "output": "[WILDCARD]"
+  }, {
+    "args": ["eval", "console.log(Deno.readTextFileSync('deno.lock').trim());"],
+    "output": "lockfile1.out"
+  }]
+}

tests/specs/jsr/link_lockfile_changes/add/deno.json

@@ -0,0 +1,4 @@
+{
+  "name": "@denotest/add",
+  "exports": "./mod.ts"
+}

tests/specs/jsr/link_lockfile_changes/add/mod.ts

@@ -0,0 +1,3 @@
+export function add(a: number, b: number): number {
+  return a + b;
+}

tests/specs/jsr/link_lockfile_changes/deno.json

@@ -0,0 +1,5 @@
+{
+  "imports": {
+    "@denotest/add": "jsr:@denotest/add"
+  }
+}

tests/specs/jsr/link_lockfile_changes/deno1.json

@@ -0,0 +1,8 @@
+{
+  "imports": {
+    "@denotest/add": "jsr:@denotest/add"
+  },
+  "links": [
+    "./add"
+  ]
+}

tests/specs/jsr/link_lockfile_changes/deno2.json

@@ -0,0 +1,5 @@
+{
+  "imports": {
+    "@denotest/add": "jsr:@denotest/add"
+  }
+}

tests/specs/jsr/link_lockfile_changes/lockfile1.out

@@ -0,0 +1,16 @@
+{
+  "version": "5",
+  "specifiers": {
+    "jsr:@denotest/add@*": "1.0.0"
+  },
+  "jsr": {
+    "@denotest/add@1.0.0": {
+      "integrity": "[WILDLINE]"
+    }
+  },
+  "workspace": {
+    "dependencies": [
+      "jsr:@denotest/add@*"
+    ]
+  }
+}

tests/specs/jsr/link_lockfile_changes/lockfile2.out

@@ -0,0 +1,11 @@
+{
+  "version": "5",
+  "workspace": {
+    "dependencies": [
+      "jsr:@denotest/add@*"
+    ],
+    "links": {
+      "jsr:@denotest/add": {}
+    }
+  }
+}