fix(npm): improve peer dependency resolution

cli/Cargo.toml

@@ -46,13 +46,14 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_gra
 deno_core.workspace = true
 deno_doc = "0.55.0"
 deno_emit = "0.15.0"
-deno_graph = "0.43.2"
+deno_graph = "0.43.3"
 deno_lint = { version = "0.38.0", features = ["docs"] }
 deno_lockfile.workspace = true
 deno_runtime.workspace = true
 deno_task_shell = "0.8.1"
 napi_sym.workspace = true
 
+async-trait.workspace = true
 atty.workspace = true
 base32 = "=0.4.0"
 base64.workspace = true

cli/args/lockfile.rs

@@ -75,8 +75,8 @@ impl Into<NpmPackageLockfileInfo> for NpmResolutionPackage {
       .collect();
 
     NpmPackageLockfileInfo {
-      display_id: self.id.display(),
-      serialized_id: self.id.as_serialized(),
+      display_id: self.pkg_id.nv.to_string(),
+      serialized_id: self.pkg_id.as_serialized(),
       integrity: self.dist.integrity().to_string(),
       dependencies,
     }

cli/cache/mod.rs

@@ -103,7 +103,8 @@ impl Loader for FetchCacher {
   ) -> LoadFuture {
     if specifier.scheme() == "npm" {
       return Box::pin(futures::future::ready(
-        match deno_graph::npm::NpmPackageReference::from_specifier(specifier) {
+        match deno_graph::npm::NpmPackageReqReference::from_specifier(specifier)
+        {
           Ok(_) => Ok(Some(deno_graph::source::LoadResponse::External {
             specifier: specifier.clone(),
           })),

cli/lsp/diagnostics.rs

@@ -26,7 +26,7 @@ use deno_core::serde::Deserialize;
 use deno_core::serde_json;
 use deno_core::serde_json::json;
 use deno_core::ModuleSpecifier;
-use deno_graph::npm::NpmPackageReference;
+use deno_graph::npm::NpmPackageReqReference;
 use deno_graph::Resolution;
 use deno_graph::ResolutionError;
 use deno_graph::SpecifierError;
@@ -614,7 +614,7 @@ pub enum DenoDiagnostic {
   /// A data module was not found in the cache.
   NoCacheData(ModuleSpecifier),
   /// A remote npm package reference was not found in the cache.
-  NoCacheNpm(NpmPackageReference, ModuleSpecifier),
+  NoCacheNpm(NpmPackageReqReference, ModuleSpecifier),
   /// A local module was not found on the local file system.
   NoLocal(ModuleSpecifier),
   /// The specifier resolved to a remote specifier that was redirected to
@@ -905,7 +905,8 @@ fn diagnose_resolution(
               .push(DenoDiagnostic::NoAssertType.to_lsp_diagnostic(&range)),
           }
         }
-      } else if let Ok(pkg_ref) = NpmPackageReference::from_specifier(specifier)
+      } else if let Ok(pkg_ref) =
+        NpmPackageReqReference::from_specifier(specifier)
       {
         if let Some(npm_resolver) = &snapshot.maybe_npm_resolver {
           // show diagnostics for npm package references that aren't cached
@@ -929,7 +930,7 @@ fn diagnose_resolution(
         } else if let Some(npm_resolver) = &snapshot.maybe_npm_resolver {
           // check that a @types/node package exists in the resolver
           let types_node_ref =
-            NpmPackageReference::from_str("npm:@types/node").unwrap();
+            NpmPackageReqReference::from_str("npm:@types/node").unwrap();
           if npm_resolver
             .resolve_package_folder_from_deno_module(&types_node_ref.req)
             .is_err()

cli/lsp/documents.rs

@@ -30,8 +30,8 @@ use deno_core::futures::future;
 use deno_core::parking_lot::Mutex;
 use deno_core::url;
 use deno_core::ModuleSpecifier;
-use deno_graph::npm::NpmPackageReference;
 use deno_graph::npm::NpmPackageReq;
+use deno_graph::npm::NpmPackageReqReference;
 use deno_graph::GraphImport;
 use deno_graph::Resolution;
 use deno_runtime::deno_node::NodeResolutionMode;
@@ -1103,7 +1103,7 @@ impl Documents {
         .and_then(|r| r.maybe_specifier())
       {
         results.push(self.resolve_dependency(specifier, maybe_npm_resolver));
-      } else if let Ok(npm_ref) = NpmPackageReference::from_str(&specifier) {
+      } else if let Ok(npm_ref) = NpmPackageReqReference::from_str(&specifier) {
         results.push(maybe_npm_resolver.map(|npm_resolver| {
           NodeResolution::into_specifier_and_media_type(
             node_resolve_npm_reference(
@@ -1243,7 +1243,7 @@ impl Documents {
           // perf: ensure this is not added to unless this specifier has never
           // been analyzed in order to not cause an extra file system lookup
           self.pending_specifiers.push_back(dep.clone());
-          if let Ok(reference) = NpmPackageReference::from_specifier(dep) {
+          if let Ok(reference) = NpmPackageReqReference::from_specifier(dep) {
             self.npm_reqs.insert(reference.req);
           }
         }
@@ -1321,7 +1321,7 @@ impl Documents {
     specifier: &ModuleSpecifier,
     maybe_npm_resolver: Option<&NpmPackageResolver>,
   ) -> Option<(ModuleSpecifier, MediaType)> {
-    if let Ok(npm_ref) = NpmPackageReference::from_specifier(specifier) {
+    if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) {
       return maybe_npm_resolver.map(|npm_resolver| {
         NodeResolution::into_specifier_and_media_type(
           node_resolve_npm_reference(

cli/lsp/language_server.rs

@@ -73,7 +73,7 @@ use crate::graph_util;
 use crate::http_util::HttpClient;
 use crate::npm::NpmCache;
 use crate::npm::NpmPackageResolver;
-use crate::npm::RealNpmRegistryApi;
+use crate::npm::NpmRegistryApi;
 use crate::proc_state::ProcState;
 use crate::tools::fmt::format_file;
 use crate::tools::fmt::format_parsed_source;
@@ -304,7 +304,7 @@ fn create_lsp_npm_resolver(
   dir: &DenoDir,
   http_client: HttpClient,
 ) -> NpmPackageResolver {
-  let registry_url = RealNpmRegistryApi::default_url();
+  let registry_url = NpmRegistryApi::default_url();
   let progress_bar = ProgressBar::new(ProgressBarStyle::TextOnly);
   let npm_cache = NpmCache::from_deno_dir(
     dir,
@@ -316,8 +316,8 @@ fn create_lsp_npm_resolver(
     http_client.clone(),
     progress_bar.clone(),
   );
-  let api = RealNpmRegistryApi::new(
-    registry_url,
+  let api = NpmRegistryApi::new(
+    registry_url.clone(),
     npm_cache.clone(),
     http_client,
     progress_bar,

cli/node/mod.rs

@@ -15,8 +15,8 @@ use deno_core::error::generic_error;
 use deno_core::error::AnyError;
 use deno_core::serde_json::Value;
 use deno_core::url::Url;
-use deno_graph::npm::NpmPackageReference;
 use deno_graph::npm::NpmPackageReq;
+use deno_graph::npm::NpmPackageReqReference;
 use deno_runtime::deno_node;
 use deno_runtime::deno_node::errors;
 use deno_runtime::deno_node::find_builtin_node_module;
@@ -241,7 +241,7 @@ pub fn node_resolve(
 }
 
 pub fn node_resolve_npm_reference(
-  reference: &NpmPackageReference,
+  reference: &NpmPackageReqReference,
   mode: NodeResolutionMode,
   npm_resolver: &NpmPackageResolver,
   permissions: &mut dyn NodePermissions,

cli/npm/cache.rs

@@ -13,6 +13,7 @@ use deno_core::error::custom_error;
 use deno_core::error::AnyError;
 use deno_core::parking_lot::Mutex;
 use deno_core::url::Url;
+use deno_graph::npm::NpmPackageNv;
 use deno_graph::semver::Version;
 
 use crate::args::CacheSetting;
@@ -107,8 +108,7 @@ pub fn with_folder_sync_lock(
 }
 
 pub struct NpmPackageCacheFolderId {
-  pub name: String,
-  pub version: Version,
+  pub nv: NpmPackageNv,
   /// Peer dependency resolution may require us to have duplicate copies
   /// of the same package.
   pub copy_index: usize,
@@ -117,16 +117,15 @@ pub struct NpmPackageCacheFolderId {
 impl NpmPackageCacheFolderId {
   pub fn with_no_count(&self) -> Self {
     Self {
-      name: self.name.clone(),
-      version: self.version.clone(),
+      nv: self.nv.clone(),
       copy_index: 0,
     }
   }
 }
 
 impl std::fmt::Display for NpmPackageCacheFolderId {
   fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-    write!(f, "{}@{}", self.name, self.version)?;
+    write!(f, "{}", self.nv)?;
     if self.copy_index > 0 {
       write!(f, "_{}", self.copy_index)?;
     }
@@ -188,14 +187,14 @@ impl ReadonlyNpmCache {
   ) -> PathBuf {
     if id.copy_index == 0 {
       self.package_folder_for_name_and_version(
-        &id.name,
-        &id.version,
+        &id.nv.name,
+        &id.nv.version,
         registry_url,
       )
     } else {
       self
-        .package_name_folder(&id.name, registry_url)
-        .join(format!("{}_{}", id.version, id.copy_index))
+        .package_name_folder(&id.nv.name, registry_url)
+        .join(format!("{}_{}", id.nv.version, id.copy_index))
     }
   }
 
@@ -304,8 +303,10 @@ impl ReadonlyNpmCache {
         (version_part, 0)
       };
     Some(NpmPackageCacheFolderId {
-      name,
-      version: Version::parse_from_npm(version).ok()?,
+      nv: NpmPackageNv {
+        name,
+        version: Version::parse_from_npm(version).ok()?,
+      },
       copy_index,
     })
   }
@@ -440,16 +441,19 @@ impl NpmCache {
       // if this file exists, then the package didn't successfully extract
       // the first time, or another process is currently extracting the zip file
       && !package_folder.join(NPM_PACKAGE_SYNC_LOCK_FILENAME).exists()
-      && self.cache_setting.should_use_for_npm_package(&id.name)
+      && self.cache_setting.should_use_for_npm_package(&id.nv.name)
     {
       return Ok(());
     }
 
-    let original_package_folder = self
-      .readonly
-      .package_folder_for_name_and_version(&id.name, &id.version, registry_url);
+    let original_package_folder =
+      self.readonly.package_folder_for_name_and_version(
+        &id.nv.name,
+        &id.nv.version,
+        registry_url,
+      );
     with_folder_sync_lock(
-      (id.name.as_str(), &id.version),
+      (id.nv.name.as_str(), &id.nv.version),
       &package_folder,
       || hard_link_dir_recursive(&original_package_folder, &package_folder),
     )?;
@@ -514,6 +518,7 @@ pub fn mixed_case_package_name_decode(name: &str) -> Option<String> {
 #[cfg(test)]
 mod test {
   use deno_core::url::Url;
+  use deno_graph::npm::NpmPackageNv;
   use deno_graph::semver::Version;
 
   use super::ReadonlyNpmCache;
@@ -529,8 +534,10 @@ mod test {
     assert_eq!(
       cache.package_folder_for_id(
         &NpmPackageCacheFolderId {
-          name: "json".to_string(),
-          version: Version::parse_from_npm("1.2.5").unwrap(),
+          nv: NpmPackageNv {
+            name: "json".to_string(),
+            version: Version::parse_from_npm("1.2.5").unwrap(),
+          },
           copy_index: 0,
         },
         &registry_url,
@@ -544,8 +551,10 @@ mod test {
     assert_eq!(
       cache.package_folder_for_id(
         &NpmPackageCacheFolderId {
-          name: "json".to_string(),
-          version: Version::parse_from_npm("1.2.5").unwrap(),
+          nv: NpmPackageNv {
+            name: "json".to_string(),
+            version: Version::parse_from_npm("1.2.5").unwrap(),
+          },
           copy_index: 1,
         },
         &registry_url,
@@ -559,8 +568,10 @@ mod test {
     assert_eq!(
       cache.package_folder_for_id(
         &NpmPackageCacheFolderId {
-          name: "JSON".to_string(),
-          version: Version::parse_from_npm("2.1.5").unwrap(),
+          nv: NpmPackageNv {
+            name: "JSON".to_string(),
+            version: Version::parse_from_npm("2.1.5").unwrap(),
+          },
           copy_index: 0,
         },
         &registry_url,
@@ -574,8 +585,10 @@ mod test {
     assert_eq!(
       cache.package_folder_for_id(
         &NpmPackageCacheFolderId {
-          name: "@types/JSON".to_string(),
-          version: Version::parse_from_npm("2.1.5").unwrap(),
+          nv: NpmPackageNv {
+            name: "@types/JSON".to_string(),
+            version: Version::parse_from_npm("2.1.5").unwrap(),
+          },
           copy_index: 0,
         },
         &registry_url,

cli/npm/mod.rs

@@ -10,9 +10,8 @@ pub use cache::NpmCache;
 #[cfg(test)]
 pub use registry::NpmPackageVersionDistInfo;
 pub use registry::NpmRegistryApi;
-pub use registry::RealNpmRegistryApi;
 pub use resolution::resolve_graph_npm_info;
-pub use resolution::NpmPackageNodeId;
+pub use resolution::NpmPackageId;
 pub use resolution::NpmResolutionPackage;
 pub use resolution::NpmResolutionSnapshot;
 pub use resolvers::NpmPackageResolver;