refactor: remove CliNpmReqResolver trait in deno_resolver (#27616)

Cargo.lock

@@ -2207,6 +2207,7 @@ dependencies = [
  "deno_package_json",
  "deno_path_util",
  "deno_semver",
+ "log",
  "node_resolver",
  "parking_lot",
  "sys_traits",

cli/factory.rs

@@ -720,11 +720,10 @@ impl CliFactory {
       .get_or_try_init_async(async {
         let npm_resolver = self.npm_resolver().await?;
         Ok(Arc::new(CliNpmReqResolver::new(NpmReqResolverOptions {
-          byonm_resolver: (npm_resolver.clone()).into_maybe_byonm(),
           sys: self.sys(),
           in_npm_pkg_checker: self.in_npm_pkg_checker()?.clone(),
           node_resolver: self.node_resolver().await?.clone(),
-          npm_req_resolver: npm_resolver.clone().into_npm_req_resolver(),
+          npm_resolver: npm_resolver.clone().into_byonm_or_managed(),
         })))
       })
       .await

cli/lsp/resolver.rs

@@ -796,10 +796,9 @@ impl<'a> ResolverFactory<'a> {
         let node_resolver = self.node_resolver()?;
         let npm_resolver = self.npm_resolver()?;
         Some(Arc::new(CliNpmReqResolver::new(NpmReqResolverOptions {
-          byonm_resolver: (npm_resolver.clone()).into_maybe_byonm(),
           in_npm_pkg_checker: self.in_npm_pkg_checker().clone(),
           node_resolver: node_resolver.clone(),
-          npm_req_resolver: npm_resolver.clone().into_npm_req_resolver(),
+          npm_resolver: npm_resolver.clone().into_byonm_or_managed(),
           sys: self.sys.clone(),
         })))
       })

cli/npm/byonm.rs

@@ -1,13 +1,17 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 
 use std::path::Path;
+use std::path::PathBuf;
 use std::sync::Arc;
 
 use deno_core::serde_json;
+use deno_core::url::Url;
 use deno_resolver::npm::ByonmNpmResolver;
 use deno_resolver::npm::ByonmNpmResolverCreateOptions;
-use deno_resolver::npm::CliNpmReqResolver;
+use deno_resolver::npm::ByonmOrManagedNpmResolver;
+use deno_resolver::npm::ResolvePkgFolderFromDenoReqError;
 use deno_runtime::ops::process::NpmProcessStateProvider;
+use deno_semver::package::PackageReq;
 use node_resolver::NpmPackageFolderResolver;
 
 use super::CliNpmResolver;
@@ -44,18 +48,16 @@ impl CliNpmResolver for CliByonmNpmResolver {
     self
   }
 
-  fn into_npm_req_resolver(self: Arc<Self>) -> Arc<dyn CliNpmReqResolver> {
-    self
-  }
-
   fn into_process_state_provider(
     self: Arc<Self>,
   ) -> Arc<dyn NpmProcessStateProvider> {
     Arc::new(CliByonmWrapper(self))
   }
 
-  fn into_maybe_byonm(self: Arc<Self>) -> Option<Arc<CliByonmNpmResolver>> {
+  fn into_byonm_or_managed(
-    Some(self)
+    self: Arc<Self>,
+  ) -> ByonmOrManagedNpmResolver<CliSys> {
+    ByonmOrManagedNpmResolver::Byonm(self)
   }
 
   fn clone_snapshotted(&self) -> Arc<dyn CliNpmResolver> {
@@ -75,4 +77,14 @@ impl CliNpmResolver for CliByonmNpmResolver {
     // so we just return None to signify check caching is not supported
     None
   }
+
+  fn resolve_pkg_folder_from_deno_module_req(
+    &self,
+    req: &PackageReq,
+    referrer: &Url,
+  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError> {
+    self
+      .resolve_pkg_folder_from_deno_module_req(req, referrer)
+      .map_err(ResolvePkgFolderFromDenoReqError::Byonm)
+  }
 }

cli/npm/managed/mod.rs

@@ -22,12 +22,11 @@ use deno_npm::NpmPackageId;
 use deno_npm::NpmResolutionPackage;
 use deno_npm::NpmSystemInfo;
 use deno_npm_cache::NpmCacheSetting;
-use deno_path_util::fs::canonicalize_path_maybe_not_exists;
-use deno_resolver::npm::managed::create_npm_fs_resolver;
-use deno_resolver::npm::managed::NpmPackageFsResolver;
-use deno_resolver::npm::managed::NpmPackageFsResolverPackageFolderError;
 use deno_resolver::npm::managed::NpmResolution;
-use deno_resolver::npm::CliNpmReqResolver;
+use deno_resolver::npm::managed::ResolvePkgFolderFromPkgIdError;
+use deno_resolver::npm::ByonmOrManagedNpmResolver;
+use deno_resolver::npm::ManagedNpmResolver;
+use deno_resolver::npm::ResolvePkgFolderFromDenoReqError;
 use deno_runtime::colors;
 use deno_runtime::ops::process::NpmProcessStateProvider;
 use deno_semver::package::PackageNv;
@@ -36,8 +35,6 @@ use installer::AddPkgReqsResult;
 use installer::NpmResolutionInstaller;
 use installers::create_npm_fs_installer;
 use installers::NpmPackageFsInstaller;
-use node_resolver::errors::PackageFolderResolveError;
-use node_resolver::errors::PackageFolderResolveIoError;
 use node_resolver::NpmPackageFolderResolver;
 
 use super::CliNpmCache;
@@ -46,7 +43,6 @@ use super::CliNpmRegistryInfoProvider;
 use super::CliNpmResolver;
 use super::CliNpmTarballCache;
 use super::InnerCliNpmResolverRef;
-use super::ResolvePkgFolderFromDenoReqError;
 use crate::args::CliLockfile;
 use crate::args::LifecycleScriptsConfig;
 use crate::args::NpmInstallDepsProvider;
@@ -181,22 +177,23 @@ fn create_inner(
     npm_system_info.clone(),
     lifecycle_scripts.clone(),
   );
-  let fs_resolver = create_npm_fs_resolver(
+  let managed_npm_resolver =
-    &npm_cache_dir,
+    Arc::new(ManagedNpmResolver::<CliSys>::new::<CliSys>(
-    &npm_rc,
+      &npm_cache_dir,
-    resolution.clone(),
+      &npm_rc,
-    sys.clone(),
+      resolution.clone(),
-    node_modules_dir_path,
+      sys.clone(),
-  );
+      node_modules_dir_path,
+    ));
   Arc::new(ManagedCliNpmResolver::new(
     fs_installer,
-    fs_resolver,
     maybe_lockfile,
-    registry_info_provider,
+    managed_npm_resolver,
     npm_cache,
     npm_cache_dir,
     npm_install_deps_provider,
     npm_rc,
+    registry_info_provider,
     resolution,
     sys,
     tarball_cache,
@@ -281,13 +278,23 @@ pub enum PackageCaching<'a> {
   All,
 }
 
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
+pub enum ResolvePkgFolderFromDenoModuleError {
+  #[class(inherit)]
+  #[error(transparent)]
+  PackageNvNotFound(#[from] deno_npm::resolution::PackageNvNotFoundError),
+  #[class(inherit)]
+  #[error(transparent)]
+  ResolvePkgFolderFromPkgId(#[from] ResolvePkgFolderFromPkgIdError),
+}
+
 /// An npm resolver where the resolution is managed by Deno rather than
 /// the user bringing their own node_modules (BYONM) on the file system.
 pub struct ManagedCliNpmResolver {
   fs_installer: Arc<dyn NpmPackageFsInstaller>,
-  fs_resolver: Arc<dyn NpmPackageFsResolver>,
   maybe_lockfile: Option<Arc<CliLockfile>>,
   registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
+  managed_npm_resolver: Arc<ManagedNpmResolver<CliSys>>,
   npm_cache: Arc<CliNpmCache>,
   npm_cache_dir: Arc<NpmCacheDir>,
   npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
@@ -304,45 +311,23 @@ pub struct ManagedCliNpmResolver {
 
 impl std::fmt::Debug for ManagedCliNpmResolver {
   fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-    f.debug_struct("ManagedNpmResolver")
+    f.debug_struct("ManagedCliNpmResolver")
       .field("<omitted>", &"<omitted>")
       .finish()
   }
 }
 
-#[derive(Debug, thiserror::Error, deno_error::JsError)]
-pub enum ResolvePkgFolderFromPkgIdError {
-  #[class(inherit)]
-  #[error("{0}")]
-  NpmPackageFsResolverPackageFolder(
-    #[from] NpmPackageFsResolverPackageFolderError,
-  ),
-  #[class(inherit)]
-  #[error("{0}")]
-  Io(#[from] std::io::Error),
-}
-
-#[derive(Debug, thiserror::Error, deno_error::JsError)]
-pub enum ResolvePkgFolderFromDenoModuleError {
-  #[class(inherit)]
-  #[error("{0}")]
-  PackageNvNotFound(#[from] deno_npm::resolution::PackageNvNotFoundError),
-  #[class(inherit)]
-  #[error("{0}")]
-  ResolvePkgFolderFromPkgId(#[from] ResolvePkgFolderFromPkgIdError),
-}
-
 impl ManagedCliNpmResolver {
   #[allow(clippy::too_many_arguments)]
   pub fn new(
     fs_installer: Arc<dyn NpmPackageFsInstaller>,
-    fs_resolver: Arc<dyn NpmPackageFsResolver>,
     maybe_lockfile: Option<Arc<CliLockfile>>,
-    registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
+    managed_npm_resolver: Arc<ManagedNpmResolver<CliSys>>,
     npm_cache: Arc<CliNpmCache>,
     npm_cache_dir: Arc<NpmCacheDir>,
     npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
     npm_rc: Arc<ResolvedNpmRc>,
+    registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
     resolution: Arc<NpmResolution>,
     sys: CliSys,
     tarball_cache: Arc<CliNpmTarballCache>,
@@ -357,13 +342,13 @@ impl ManagedCliNpmResolver {
     );
     Self {
       fs_installer,
-      fs_resolver,
       maybe_lockfile,
-      registry_info_provider,
+      managed_npm_resolver,
       npm_cache,
       npm_cache_dir,
       npm_install_deps_provider,
       npm_rc,
+      registry_info_provider,
       text_only_progress_bar,
       resolution,
       resolution_installer,
@@ -379,14 +364,9 @@ impl ManagedCliNpmResolver {
     &self,
     pkg_id: &NpmPackageId,
   ) -> Result<PathBuf, ResolvePkgFolderFromPkgIdError> {
-    let path = self.fs_resolver.package_folder(pkg_id)?;
+    self
-    let path = canonicalize_path_maybe_not_exists(&self.sys, &path)?;
+      .managed_npm_resolver
-    log::debug!(
+      .resolve_pkg_folder_from_pkg_id(pkg_id)
-      "Resolved package folder of {} to {}",
-      pkg_id.as_serialized(),
-      path.display()
-    );
-    Ok(path)
   }
 
   /// Resolves the package id from the provided specifier.
@@ -395,7 +375,7 @@ impl ManagedCliNpmResolver {
     specifier: &ModuleSpecifier,
   ) -> Result<Option<NpmPackageId>, AnyError> {
     let Some(cache_folder_id) = self
-      .fs_resolver
+      .managed_npm_resolver
       .resolve_package_cache_folder_id_from_specifier(specifier)?
     else {
       return Ok(None);
@@ -419,7 +399,9 @@ impl ManagedCliNpmResolver {
     &self,
     package_id: &NpmPackageId,
   ) -> Result<u64, AnyError> {
-    let package_folder = self.fs_resolver.package_folder(package_id)?;
+    let package_folder = self
+      .managed_npm_resolver
+      .resolve_pkg_folder_from_pkg_id(package_id)?;
     Ok(crate::util::fs::dir_size(&package_folder)?)
   }
 
@@ -435,7 +417,12 @@ impl ManagedCliNpmResolver {
     self
       .resolve_pkg_id_from_pkg_req(req)
       .ok()
-      .and_then(|id| self.fs_resolver.package_folder(&id).ok())
+      .and_then(|id| {
+        self
+          .managed_npm_resolver
+          .resolve_pkg_folder_from_pkg_id(&id)
+          .ok()
+      })
       .map(|folder| folder.exists())
       .unwrap_or(false)
   }
@@ -648,7 +635,7 @@ impl ManagedCliNpmResolver {
   }
 
   pub fn maybe_node_modules_path(&self) -> Option<&Path> {
-    self.fs_resolver.node_modules_path()
+    self.managed_npm_resolver.node_modules_path()
   }
 
   pub fn global_cache_root_path(&self) -> &Path {
@@ -672,61 +659,20 @@ fn npm_process_state(
   .unwrap()
 }
 
-impl NpmPackageFolderResolver for ManagedCliNpmResolver {
-  fn resolve_package_folder_from_package(
-    &self,
-    name: &str,
-    referrer: &ModuleSpecifier,
-  ) -> Result<PathBuf, PackageFolderResolveError> {
-    let path = self
-      .fs_resolver
-      .resolve_package_folder_from_package(name, referrer)?;
-    let path =
-      canonicalize_path_maybe_not_exists(&self.sys, &path).map_err(|err| {
-        PackageFolderResolveIoError {
-          package_name: name.to_string(),
-          referrer: referrer.clone(),
-          source: err,
-        }
-      })?;
-    log::debug!("Resolved {} from {} to {}", name, referrer, path.display());
-    Ok(path)
-  }
-}
-
 impl NpmProcessStateProvider for ManagedCliNpmResolver {
   fn get_npm_process_state(&self) -> String {
     npm_process_state(
       self.resolution.serialized_valid_snapshot(),
-      self.fs_resolver.node_modules_path(),
+      self.managed_npm_resolver.node_modules_path(),
     )
   }
 }
 
-impl CliNpmReqResolver for ManagedCliNpmResolver {
-  fn resolve_pkg_folder_from_deno_module_req(
-    &self,
-    req: &PackageReq,
-    _referrer: &ModuleSpecifier,
-  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError> {
-    let pkg_id = self.resolve_pkg_id_from_pkg_req(req).map_err(|err| {
-      ResolvePkgFolderFromDenoReqError::Managed(Box::new(err))
-    })?;
-    self
-      .resolve_pkg_folder_from_pkg_id(&pkg_id)
-      .map_err(|err| ResolvePkgFolderFromDenoReqError::Managed(Box::new(err)))
-  }
-}
-
 impl CliNpmResolver for ManagedCliNpmResolver {
   fn into_npm_pkg_folder_resolver(
     self: Arc<Self>,
   ) -> Arc<dyn NpmPackageFolderResolver> {
-    self
+    self.managed_npm_resolver.clone()
-  }
-
-  fn into_npm_req_resolver(self: Arc<Self>) -> Arc<dyn CliNpmReqResolver> {
-    self
   }
 
   fn into_process_state_provider(
@@ -735,6 +681,12 @@ impl CliNpmResolver for ManagedCliNpmResolver {
     self
   }
 
+  fn into_byonm_or_managed(
+    self: Arc<Self>,
+  ) -> ByonmOrManagedNpmResolver<CliSys> {
+    ByonmOrManagedNpmResolver::Managed(self.managed_npm_resolver.clone())
+  }
+
   fn clone_snapshotted(&self) -> Arc<dyn CliNpmResolver> {
     // create a new snapshotted npm resolution and resolver
     let npm_resolution =
@@ -752,19 +704,19 @@ impl CliNpmResolver for ManagedCliNpmResolver {
         self.npm_system_info.clone(),
         self.lifecycle_scripts.clone(),
       ),
-      create_npm_fs_resolver(
+      self.maybe_lockfile.clone(),
+      Arc::new(ManagedNpmResolver::<CliSys>::new::<CliSys>(
         &self.npm_cache_dir,
         &self.npm_rc,
         npm_resolution.clone(),
         self.sys.clone(),
         self.root_node_modules_path().map(ToOwned::to_owned),
-      ),
+      )),
-      self.maybe_lockfile.clone(),
-      self.registry_info_provider.clone(),
       self.npm_cache.clone(),
       self.npm_cache_dir.clone(),
       self.npm_install_deps_provider.clone(),
       self.npm_rc.clone(),
+      self.registry_info_provider.clone(),
       npm_resolution,
       self.sys.clone(),
       self.tarball_cache.clone(),
@@ -779,7 +731,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
   }
 
   fn root_node_modules_path(&self) -> Option<&Path> {
-    self.fs_resolver.node_modules_path()
+    self.managed_npm_resolver.node_modules_path()
   }
 
   fn check_state_hash(&self) -> Option<u64> {
@@ -794,11 +746,23 @@ impl CliNpmResolver for ManagedCliNpmResolver {
     let mut hasher = FastInsecureHasher::new_without_deno_version();
     // ensure the cache gets busted when turning nodeModulesDir on or off
     // as this could cause changes in resolution
-    hasher.write_hashable(self.fs_resolver.node_modules_path().is_some());
+    hasher
+      .write_hashable(self.managed_npm_resolver.node_modules_path().is_some());
     for (pkg_req, pkg_nv) in package_reqs {
       hasher.write_hashable(&pkg_req);
       hasher.write_hashable(&pkg_nv);
     }
     Some(hasher.finish())
   }
+
+  fn resolve_pkg_folder_from_deno_module_req(
+    &self,
+    req: &PackageReq,
+    referrer: &Url,
+  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError> {
+    self
+      .managed_npm_resolver
+      .resolve_pkg_folder_from_deno_module_req(req, referrer)
+      .map_err(ResolvePkgFolderFromDenoReqError::Managed)
+  }
 }

cli/npm/mod.rs

@@ -5,6 +5,7 @@ mod managed;
 mod permission_checker;
 
 use std::path::Path;
+use std::path::PathBuf;
 use std::sync::Arc;
 
 use dashmap::DashMap;
@@ -15,7 +16,7 @@ use deno_error::JsErrorBox;
 use deno_npm::npm_rc::ResolvedNpmRc;
 use deno_npm::registry::NpmPackageInfo;
 use deno_resolver::npm::ByonmNpmResolver;
-use deno_resolver::npm::CliNpmReqResolver;
+use deno_resolver::npm::ByonmOrManagedNpmResolver;
 use deno_resolver::npm::ResolvePkgFolderFromDenoReqError;
 use deno_runtime::ops::process::NpmProcessStateProvider;
 use deno_semver::package::PackageNv;
@@ -136,17 +137,17 @@ pub enum InnerCliNpmResolverRef<'a> {
   Byonm(&'a CliByonmNpmResolver),
 }
 
-pub trait CliNpmResolver: CliNpmReqResolver {
+// todo(dsherret): replace with an enum
+pub trait CliNpmResolver: Send + Sync + std::fmt::Debug {
   fn into_npm_pkg_folder_resolver(
     self: Arc<Self>,
   ) -> Arc<dyn NpmPackageFolderResolver>;
-  fn into_npm_req_resolver(self: Arc<Self>) -> Arc<dyn CliNpmReqResolver>;
   fn into_process_state_provider(
     self: Arc<Self>,
   ) -> Arc<dyn NpmProcessStateProvider>;
-  fn into_maybe_byonm(self: Arc<Self>) -> Option<Arc<CliByonmNpmResolver>> {
+  fn into_byonm_or_managed(
-    None
+    self: Arc<Self>,
-  }
+  ) -> ByonmOrManagedNpmResolver<CliSys>;
 
   fn clone_snapshotted(&self) -> Arc<dyn CliNpmResolver>;
 
@@ -166,6 +167,12 @@ pub trait CliNpmResolver: CliNpmReqResolver {
     }
   }
 
+  fn resolve_pkg_folder_from_deno_module_req(
+    &self,
+    req: &PackageReq,
+    referrer: &Url,
+  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError>;
+
   fn root_node_modules_path(&self) -> Option<&Path>;
 
   /// Returns a hash returning the state of the npm resolver

cli/standalone/mod.rs

@@ -849,11 +849,10 @@ pub async fn run(
   let node_analysis_cache = NodeAnalysisCache::new(cache_db.node_analysis_db());
   let npm_req_resolver =
     Arc::new(CliNpmReqResolver::new(NpmReqResolverOptions {
-      byonm_resolver: (npm_resolver.clone()).into_maybe_byonm(),
       sys: sys.clone(),
       in_npm_pkg_checker: in_npm_pkg_checker.clone(),
       node_resolver: node_resolver.clone(),
-      npm_req_resolver: npm_resolver.clone().into_npm_req_resolver(),
+      npm_resolver: npm_resolver.clone().into_byonm_or_managed(),
     }));
   let cjs_esm_code_analyzer = CliCjsCodeAnalyzer::new(
     node_analysis_cache,

resolvers/deno/Cargo.toml

@@ -30,6 +30,7 @@ deno_npm.workspace = true
 deno_package_json.workspace = true
 deno_path_util.workspace = true
 deno_semver.workspace = true
+log.workspace = true
 node_resolver.workspace = true
 parking_lot.workspace = true
 sys_traits.workspace = true

resolvers/deno/npm/byonm.rs

@@ -27,8 +27,8 @@ use thiserror::Error;
 use url::Url;
 
 use super::local::normalize_pkg_name_for_node_modules_deno_folder;
-use super::CliNpmReqResolver;
+use crate::sync::MaybeSend;
-use super::ResolvePkgFolderFromDenoReqError;
+use crate::sync::MaybeSync;
 
 #[derive(Debug, Error, deno_error::JsError)]
 pub enum ByonmResolvePkgFolderFromDenoReqError {
@@ -377,35 +377,13 @@ impl<TSys: FsCanonicalize + FsRead + FsMetadata + FsReadDir>
   }
 }
 
-impl<
-    Sys: FsCanonicalize
-      + FsMetadata
-      + FsRead
-      + FsReadDir
-      + Send
-      + Sync
-      + std::fmt::Debug,
-  > CliNpmReqResolver for ByonmNpmResolver<Sys>
-{
-  fn resolve_pkg_folder_from_deno_module_req(
-    &self,
-    req: &PackageReq,
-    referrer: &Url,
-  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError> {
-    ByonmNpmResolver::resolve_pkg_folder_from_deno_module_req(
-      self, req, referrer,
-    )
-    .map_err(ResolvePkgFolderFromDenoReqError::Byonm)
-  }
-}
-
 impl<
     TSys: FsCanonicalize
       + FsMetadata
       + FsRead
       + FsReadDir
-      + Send
+      + MaybeSend
-      + Sync
+      + MaybeSync
       + std::fmt::Debug,
   > NpmPackageFolderResolver for ByonmNpmResolver<TSys>
 {

resolvers/deno/npm/managed/common.rs

@@ -12,14 +12,9 @@ use crate::sync::MaybeSend;
 use crate::sync::MaybeSync;
 
 #[allow(clippy::disallowed_types)]
-pub(super) type NpmPackageFsResolverRc =
+pub type NpmPackageFsResolverRc =
   crate::sync::MaybeArc<dyn NpmPackageFsResolver>;
 
-#[derive(Debug, thiserror::Error, deno_error::JsError)]
-#[class(generic)]
-#[error("Package folder not found for '{0}'")]
-pub struct NpmPackageFsResolverPackageFolderError(deno_semver::StackString);
-
 /// Part of the resolution that interacts with the file system.
 pub trait NpmPackageFsResolver:
   NpmPackageFolderResolver + MaybeSend + MaybeSync
@@ -29,15 +24,6 @@ pub trait NpmPackageFsResolver:
 
   fn maybe_package_folder(&self, package_id: &NpmPackageId) -> Option<PathBuf>;
 
-  fn package_folder(
-    &self,
-    package_id: &NpmPackageId,
-  ) -> Result<PathBuf, NpmPackageFsResolverPackageFolderError> {
-    self.maybe_package_folder(package_id).ok_or_else(|| {
-      NpmPackageFsResolverPackageFolderError(package_id.as_serialized())
-    })
-  }
-
   fn resolve_package_cache_folder_id_from_specifier(
     &self,
     specifier: &Url,

resolvers/deno/npm/managed/local.rs

@@ -23,12 +23,14 @@ use super::resolution::NpmResolutionRc;
 use super::NpmPackageFsResolver;
 use crate::npm::local::get_package_folder_id_folder_name_from_parts;
 use crate::npm::local::get_package_folder_id_from_folder_name;
+use crate::sync::MaybeSend;
+use crate::sync::MaybeSync;
 
 /// Resolver that creates a local node_modules directory
 /// and resolves packages from it.
 #[derive(Debug)]
 pub struct LocalNpmPackageResolver<
-  TSys: FsCanonicalize + FsMetadata + Send + Sync,
+  TSys: FsCanonicalize + FsMetadata + MaybeSend + MaybeSync,
 > {
   resolution: NpmResolutionRc,
   sys: TSys,
@@ -36,7 +38,7 @@ pub struct LocalNpmPackageResolver<
   root_node_modules_url: Url,
 }
 
-impl<TSys: FsCanonicalize + FsMetadata + Send + Sync>
+impl<TSys: FsCanonicalize + FsMetadata + MaybeSend + MaybeSync>
   LocalNpmPackageResolver<TSys>
 {
   #[allow(clippy::too_many_arguments)]
@@ -99,8 +101,9 @@ impl<TSys: FsCanonicalize + FsMetadata + Send + Sync>
   }
 }
 
-impl<TSys: FsCanonicalize + FsMetadata + Send + Sync + std::fmt::Debug>
+impl<
-  NpmPackageFolderResolver for LocalNpmPackageResolver<TSys>
+    TSys: FsCanonicalize + FsMetadata + MaybeSend + MaybeSync + std::fmt::Debug,
+  > NpmPackageFolderResolver for LocalNpmPackageResolver<TSys>
 {
   fn resolve_package_folder_from_package(
     &self,
@@ -160,8 +163,9 @@ impl<TSys: FsCanonicalize + FsMetadata + Send + Sync + std::fmt::Debug>
   }
 }
 
-impl<TSys: FsCanonicalize + FsMetadata + Send + Sync + std::fmt::Debug>
+impl<
-  NpmPackageFsResolver for LocalNpmPackageResolver<TSys>
+    TSys: FsCanonicalize + FsMetadata + MaybeSend + MaybeSync + std::fmt::Debug,
+  > NpmPackageFsResolver for LocalNpmPackageResolver<TSys>
 {
   fn node_modules_path(&self) -> Option<&Path> {
     Some(self.root_node_modules_path.as_ref())

resolvers/deno/npm/managed/mod.rs

@@ -8,42 +8,180 @@ mod resolution;
 use std::path::Path;
 use std::path::PathBuf;
 
+use deno_npm::resolution::PackageReqNotFoundError;
+use deno_npm::NpmPackageCacheFolderId;
+use deno_npm::NpmPackageId;
+use deno_path_util::fs::canonicalize_path_maybe_not_exists;
+use deno_semver::package::PackageReq;
 use node_resolver::InNpmPackageChecker;
+use node_resolver::NpmPackageFolderResolver;
 use sys_traits::FsCanonicalize;
 use sys_traits::FsMetadata;
 use url::Url;
 
-pub use self::common::NpmPackageFsResolver;
+use self::common::NpmPackageFsResolver;
-pub use self::common::NpmPackageFsResolverPackageFolderError;
 use self::common::NpmPackageFsResolverRc;
 use self::global::GlobalNpmPackageResolver;
 use self::local::LocalNpmPackageResolver;
 pub use self::resolution::NpmResolution;
-use self::resolution::NpmResolutionRc;
+pub use self::resolution::NpmResolutionRc;
 use crate::sync::new_rc;
+use crate::sync::MaybeSend;
+use crate::sync::MaybeSync;
 use crate::NpmCacheDirRc;
 use crate::ResolvedNpmRcRc;
 
-pub fn create_npm_fs_resolver<
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
-  TSys: FsCanonicalize + FsMetadata + std::fmt::Debug + Send + Sync + 'static,
+#[error(transparent)]
->(
+pub enum ResolvePkgFolderFromPkgIdError {
-  npm_cache_dir: &NpmCacheDirRc,
+  #[class(inherit)]
-  npm_rc: &ResolvedNpmRcRc,
+  #[error(transparent)]
+  NotFound(#[from] NpmManagedPackageFolderNotFoundError),
+  #[class(inherit)]
+  #[error(transparent)]
+  FailedCanonicalizing(#[from] FailedCanonicalizingError),
+}
+
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
+#[class(generic)]
+#[error("Package folder not found for '{0}'")]
+pub struct NpmManagedPackageFolderNotFoundError(deno_semver::StackString);
+
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
+#[class(generic)]
+#[error("Failed canonicalizing '{}'", path.display())]
+pub struct FailedCanonicalizingError {
+  path: PathBuf,
+  #[source]
+  source: std::io::Error,
+}
+
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
+pub enum ManagedResolvePkgFolderFromDenoReqError {
+  #[class(inherit)]
+  #[error(transparent)]
+  PackageReqNotFound(#[from] PackageReqNotFoundError),
+  #[class(inherit)]
+  #[error(transparent)]
+  ResolvePkgFolderFromPkgId(#[from] ResolvePkgFolderFromPkgIdError),
+}
+
+#[allow(clippy::disallowed_types)]
+pub type ManagedNpmResolverRc<TSys> =
+  crate::sync::MaybeArc<ManagedNpmResolver<TSys>>;
+
+#[derive(Debug)]
+pub struct ManagedNpmResolver<TSys: FsCanonicalize> {
+  fs_resolver: NpmPackageFsResolverRc,
   resolution: NpmResolutionRc,
   sys: TSys,
-  maybe_node_modules_path: Option<PathBuf>,
+}
-) -> NpmPackageFsResolverRc {
+
-  match maybe_node_modules_path {
+impl<TSys: FsCanonicalize> ManagedNpmResolver<TSys> {
-    Some(node_modules_folder) => new_rc(LocalNpmPackageResolver::new(
+  pub fn new<
-      resolution,
+    TCreateSys: FsCanonicalize
+      + FsMetadata
+      + std::fmt::Debug
+      + MaybeSend
+      + MaybeSync
+      + Clone
+      + 'static,
+  >(
+    npm_cache_dir: &NpmCacheDirRc,
+    npm_rc: &ResolvedNpmRcRc,
+    resolution: NpmResolutionRc,
+    sys: TCreateSys,
+    maybe_node_modules_path: Option<PathBuf>,
+  ) -> ManagedNpmResolver<TCreateSys> {
+    let fs_resolver: NpmPackageFsResolverRc = match maybe_node_modules_path {
+      Some(node_modules_folder) => new_rc(LocalNpmPackageResolver::new(
+        resolution.clone(),
+        sys.clone(),
+        node_modules_folder,
+      )),
+      None => new_rc(GlobalNpmPackageResolver::new(
+        npm_cache_dir.clone(),
+        npm_rc.clone(),
+        resolution.clone(),
+      )),
+    };
+
+    ManagedNpmResolver {
+      fs_resolver,
       sys,
-      node_modules_folder,
-    )),
-    None => new_rc(GlobalNpmPackageResolver::new(
-      npm_cache_dir.clone(),
-      npm_rc.clone(),
       resolution,
-    )),
+    }
+  }
+
+  #[inline]
+  pub fn node_modules_path(&self) -> Option<&Path> {
+    self.fs_resolver.node_modules_path()
+  }
+
+  pub fn resolve_pkg_folder_from_pkg_id(
+    &self,
+    package_id: &NpmPackageId,
+  ) -> Result<PathBuf, ResolvePkgFolderFromPkgIdError> {
+    let path = self
+      .fs_resolver
+      .maybe_package_folder(package_id)
+      .ok_or_else(|| {
+        NpmManagedPackageFolderNotFoundError(package_id.as_serialized())
+      })?;
+    // todo(dsherret): investigate if this canonicalization is always
+    // necessary. For example, maybe it's not necessary for the global cache
+    let path = canonicalize_path_maybe_not_exists(&self.sys, &path).map_err(
+      |source| FailedCanonicalizingError {
+        path: path.to_path_buf(),
+        source,
+      },
+    )?;
+    log::debug!(
+      "Resolved package folder of {} to {}",
+      package_id.as_serialized(),
+      path.display()
+    );
+    Ok(path)
+  }
+
+  pub fn resolve_pkg_folder_from_deno_module_req(
+    &self,
+    req: &PackageReq,
+    _referrer: &Url,
+  ) -> Result<PathBuf, ManagedResolvePkgFolderFromDenoReqError> {
+    let pkg_id = self.resolution.resolve_pkg_id_from_pkg_req(req)?;
+    Ok(self.resolve_pkg_folder_from_pkg_id(&pkg_id)?)
+  }
+
+  #[inline]
+  pub fn resolve_package_cache_folder_id_from_specifier(
+    &self,
+    specifier: &Url,
+  ) -> Result<Option<NpmPackageCacheFolderId>, std::io::Error> {
+    self
+      .fs_resolver
+      .resolve_package_cache_folder_id_from_specifier(specifier)
+  }
+}
+
+impl<TSys: FsCanonicalize + std::fmt::Debug + MaybeSend + MaybeSync>
+  NpmPackageFolderResolver for ManagedNpmResolver<TSys>
+{
+  fn resolve_package_folder_from_package(
+    &self,
+    specifier: &str,
+    referrer: &Url,
+  ) -> Result<PathBuf, node_resolver::errors::PackageFolderResolveError> {
+    let path = self
+      .fs_resolver
+      .resolve_package_folder_from_package(specifier, referrer)?;
+    log::debug!(
+      "Resolved {} from {} to {}",
+      specifier,
+      referrer,
+      path.display()
+    );
+    Ok(path)
   }
 }
 

resolvers/deno/npm/managed/resolution.rs

@@ -18,7 +18,7 @@ use deno_semver::package::PackageReq;
 use parking_lot::RwLock;
 
 #[allow(clippy::disallowed_types)]
-pub(super) type NpmResolutionRc = crate::sync::MaybeArc<NpmResolution>;
+pub type NpmResolutionRc = crate::sync::MaybeArc<NpmResolution>;
 
 /// Handles updating and storing npm resolution in memory.
 ///

resolvers/deno/npm/mod.rs

@@ -36,9 +36,9 @@ pub use self::local::get_package_folder_id_folder_name;
 pub use self::local::normalize_pkg_name_for_node_modules_deno_folder;
 use self::managed::create_managed_in_npm_pkg_checker;
 use self::managed::ManagedInNpmPkgCheckerCreateOptions;
+pub use self::managed::ManagedNpmResolver;
+pub use self::managed::ManagedNpmResolverRc;
 use crate::sync::new_rc;
-use crate::sync::MaybeSend;
-use crate::sync::MaybeSync;
 
 mod byonm;
 mod local;
@@ -108,37 +108,50 @@ pub enum ResolveReqWithSubPathErrorKind {
 #[derive(Debug, Error, JsError)]
 pub enum ResolvePkgFolderFromDenoReqError {
   #[class(inherit)]
-  #[error("{0}")]
+  #[error(transparent)]
-  Managed(Box<dyn deno_error::JsErrorClass>),
+  Managed(managed::ManagedResolvePkgFolderFromDenoReqError),
   #[class(inherit)]
   #[error(transparent)]
-  Byonm(#[from] ByonmResolvePkgFolderFromDenoReqError),
+  Byonm(byonm::ByonmResolvePkgFolderFromDenoReqError),
 }
 
-#[allow(clippy::disallowed_types)]
+#[derive(Debug, Clone)]
-pub type CliNpmReqResolverRc = crate::sync::MaybeArc<dyn CliNpmReqResolver>;
+pub enum ByonmOrManagedNpmResolver<
+  TSys: FsCanonicalize + FsMetadata + FsRead + FsReadDir,
+> {
+  /// The resolver when "bring your own node_modules" is enabled where Deno
+  /// does not setup the node_modules directories automatically, but instead
+  /// uses what already exists on the file system.
+  Byonm(ByonmNpmResolverRc<TSys>),
+  Managed(ManagedNpmResolverRc<TSys>),
+}
 
-// todo(dsherret): a temporary trait until we extract
+impl<TSys: FsCanonicalize + FsMetadata + FsRead + FsReadDir>
-// out the CLI npm resolver into here
+  ByonmOrManagedNpmResolver<TSys>
-pub trait CliNpmReqResolver: Debug + MaybeSend + MaybeSync {
+{
-  fn resolve_pkg_folder_from_deno_module_req(
+  pub fn resolve_pkg_folder_from_deno_module_req(
     &self,
     req: &PackageReq,
     referrer: &Url,
-  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError>;
+  ) -> Result<PathBuf, ResolvePkgFolderFromDenoReqError> {
+    match self {
+      ByonmOrManagedNpmResolver::Byonm(byonm_resolver) => byonm_resolver
+        .resolve_pkg_folder_from_deno_module_req(req, referrer)
+        .map_err(ResolvePkgFolderFromDenoReqError::Byonm),
+      ByonmOrManagedNpmResolver::Managed(managed_resolver) => managed_resolver
+        .resolve_pkg_folder_from_deno_module_req(req, referrer)
+        .map_err(ResolvePkgFolderFromDenoReqError::Managed),
+    }
+  }
 }
 
 pub struct NpmReqResolverOptions<
   TIsBuiltInNodeModuleChecker: IsBuiltInNodeModuleChecker,
   TSys: FsCanonicalize + FsMetadata + FsRead + FsReadDir,
 > {
-  /// The resolver when "bring your own node_modules" is enabled where Deno
-  /// does not setup the node_modules directories automatically, but instead
-  /// uses what already exists on the file system.
-  pub byonm_resolver: Option<ByonmNpmResolverRc<TSys>>,
   pub in_npm_pkg_checker: InNpmPackageCheckerRc,
   pub node_resolver: NodeResolverRc<TIsBuiltInNodeModuleChecker, TSys>,
-  pub npm_req_resolver: CliNpmReqResolverRc,
+  pub npm_resolver: ByonmOrManagedNpmResolver<TSys>,
   pub sys: TSys,
 }
 
@@ -151,11 +164,10 @@ pub struct NpmReqResolver<
   TIsBuiltInNodeModuleChecker: IsBuiltInNodeModuleChecker,
   TSys: FsCanonicalize + FsMetadata + FsRead + FsReadDir,
 > {
-  byonm_resolver: Option<ByonmNpmResolverRc<TSys>>,
   sys: TSys,
   in_npm_pkg_checker: InNpmPackageCheckerRc,
   node_resolver: NodeResolverRc<TIsBuiltInNodeModuleChecker, TSys>,
-  npm_resolver: CliNpmReqResolverRc,
+  npm_resolver: ByonmOrManagedNpmResolver<TSys>,
 }
 
 impl<
@@ -167,11 +179,10 @@ impl<
     options: NpmReqResolverOptions<TIsBuiltInNodeModuleChecker, TSys>,
   ) -> Self {
     Self {
-      byonm_resolver: options.byonm_resolver,
       sys: options.sys,
       in_npm_pkg_checker: options.in_npm_pkg_checker,
       node_resolver: options.node_resolver,
-      npm_resolver: options.npm_req_resolver,
+      npm_resolver: options.npm_resolver,
     }
   }
 
@@ -213,7 +224,7 @@ impl<
     match resolution_result {
       Ok(url) => Ok(url),
       Err(err) => {
-        if self.byonm_resolver.is_some() {
+        if matches!(self.npm_resolver, ByonmOrManagedNpmResolver::Byonm(_)) {
           let package_json_path = package_folder.join("package.json");
           if !self.sys.fs_exists_no_err(&package_json_path) {
             return Err(
@@ -281,7 +292,10 @@ impl<
                         .into_box(),
                       );
                     }
-                    if let Some(byonm_npm_resolver) = &self.byonm_resolver {
+                    if let ByonmOrManagedNpmResolver::Byonm(
+                      byonm_npm_resolver,
+                    ) = &self.npm_resolver
+                    {
                       if byonm_npm_resolver
                         .find_ancestor_package_json_with_dep(
                           package_name,

resolvers/deno/sync.rs

@@ -43,7 +43,13 @@ mod inner {
   }
 
   impl<K: Eq + Hash, V, S: BuildHasher> MaybeDashMap<K, V, S> {
-    pub fn get<'a>(&'a self, key: &K) -> Option<Ref<'a, V>> {
+    pub fn get<'a, Q: Eq + Hash + ?Sized>(
+      &'a self,
+      key: &Q,
+    ) -> Option<Ref<'a, V>>
+    where
+      K: std::borrow::Borrow<Q>,
+    {
       Ref::filter_map(self.0.borrow(), |map| map.get(key)).ok()
     }