From c27248a8f36155d1677385998e36028a01fea02d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 10 Jan 2025 14:48:43 -0500 Subject: [PATCH] refactor: remove `CliNpmReqResolver` trait in deno_resolver (#27616) --- Cargo.lock | 1 + cli/factory.rs | 3 +- cli/lsp/resolver.rs | 3 +- cli/npm/byonm.rs | 26 +++- cli/npm/managed/mod.rs | 176 +++++++++-------------- cli/npm/mod.rs | 19 ++- cli/standalone/mod.rs | 3 +- resolvers/deno/Cargo.toml | 1 + resolvers/deno/npm/byonm.rs | 30 +--- resolvers/deno/npm/managed/common.rs | 16 +-- resolvers/deno/npm/managed/local.rs | 16 ++- resolvers/deno/npm/managed/mod.rs | 176 ++++++++++++++++++++--- resolvers/deno/npm/managed/resolution.rs | 2 +- resolvers/deno/npm/mod.rs | 60 +++++--- resolvers/deno/sync.rs | 8 +- 15 files changed, 324 insertions(+), 216 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 71979d448a..1f13898fe0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2207,6 +2207,7 @@ dependencies = [ "deno_package_json", "deno_path_util", "deno_semver", + "log", "node_resolver", "parking_lot", "sys_traits", diff --git a/cli/factory.rs b/cli/factory.rs index d545fd6ddf..5cc99830bc 100644 --- a/cli/factory.rs +++ b/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 diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 9ae28405bf..57ef2e6a3c 100644 --- a/cli/lsp/resolver.rs +++ b/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(), }))) }) diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs index 2c11a417f3..bd29a6ec72 100644 --- a/cli/npm/byonm.rs +++ b/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) -> Arc { - self - } - fn into_process_state_provider( self: Arc, ) -> Arc { Arc::new(CliByonmWrapper(self)) } - fn into_maybe_byonm(self: Arc) -> Option> { - Some(self) + fn into_byonm_or_managed( + self: Arc, + ) -> ByonmOrManagedNpmResolver { + ByonmOrManagedNpmResolver::Byonm(self) } fn clone_snapshotted(&self) -> Arc { @@ -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 { + self + .resolve_pkg_folder_from_deno_module_req(req, referrer) + .map_err(ResolvePkgFolderFromDenoReqError::Byonm) + } } diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs index 3f4390988a..8d2ede7e67 100644 --- a/cli/npm/managed/mod.rs +++ b/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( - &npm_cache_dir, - &npm_rc, - resolution.clone(), - sys.clone(), - node_modules_dir_path, - ); + let managed_npm_resolver = + Arc::new(ManagedNpmResolver::::new::( + &npm_cache_dir, + &npm_rc, + resolution.clone(), + 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, - fs_resolver: Arc, maybe_lockfile: Option>, registry_info_provider: Arc, + managed_npm_resolver: Arc>, npm_cache: Arc, npm_cache_dir: Arc, npm_install_deps_provider: Arc, @@ -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("", &"") .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, - fs_resolver: Arc, maybe_lockfile: Option>, - registry_info_provider: Arc, + managed_npm_resolver: Arc>, npm_cache: Arc, npm_cache_dir: Arc, npm_install_deps_provider: Arc, npm_rc: Arc, + registry_info_provider: Arc, resolution: Arc, sys: CliSys, tarball_cache: Arc, @@ -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 { - let path = self.fs_resolver.package_folder(pkg_id)?; - let path = canonicalize_path_maybe_not_exists(&self.sys, &path)?; - log::debug!( - "Resolved package folder of {} to {}", - pkg_id.as_serialized(), - path.display() - ); - Ok(path) + self + .managed_npm_resolver + .resolve_pkg_folder_from_pkg_id(pkg_id) } /// Resolves the package id from the provided specifier. @@ -395,7 +375,7 @@ impl ManagedCliNpmResolver { specifier: &ModuleSpecifier, ) -> Result, 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 { - 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 { - 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 { - 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, ) -> Arc { - self - } - - fn into_npm_req_resolver(self: Arc) -> Arc { - self + self.managed_npm_resolver.clone() } fn into_process_state_provider( @@ -735,6 +681,12 @@ impl CliNpmResolver for ManagedCliNpmResolver { self } + fn into_byonm_or_managed( + self: Arc, + ) -> ByonmOrManagedNpmResolver { + ByonmOrManagedNpmResolver::Managed(self.managed_npm_resolver.clone()) + } + fn clone_snapshotted(&self) -> Arc { // 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::::new::( &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 { @@ -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 { + self + .managed_npm_resolver + .resolve_pkg_folder_from_deno_module_req(req, referrer) + .map_err(ResolvePkgFolderFromDenoReqError::Managed) + } } diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 837fe26cf9..052a98e6cf 100644 --- a/cli/npm/mod.rs +++ b/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, ) -> Arc; - fn into_npm_req_resolver(self: Arc) -> Arc; fn into_process_state_provider( self: Arc, ) -> Arc; - fn into_maybe_byonm(self: Arc) -> Option> { - None - } + fn into_byonm_or_managed( + self: Arc, + ) -> ByonmOrManagedNpmResolver; fn clone_snapshotted(&self) -> Arc; @@ -166,6 +167,12 @@ pub trait CliNpmResolver: CliNpmReqResolver { } } + fn resolve_pkg_folder_from_deno_module_req( + &self, + req: &PackageReq, + referrer: &Url, + ) -> Result; + fn root_node_modules_path(&self) -> Option<&Path>; /// Returns a hash returning the state of the npm resolver diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index ecf9805b2d..961e8d6b51 100644 --- a/cli/standalone/mod.rs +++ b/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, diff --git a/resolvers/deno/Cargo.toml b/resolvers/deno/Cargo.toml index 1c603ecaed..534a07ccf2 100644 --- a/resolvers/deno/Cargo.toml +++ b/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 diff --git a/resolvers/deno/npm/byonm.rs b/resolvers/deno/npm/byonm.rs index 710608490a..bf25ba5646 100644 --- a/resolvers/deno/npm/byonm.rs +++ b/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 super::ResolvePkgFolderFromDenoReqError; +use crate::sync::MaybeSend; +use crate::sync::MaybeSync; #[derive(Debug, Error, deno_error::JsError)] pub enum ByonmResolvePkgFolderFromDenoReqError { @@ -377,35 +377,13 @@ impl } } -impl< - Sys: FsCanonicalize - + FsMetadata - + FsRead - + FsReadDir - + Send - + Sync - + std::fmt::Debug, - > CliNpmReqResolver for ByonmNpmResolver -{ - fn resolve_pkg_folder_from_deno_module_req( - &self, - req: &PackageReq, - referrer: &Url, - ) -> Result { - ByonmNpmResolver::resolve_pkg_folder_from_deno_module_req( - self, req, referrer, - ) - .map_err(ResolvePkgFolderFromDenoReqError::Byonm) - } -} - impl< TSys: FsCanonicalize + FsMetadata + FsRead + FsReadDir - + Send - + Sync + + MaybeSend + + MaybeSync + std::fmt::Debug, > NpmPackageFolderResolver for ByonmNpmResolver { diff --git a/resolvers/deno/npm/managed/common.rs b/resolvers/deno/npm/managed/common.rs index 8a523f3ea4..6569a4594f 100644 --- a/resolvers/deno/npm/managed/common.rs +++ b/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; -#[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; - fn package_folder( - &self, - package_id: &NpmPackageId, - ) -> Result { - 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, diff --git a/resolvers/deno/npm/managed/local.rs b/resolvers/deno/npm/managed/local.rs index f23f7bd591..3aa1275d66 100644 --- a/resolvers/deno/npm/managed/local.rs +++ b/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 +impl LocalNpmPackageResolver { #[allow(clippy::too_many_arguments)] @@ -99,8 +101,9 @@ impl } } -impl - NpmPackageFolderResolver for LocalNpmPackageResolver +impl< + TSys: FsCanonicalize + FsMetadata + MaybeSend + MaybeSync + std::fmt::Debug, + > NpmPackageFolderResolver for LocalNpmPackageResolver { fn resolve_package_folder_from_package( &self, @@ -160,8 +163,9 @@ impl } } -impl - NpmPackageFsResolver for LocalNpmPackageResolver +impl< + TSys: FsCanonicalize + FsMetadata + MaybeSend + MaybeSync + std::fmt::Debug, + > NpmPackageFsResolver for LocalNpmPackageResolver { fn node_modules_path(&self) -> Option<&Path> { Some(self.root_node_modules_path.as_ref()) diff --git a/resolvers/deno/npm/managed/mod.rs b/resolvers/deno/npm/managed/mod.rs index d08ee07d6a..a9775ee374 100644 --- a/resolvers/deno/npm/managed/mod.rs +++ b/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; -pub use self::common::NpmPackageFsResolverPackageFolderError; +use self::common::NpmPackageFsResolver; 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< - TSys: FsCanonicalize + FsMetadata + std::fmt::Debug + Send + Sync + 'static, ->( - npm_cache_dir: &NpmCacheDirRc, - npm_rc: &ResolvedNpmRcRc, +#[derive(Debug, thiserror::Error, deno_error::JsError)] +#[error(transparent)] +pub enum ResolvePkgFolderFromPkgIdError { + #[class(inherit)] + #[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 = + crate::sync::MaybeArc>; + +#[derive(Debug)] +pub struct ManagedNpmResolver { + fs_resolver: NpmPackageFsResolverRc, resolution: NpmResolutionRc, sys: TSys, - maybe_node_modules_path: Option, -) -> NpmPackageFsResolverRc { - match maybe_node_modules_path { - Some(node_modules_folder) => new_rc(LocalNpmPackageResolver::new( - resolution, +} + +impl ManagedNpmResolver { + pub fn new< + 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, + ) -> ManagedNpmResolver { + 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 { + 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 { + 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, std::io::Error> { + self + .fs_resolver + .resolve_package_cache_folder_id_from_specifier(specifier) + } +} + +impl + NpmPackageFolderResolver for ManagedNpmResolver +{ + fn resolve_package_folder_from_package( + &self, + specifier: &str, + referrer: &Url, + ) -> Result { + let path = self + .fs_resolver + .resolve_package_folder_from_package(specifier, referrer)?; + log::debug!( + "Resolved {} from {} to {}", + specifier, + referrer, + path.display() + ); + Ok(path) } } diff --git a/resolvers/deno/npm/managed/resolution.rs b/resolvers/deno/npm/managed/resolution.rs index 40ab91202c..8ddd1ac70f 100644 --- a/resolvers/deno/npm/managed/resolution.rs +++ b/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; +pub type NpmResolutionRc = crate::sync::MaybeArc; /// Handles updating and storing npm resolution in memory. /// diff --git a/resolvers/deno/npm/mod.rs b/resolvers/deno/npm/mod.rs index 4b102d6491..26b156d29d 100644 --- a/resolvers/deno/npm/mod.rs +++ b/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}")] - Managed(Box), + #[error(transparent)] + Managed(managed::ManagedResolvePkgFolderFromDenoReqError), #[class(inherit)] #[error(transparent)] - Byonm(#[from] ByonmResolvePkgFolderFromDenoReqError), + Byonm(byonm::ByonmResolvePkgFolderFromDenoReqError), } -#[allow(clippy::disallowed_types)] -pub type CliNpmReqResolverRc = crate::sync::MaybeArc; +#[derive(Debug, Clone)] +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), + Managed(ManagedNpmResolverRc), +} -// todo(dsherret): a temporary trait until we extract -// out the CLI npm resolver into here -pub trait CliNpmReqResolver: Debug + MaybeSend + MaybeSync { - fn resolve_pkg_folder_from_deno_module_req( +impl + ByonmOrManagedNpmResolver +{ + pub fn resolve_pkg_folder_from_deno_module_req( &self, req: &PackageReq, referrer: &Url, - ) -> Result; + ) -> Result { + 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>, pub in_npm_pkg_checker: InNpmPackageCheckerRc, pub node_resolver: NodeResolverRc, - pub npm_req_resolver: CliNpmReqResolverRc, + pub npm_resolver: ByonmOrManagedNpmResolver, pub sys: TSys, } @@ -151,11 +164,10 @@ pub struct NpmReqResolver< TIsBuiltInNodeModuleChecker: IsBuiltInNodeModuleChecker, TSys: FsCanonicalize + FsMetadata + FsRead + FsReadDir, > { - byonm_resolver: Option>, sys: TSys, in_npm_pkg_checker: InNpmPackageCheckerRc, node_resolver: NodeResolverRc, - npm_resolver: CliNpmReqResolverRc, + npm_resolver: ByonmOrManagedNpmResolver, } impl< @@ -167,11 +179,10 @@ impl< options: NpmReqResolverOptions, ) -> 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, diff --git a/resolvers/deno/sync.rs b/resolvers/deno/sync.rs index 1734e84231..af3e290bb2 100644 --- a/resolvers/deno/sync.rs +++ b/resolvers/deno/sync.rs @@ -43,7 +43,13 @@ mod inner { } impl MaybeDashMap { - pub fn get<'a>(&'a self, key: &K) -> Option> { + pub fn get<'a, Q: Eq + Hash + ?Sized>( + &'a self, + key: &Q, + ) -> Option> + where + K: std::borrow::Borrow, + { Ref::filter_map(self.0.borrow(), |map| map.get(key)).ok() }