feat(unstable/pm): support npm packages in 'deno add' (#22715)

cli/args/mod.rs

@@ -75,7 +75,7 @@ use deno_config::FmtConfig;
 use deno_config::LintConfig;
 use deno_config::TestConfig;
 
-pub fn npm_registry_default_url() -> &'static Url {
+pub fn npm_registry_url() -> &'static Url {
   static NPM_REGISTRY_DEFAULT_URL: Lazy<Url> = Lazy::new(|| {
     let env_var_name = "NPM_CONFIG_REGISTRY";
     if let Ok(registry_url) = std::env::var(env_var_name) {

cli/factory.rs

@@ -443,7 +443,7 @@ impl CliFactory {
                 self.package_json_deps_provider().clone(),
               ),
             npm_system_info: self.options.npm_system_info(),
-            npm_registry_url: crate::args::npm_registry_default_url().to_owned(),
+            npm_registry_url: crate::args::npm_registry_url().to_owned(),
           })
         }).await
       }.boxed_local())

cli/jsr.rs

@@ -226,7 +226,7 @@ impl JsrFetchResolver {
     if let Some(info) = self.info_by_name.get(name) {
       return info.value().clone();
     }
-    let read_cached_package_info = || async {
+    let fetch_package_info = || async {
       let meta_url = jsr_url().join(&format!("{}/meta.json", name)).ok()?;
       let file = self
         .file_fetcher
@@ -235,7 +235,7 @@ impl JsrFetchResolver {
         .ok()?;
       serde_json::from_slice::<JsrPackageInfo>(&file.source).ok()
     };
-    let info = read_cached_package_info().await.map(Arc::new);
+    let info = fetch_package_info().await.map(Arc::new);
     self.info_by_name.insert(name.to_string(), info.clone());
     info
   }
@@ -247,7 +247,7 @@ impl JsrFetchResolver {
     if let Some(info) = self.info_by_nv.get(nv) {
       return info.value().clone();
     }
-    let read_cached_package_version_info = || async {
+    let fetch_package_version_info = || async {
       let meta_url = jsr_url()
         .join(&format!("{}/{}_meta.json", &nv.name, &nv.version))
         .ok()?;
@@ -258,7 +258,7 @@ impl JsrFetchResolver {
         .ok()?;
       partial_jsr_package_version_info_from_slice(&file.source).ok()
     };
-    let info = read_cached_package_version_info().await.map(Arc::new);
+    let info = fetch_package_version_info().await.map(Arc::new);
     self.info_by_nv.insert(nv.clone(), info.clone());
     info
   }

cli/lsp/jsr.rs

@@ -15,20 +15,18 @@ use std::sync::Arc;
 
 use super::search::PackageSearchApi;
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct CliJsrSearchApi {
   file_fetcher: FileFetcher,
-  /// We only store this here so the completion system has access to a resolver
+  resolver: JsrFetchResolver,
-  /// that always uses the global cache.
+  search_cache: DashMap<String, Arc<Vec<String>>>,
-  resolver: Arc<JsrFetchResolver>,
+  versions_cache: DashMap<String, Arc<Vec<Version>>>,
-  search_cache: Arc<DashMap<String, Arc<Vec<String>>>>,
+  exports_cache: DashMap<PackageNv, Arc<Vec<String>>>,
-  versions_cache: Arc<DashMap<String, Arc<Vec<Version>>>>,
-  exports_cache: Arc<DashMap<PackageNv, Arc<Vec<String>>>>,
 }
 
 impl CliJsrSearchApi {
   pub fn new(file_fetcher: FileFetcher) -> Self {
-    let resolver = Arc::new(JsrFetchResolver::new(file_fetcher.clone()));
+    let resolver = JsrFetchResolver::new(file_fetcher.clone());
     Self {
       file_fetcher,
       resolver,
@@ -38,7 +36,7 @@ impl CliJsrSearchApi {
     }
   }
 
-  pub fn get_resolver(&self) -> &Arc<JsrFetchResolver> {
+  pub fn get_resolver(&self) -> &JsrFetchResolver {
     &self.resolver
   }
 }
@@ -49,12 +47,7 @@ impl PackageSearchApi for CliJsrSearchApi {
     if let Some(names) = self.search_cache.get(query) {
       return Ok(names.clone());
     }
-    let mut search_url = jsr_api_url().clone();
+    let mut search_url = jsr_api_url().join("packages")?;
-    search_url
-      .path_segments_mut()
-      .map_err(|_| anyhow!("Custom jsr URL cannot be a base."))?
-      .pop_if_empty()
-      .push("packages");
     search_url.query_pairs_mut().append_pair("query", query);
     let file = self
       .file_fetcher

cli/lsp/language_server.rs

@@ -875,9 +875,8 @@ impl Inner {
       None,
     );
     deps_file_fetcher.set_download_log_level(super::logging::lsp_log_level());
-    self.jsr_search_api = CliJsrSearchApi::new(deps_file_fetcher);
+    self.jsr_search_api = CliJsrSearchApi::new(deps_file_fetcher.clone());
-    self.npm.search_api =
+    self.npm.search_api = CliNpmSearchApi::new(deps_file_fetcher);
-      CliNpmSearchApi::new(self.module_registries.file_fetcher.clone());
     let maybe_local_cache =
       self.config.maybe_vendor_dir_path().map(|local_path| {
         Arc::new(LocalLspHttpCache::new(local_path, global_cache.clone()))
@@ -1182,7 +1181,7 @@ async fn create_npm_resolver(
       // do not install while resolving in the lsp—leave that to the cache command
       package_json_installer:
         CliNpmResolverManagedPackageJsonInstallerOption::NoInstall,
-      npm_registry_url: crate::args::npm_registry_default_url().to_owned(),
+      npm_registry_url: crate::args::npm_registry_url().to_owned(),
       npm_system_info: NpmSystemInfo::default(),
     })
   })

cli/lsp/npm.rs

@@ -4,29 +4,32 @@ use dashmap::DashMap;
 use deno_core::anyhow::anyhow;
 use deno_core::error::AnyError;
 use deno_core::serde_json;
-use deno_npm::registry::NpmPackageInfo;
 use deno_runtime::permissions::PermissionsContainer;
 use deno_semver::package::PackageNv;
 use deno_semver::Version;
 use serde::Deserialize;
 use std::sync::Arc;
 
-use crate::args::npm_registry_default_url;
+use crate::args::npm_registry_url;
 use crate::file_fetcher::FileFetcher;
+use crate::npm::NpmFetchResolver;
 
 use super::search::PackageSearchApi;
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct CliNpmSearchApi {
   file_fetcher: FileFetcher,
-  search_cache: Arc<DashMap<String, Arc<Vec<String>>>>,
+  resolver: NpmFetchResolver,
-  versions_cache: Arc<DashMap<String, Arc<Vec<Version>>>>,
+  search_cache: DashMap<String, Arc<Vec<String>>>,
+  versions_cache: DashMap<String, Arc<Vec<Version>>>,
 }
 
 impl CliNpmSearchApi {
   pub fn new(file_fetcher: FileFetcher) -> Self {
+    let resolver = NpmFetchResolver::new(file_fetcher.clone());
     Self {
       file_fetcher,
+      resolver,
       search_cache: Default::default(),
       versions_cache: Default::default(),
     }
@@ -39,12 +42,7 @@ impl PackageSearchApi for CliNpmSearchApi {
     if let Some(names) = self.search_cache.get(query) {
       return Ok(names.clone());
     }
-    let mut search_url = npm_registry_default_url().clone();
+    let mut search_url = npm_registry_url().join("-/v1/search")?;
-    search_url
-      .path_segments_mut()
-      .map_err(|_| anyhow!("Custom npm registry URL cannot be a base."))?
-      .pop_if_empty()
-      .extend("-/v1/search".split('/'));
     search_url
       .query_pairs_mut()
       .append_pair("text", &format!("{} boost-exact:false", query));
@@ -62,18 +60,12 @@ impl PackageSearchApi for CliNpmSearchApi {
     if let Some(versions) = self.versions_cache.get(name) {
       return Ok(versions.clone());
     }
-    let mut info_url = npm_registry_default_url().clone();
+    let info = self
-    info_url
+      .resolver
-      .path_segments_mut()
+      .package_info(name)
-      .map_err(|_| anyhow!("Custom npm registry URL cannot be a base."))?
+      .await
-      .pop_if_empty()
+      .ok_or_else(|| anyhow!("npm package info not found: {}", name))?;
-      .push(name);
+    let mut versions = info.versions.keys().cloned().collect::<Vec<_>>();
-    let file = self
-      .file_fetcher
-      .fetch(&info_url, PermissionsContainer::allow_all())
-      .await?;
-    let info = serde_json::from_slice::<NpmPackageInfo>(&file.source)?;
-    let mut versions = info.versions.into_keys().collect::<Vec<_>>();
     versions.sort();
     versions.reverse();
     let versions = Arc::new(versions);

cli/npm/mod.rs

@@ -8,11 +8,19 @@ mod managed;
 use std::path::PathBuf;
 use std::sync::Arc;
 
+use dashmap::DashMap;
 use deno_ast::ModuleSpecifier;
 use deno_core::error::AnyError;
+use deno_core::serde_json;
+use deno_npm::registry::NpmPackageInfo;
 use deno_runtime::deno_node::NpmResolver;
+use deno_runtime::permissions::PermissionsContainer;
+use deno_semver::package::PackageNv;
 use deno_semver::package::PackageReq;
 
+use crate::args::npm_registry_url;
+use crate::file_fetcher::FileFetcher;
+
 pub use self::byonm::ByonmCliNpmResolver;
 pub use self::byonm::CliNpmResolverByonmCreateOptions;
 pub use self::cache_dir::NpmCacheDir;
@@ -87,3 +95,60 @@ pub trait CliNpmResolver: NpmResolver {
   /// or `None` if the state currently can't be determined.
   fn check_state_hash(&self) -> Option<u64>;
 }
+
+#[derive(Debug)]
+pub struct NpmFetchResolver {
+  nv_by_req: DashMap<PackageReq, Option<PackageNv>>,
+  info_by_name: DashMap<String, Option<Arc<NpmPackageInfo>>>,
+  file_fetcher: FileFetcher,
+}
+
+impl NpmFetchResolver {
+  pub fn new(file_fetcher: FileFetcher) -> Self {
+    Self {
+      nv_by_req: Default::default(),
+      info_by_name: Default::default(),
+      file_fetcher,
+    }
+  }
+
+  pub async fn req_to_nv(&self, req: &PackageReq) -> Option<PackageNv> {
+    if let Some(nv) = self.nv_by_req.get(req) {
+      return nv.value().clone();
+    }
+    let maybe_get_nv = || async {
+      let name = req.name.clone();
+      let package_info = self.package_info(&name).await?;
+      // Find the first matching version of the package which is cached.
+      let mut versions = package_info.versions.keys().collect::<Vec<_>>();
+      versions.sort();
+      let version = versions
+        .into_iter()
+        .rev()
+        .find(|v| req.version_req.tag().is_none() && req.version_req.matches(v))
+        .cloned()?;
+      Some(PackageNv { name, version })
+    };
+    let nv = maybe_get_nv().await;
+    self.nv_by_req.insert(req.clone(), nv.clone());
+    nv
+  }
+
+  pub async fn package_info(&self, name: &str) -> Option<Arc<NpmPackageInfo>> {
+    if let Some(info) = self.info_by_name.get(name) {
+      return info.value().clone();
+    }
+    let fetch_package_info = || async {
+      let info_url = npm_registry_url().join(name).ok()?;
+      let file = self
+        .file_fetcher
+        .fetch(&info_url, PermissionsContainer::allow_all())
+        .await
+        .ok()?;
+      serde_json::from_slice::<NpmPackageInfo>(&file.source).ok()
+    };
+    let info = fetch_package_info().await.map(Arc::new);
+    self.info_by_name.insert(name.to_string(), info.clone());
+    info
+  }
+}

cli/tools/registry/pm.rs

@@ -23,6 +23,7 @@ use crate::args::Flags;
 use crate::factory::CliFactory;
 use crate::file_fetcher::FileFetcher;
 use crate::jsr::JsrFetchResolver;
+use crate::npm::NpmFetchResolver;
 
 pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> {
   let cli_factory = CliFactory::from_flags(flags.clone()).await?;
@@ -77,13 +78,18 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> {
     None,
   );
   deps_file_fetcher.set_download_log_level(log::Level::Trace);
-  let jsr_resolver = Arc::new(JsrFetchResolver::new(deps_file_fetcher));
+  let jsr_resolver = Arc::new(JsrFetchResolver::new(deps_file_fetcher.clone()));
+  let npm_resolver = Arc::new(NpmFetchResolver::new(deps_file_fetcher));
 
   let package_futures = package_reqs
     .into_iter()
     .map(move |package_req| {
-      find_package_and_select_version_for_req(jsr_resolver.clone(), package_req)
+      find_package_and_select_version_for_req(
-        .boxed_local()
+        jsr_resolver.clone(),
+        npm_resolver.clone(),
+        package_req,
+      )
+      .boxed_local()
     })
     .collect::<Vec<_>>();
 
@@ -183,6 +189,7 @@ enum PackageAndVersion {
 
 async fn find_package_and_select_version_for_req(
   jsr_resolver: Arc<JsrFetchResolver>,
+  npm_resolver: Arc<NpmFetchResolver>,
   add_package_req: AddPackageReq,
 ) -> Result<PackageAndVersion, AnyError> {
   match add_package_req {
@@ -203,11 +210,22 @@ async fn find_package_and_select_version_for_req(
         version_req: format!("{}{}", range_symbol, &nv.version),
       }))
     }
-    AddPackageReq::Npm(pkg_req) => {
+    AddPackageReq::Npm(pkg_ref) => {
-      bail!(
+      let req = pkg_ref.req();
-        "Adding npm: packages is currently not supported. Package: npm:{}",
+      let npm_prefixed_name = format!("npm:{}", &req.name);
-        pkg_req.req().name
+      let Some(nv) = npm_resolver.req_to_nv(req).await else {
-      );
+        return Ok(PackageAndVersion::NotFound(npm_prefixed_name));
+      };
+      let range_symbol = if req.version_req.version_text().starts_with('~') {
+        '~'
+      } else {
+        '^'
+      };
+      Ok(PackageAndVersion::Selected(SelectedPackage {
+        import_name: req.name.to_string(),
+        package_name: npm_prefixed_name,
+        version_req: format!("{}{}", range_symbol, &nv.version),
+      }))
     }
   }
 }

tests/integration/pm_tests.rs

@@ -2,9 +2,7 @@
 
 use deno_core::serde_json::json;
 use test_util::assert_contains;
-use test_util::env_vars_for_jsr_tests;
+use test_util::env_vars_for_jsr_npm_tests;
-// use test_util::env_vars_for_npm_tests;
-// use test_util::itest;
 use test_util::TestContextBuilder;
 
 #[test]
@@ -110,21 +108,24 @@ fn add_multiple() {
 }
 
 #[test]
-fn add_not_supported_npm() {
+fn add_npm() {
   let context = pm_context_builder().build();
+  let temp_dir = context.temp_dir().path();
 
-  let output = context
+  let output = context.new_command().args("add npm:chalk@4.1").run();
-    .new_command()
+  output.assert_exit_code(0);
-    .args("add @denotest/add npm:express")
-    .run();
-  output.assert_exit_code(1);
   let output = output.combined_output();
-  assert_contains!(output, "error: Adding npm: packages is currently not supported. Package: npm:express");
+  assert_contains!(output, "Add chalk");
+  temp_dir.join("deno.json").assert_matches_json(json!({
+    "imports": {
+      "chalk": "npm:chalk@^4.1.2"
+    }
+  }));
 }
 
 fn pm_context_builder() -> TestContextBuilder {
   TestContextBuilder::new()
     .use_http_server()
-    .envs(env_vars_for_jsr_tests())
+    .envs(env_vars_for_jsr_npm_tests())
     .use_temp_cwd()
 }