fix(add): Handle packages without root exports (#25102)

Fixes #24607.

This PR makes the logic that caches top level dependencies (things
present in import map) smarter, so we handle JSR dependencies without
root exports.

cli/module_loader.rs

@@ -18,7 +18,6 @@ use crate::cache::CodeCache;
 use crate::cache::FastInsecureHasher;
 use crate::cache::ParsedSourceCache;
 use crate::emit::Emitter;
-use crate::factory::CliFactory;
 use crate::graph_container::MainModuleGraphContainer;
 use crate::graph_container::ModuleGraphContainer;
 use crate::graph_container::ModuleGraphUpdatePermit;
@@ -70,54 +69,6 @@ use deno_runtime::deno_permissions::PermissionsContainer;
 use deno_semver::npm::NpmPackageReqReference;
 use node_resolver::NodeResolutionMode;
 
-pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> {
-  let npm_resolver = factory.npm_resolver().await?;
-  let cli_options = factory.cli_options()?;
-  if let Some(npm_resolver) = npm_resolver.as_managed() {
-    if !npm_resolver.ensure_top_level_package_json_install().await? {
-      if let Some(lockfile) = cli_options.maybe_lockfile() {
-        lockfile.error_if_changed()?;
-      }
-
-      npm_resolver.cache_packages().await?;
-    }
-  }
-  // cache as many entries in the import map as we can
-  let resolver = factory.workspace_resolver().await?;
-  if let Some(import_map) = resolver.maybe_import_map() {
-    let roots = import_map
-      .imports()
-      .entries()
-      .filter_map(|entry| {
-        if entry.key.ends_with('/') {
-          None
-        } else {
-          entry.value.cloned()
-        }
-      })
-      .collect::<Vec<_>>();
-    let mut graph_permit = factory
-      .main_module_graph_container()
-      .await?
-      .acquire_update_permit()
-      .await;
-    let graph = graph_permit.graph_mut();
-    factory
-      .module_load_preparer()
-      .await?
-      .prepare_module_load(
-        graph,
-        &roots,
-        false,
-        factory.cli_options()?.ts_type_lib_window(),
-        deno_runtime::deno_permissions::PermissionsContainer::allow_all(),
-      )
-      .await?;
-  }
-
-  Ok(())
-}
-
 pub struct ModuleLoadPreparer {
   options: Arc<CliOptions>,
   lockfile: Option<Arc<CliLockfile>>,

cli/tools/installer.rs

@@ -275,7 +275,7 @@ async fn install_local(
   }
 
   let factory = CliFactory::from_flags(flags);
-  crate::module_loader::load_top_level_deps(&factory).await?;
+  crate::tools::registry::cache_top_level_deps(&factory, None).await?;
 
   if let Some(lockfile) = factory.cli_options()?.maybe_lockfile() {
     lockfile.write_if_changed()?;

cli/tools/registry/mod.rs

@@ -52,6 +52,7 @@ use crate::util::display::human_size;
 
 mod api;
 mod auth;
+
 mod diagnostics;
 mod graph;
 mod paths;
@@ -64,6 +65,7 @@ mod unfurl;
 use auth::get_auth_method;
 use auth::AuthMethod;
 pub use pm::add;
+pub use pm::cache_top_level_deps;
 pub use pm::remove;
 pub use pm::AddCommandName;
 use publish_order::PublishOrderGraph;

cli/tools/registry/pm.rs

@@ -1,5 +1,9 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
+mod cache_deps;
+
+pub use cache_deps::cache_top_level_deps;
+
 use std::borrow::Cow;
 use std::path::Path;
 use std::path::PathBuf;
@@ -236,13 +240,16 @@ pub async fn add(
 
   let package_futures = package_reqs
     .into_iter()
-    .map(move |package_req| {
+    .map({
+      let jsr_resolver = jsr_resolver.clone();
+      move |package_req| {
         find_package_and_select_version_for_req(
           jsr_resolver.clone(),
           npm_resolver.clone(),
           package_req,
         )
         .boxed_local()
+      }
     })
     .collect::<Vec<_>>();
 
@@ -350,7 +357,7 @@ pub async fn add(
   // make a new CliFactory to pick up the updated config file
   let cli_factory = CliFactory::from_flags(flags);
   // cache deps
-  crate::module_loader::load_top_level_deps(&cli_factory).await?;
+  cache_deps::cache_top_level_deps(&cli_factory, Some(jsr_resolver)).await?;
 
   Ok(())
 }
@@ -597,7 +604,7 @@ pub async fn remove(
     // Update deno.lock
     node_resolver::PackageJsonThreadLocalCache::clear();
     let cli_factory = CliFactory::from_flags(flags);
-    crate::module_loader::load_top_level_deps(&cli_factory).await?;
+    cache_deps::cache_top_level_deps(&cli_factory, None).await?;
   }
 
   Ok(())

cli/tools/registry/pm/cache_deps.rs

@@ -0,0 +1,115 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+
+use std::sync::Arc;
+
+use crate::factory::CliFactory;
+use crate::graph_container::ModuleGraphContainer;
+use crate::graph_container::ModuleGraphUpdatePermit;
+use deno_core::error::AnyError;
+use deno_core::futures::stream::FuturesUnordered;
+use deno_core::futures::StreamExt;
+use deno_semver::package::PackageReq;
+
+pub async fn cache_top_level_deps(
+  factory: &CliFactory,
+  jsr_resolver: Option<Arc<crate::jsr::JsrFetchResolver>>,
+) -> Result<(), AnyError> {
+  let npm_resolver = factory.npm_resolver().await?;
+  let cli_options = factory.cli_options()?;
+  if let Some(npm_resolver) = npm_resolver.as_managed() {
+    if !npm_resolver.ensure_top_level_package_json_install().await? {
+      if let Some(lockfile) = cli_options.maybe_lockfile() {
+        lockfile.error_if_changed()?;
+      }
+
+      npm_resolver.cache_packages().await?;
+    }
+  }
+  // cache as many entries in the import map as we can
+  let resolver = factory.workspace_resolver().await?;
+  if let Some(import_map) = resolver.maybe_import_map() {
+    let jsr_resolver = if let Some(resolver) = jsr_resolver {
+      resolver
+    } else {
+      Arc::new(crate::jsr::JsrFetchResolver::new(
+        factory.file_fetcher()?.clone(),
+      ))
+    };
+
+    let mut roots = Vec::new();
+
+    let mut info_futures = FuturesUnordered::new();
+
+    let mut seen_reqs = std::collections::HashSet::new();
+
+    for entry in import_map.imports().entries() {
+      let Some(specifier) = entry.value else {
+        continue;
+      };
+
+      match specifier.scheme() {
+        "jsr" => {
+          let specifier_str = specifier.as_str();
+          let specifier_str =
+            specifier_str.strip_prefix("jsr:").unwrap_or(specifier_str);
+          if let Ok(req) = PackageReq::from_str(specifier_str) {
+            if !seen_reqs.insert(req.clone()) {
+              continue;
+            }
+            let jsr_resolver = jsr_resolver.clone();
+            info_futures.push(async move {
+              if let Some(nv) = jsr_resolver.req_to_nv(&req).await {
+                if let Some(info) = jsr_resolver.package_version_info(&nv).await
+                {
+                  return Some((specifier.clone(), info));
+                }
+              }
+              None
+            });
+          }
+        }
+        "npm" => roots.push(specifier.clone()),
+        _ => {
+          if entry.key.ends_with('/') && specifier.as_str().ends_with('/') {
+            continue;
+          }
+          roots.push(specifier.clone());
+        }
+      }
+    }
+
+    while let Some(info_future) = info_futures.next().await {
+      if let Some((specifier, info)) = info_future {
+        if info.export(".").is_some() {
+          roots.push(specifier.clone());
+          continue;
+        }
+        let exports = info.exports();
+        for (k, _) in exports {
+          if let Ok(spec) = specifier.join(k) {
+            roots.push(spec);
+          }
+        }
+      }
+    }
+    let mut graph_permit = factory
+      .main_module_graph_container()
+      .await?
+      .acquire_update_permit()
+      .await;
+    let graph = graph_permit.graph_mut();
+    factory
+      .module_load_preparer()
+      .await?
+      .prepare_module_load(
+        graph,
+        &roots,
+        false,
+        deno_config::deno_json::TsTypeLib::DenoWorker,
+        deno_runtime::deno_permissions::PermissionsContainer::allow_all(),
+      )
+      .await?;
+  }
+
+  Ok(())
+}

tests/registry/jsr/@std/testing/1.0.0/bdd.ts

@@ -0,0 +1,2 @@
+export function it(_name: string, _fn: () => void) {
+}

tests/registry/jsr/@std/testing/1.0.0/types.ts

@@ -0,0 +1 @@
+export type AssertType<A, B> = A extends B ? true : never;

tests/registry/jsr/@std/testing/1.0.0_meta.json

@@ -0,0 +1,6 @@
+{
+    "exports": {
+        "./bdd": "./bdd.ts",
+        "./types": "./types.ts"
+    }
+}

tests/registry/jsr/@std/testing/meta.json

@@ -0,0 +1,8 @@
+{
+  "scope": "std",
+  "name": "path",
+  "latest": "1.0.0",
+  "versions": {
+    "1.0.0": {}
+  }
+}

tests/specs/add/no_root_export/__test__.jsonc

@@ -0,0 +1,9 @@
+{
+  "tempDir": true,
+  "steps": [
+    {
+      "args": "add @std/testing",
+      "output": "add.out"
+    }
+  ]
+}

tests/specs/add/no_root_export/add.out

@@ -0,0 +1,5 @@
+Add jsr:@std/testing@1.0.0
+[UNORDERED_START]
+Download http://127.0.0.1:4250/@std/testing/1.0.0/bdd.ts
+Download http://127.0.0.1:4250/@std/testing/1.0.0/types.ts
+[UNORDERED_END]

tests/specs/add/no_root_export/main.ts

@@ -0,0 +1,3 @@
+import { it } from "@std/testing/bdd";
+
+const _it = it;

tests/specs/install/future_install_local_deno/deno.json

@@ -3,6 +3,8 @@
     "@std/fs/": "https://deno.land/std@0.224.0/fs/",
     "@denotest/esm-basic": "npm:@denotest/esm-basic@^1.0.0",
     "@denotest/add": "jsr:@denotest/add",
-    "test-http": "http://localhost:4545/v1/extensionless"
+    "test-http": "http://localhost:4545/v1/extensionless",
+    "@std/testing": "jsr:@std/testing",
+    "@std/testing/": "jsr:/@std/testing/"
   }
 }

tests/specs/install/future_install_local_deno/deno.lock.out

@@ -2,11 +2,15 @@
   "version": "4",
   "specifiers": {
     "jsr:@denotest/add": "jsr:@denotest/add@1.0.0",
+    "jsr:@std/testing": "jsr:@std/testing@1.0.0",
     "npm:@denotest/esm-basic@^1.0.0": "npm:@denotest/esm-basic@1.0.0"
   },
   "jsr": {
     "@denotest/add@1.0.0": {
       "integrity": "[WILDCARD]"
+    },
+    "@std/testing@1.0.0": {
+      "integrity": "[WILDCARD]"
     }
   },
   "npm": {
@@ -14,10 +18,16 @@
       "integrity": "[WILDCARD]"
     }
   },
-  "remote": [WILDCARD],
+  "remote": {
+    "http://localhost:4545/subdir/mod1.ts": "[WILDCARD]",
+    "http://localhost:4545/subdir/print_hello.ts": "[WILDCARD]",
+    "http://localhost:4545/subdir/subdir2/mod2.ts": "[WILDCARD]",
+    "http://localhost:4545/v1/extensionless": "[WILDCARD]"
+  },
   "workspace": {
     "dependencies": [
       "jsr:@denotest/add",
+      "jsr:@std/testing",
       "npm:@denotest/esm-basic@^1.0.0"
     ]
   }

tests/specs/install/future_install_local_deno/install.out

@@ -1,4 +1,6 @@
 [UNORDERED_START]
+Download http://127.0.0.1:4250/@std/testing/meta.json
+Download http://127.0.0.1:4250/@std/testing/1.0.0_meta.json
 Download http://localhost:4545/v1/extensionless
 Download http://localhost:4545/subdir/mod1.ts
 Download http://localhost:4545/subdir/subdir2/mod2.ts
@@ -8,4 +10,6 @@ Download http://127.0.0.1:4250/@denotest/add/1.0.0_meta.json
 Download http://127.0.0.1:4250/@denotest/add/1.0.0/mod.ts
 Download http://localhost:4260/@denotest/esm-basic
 Download http://localhost:4260/@denotest/esm-basic/1.0.0.tgz
+Download http://127.0.0.1:4250/@std/testing/1.0.0/bdd.ts
+Download http://127.0.0.1:4250/@std/testing/1.0.0/types.ts
 [UNORDERED_END]

tests/specs/remove/basic/add.out

@@ -1,9 +1,9 @@
 Created deno.json configuration file.
-Add jsr:@std/assert@1.0.0
-Add jsr:@std/http@1.0.0
 [UNORDERED_START]
-Download http://127.0.0.1:4250/@std/http/1.0.0_meta.json
-Download http://127.0.0.1:4250/@std/assert/1.0.0_meta.json
+Add jsr:@std/http@1.0.0
+Add jsr:@std/assert@1.0.0
+[UNORDERED_END]
+[UNORDERED_START]
 Download http://127.0.0.1:4250/@std/http/1.0.0/mod.ts
 Download http://127.0.0.1:4250/@std/assert/1.0.0/mod.ts
 Download http://127.0.0.1:4250/@std/assert/1.0.0/assert_equals.ts