From 6a85593392b95d113fb248421914209482da4fba Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 30 Sep 2023 12:06:38 -0400 Subject: [PATCH] refactor(npm): create `cli::npm::managed` module (#20740) Creates the `cli::npm::managed` module and starts moving more functionality into it. --- cli/factory.rs | 11 +- cli/graph_util.rs | 11 +- cli/lsp/analysis.rs | 7 +- cli/lsp/documents.rs | 37 ++--- cli/lsp/language_server.rs | 11 +- cli/main.rs | 5 +- cli/npm/{ => managed}/installer.rs | 2 +- cli/npm/{resolvers => managed}/mod.rs | 174 ++++++++++------------ cli/npm/{ => managed}/resolution.rs | 10 +- cli/npm/{ => managed}/resolvers/common.rs | 0 cli/npm/{ => managed}/resolvers/global.rs | 4 +- cli/npm/{ => managed}/resolvers/local.rs | 2 +- cli/npm/managed/resolvers/mod.rs | 50 +++++++ cli/npm/mod.rs | 102 +++++++++++-- cli/npm/registry.rs | 6 - cli/resolver.rs | 117 ++++----------- cli/standalone/binary.rs | 4 +- cli/standalone/mod.rs | 18 ++- cli/tools/run.rs | 10 +- cli/tools/vendor/test.rs | 13 +- 20 files changed, 316 insertions(+), 278 deletions(-) rename cli/npm/{ => managed}/installer.rs (98%) rename cli/npm/{resolvers => managed}/mod.rs (77%) rename cli/npm/{ => managed}/resolution.rs (99%) rename cli/npm/{ => managed}/resolvers/common.rs (100%) rename cli/npm/{ => managed}/resolvers/global.rs (98%) rename cli/npm/{ => managed}/resolvers/local.rs (99%) create mode 100644 cli/npm/managed/resolvers/mod.rs diff --git a/cli/factory.rs b/cli/factory.rs index c56f641394..8a58062c76 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -354,10 +354,12 @@ impl CliFactory { self.options.npm_system_info(), ); Ok(Arc::new(ManagedCliNpmResolver::new( + self.npm_api()?.clone(), fs.clone(), npm_resolution.clone(), npm_fs_resolver, self.maybe_lockfile().as_ref().cloned(), + self.package_json_deps_installer().await?.clone(), )) as Arc) }) .await @@ -428,17 +430,18 @@ impl CliFactory { .resolver .get_or_try_init_async(async { Ok(Arc::new(CliGraphResolver::new( - self.npm_api()?.clone(), - self.npm_resolution().await?.clone(), + if self.options.no_npm() { + None + } else { + Some(self.npm_resolver().await?.clone()) + }, self.package_json_deps_provider().clone(), - self.package_json_deps_installer().await?.clone(), CliGraphResolverOptions { maybe_jsx_import_source_config: self .options .to_maybe_jsx_import_source_config()?, maybe_import_map: self.maybe_import_map().await?.clone(), maybe_vendor_dir: self.options.vendor_dir_path(), - no_npm: self.options.no_npm(), }, ))) }) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index a4a5dcab11..17437ca997 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -322,7 +322,9 @@ impl ModuleGraphBuilder { // ensure an "npm install" is done if the user has explicitly // opted into using a node_modules directory if self.options.node_modules_dir_enablement() == Some(true) { - self.resolver.force_top_level_package_json_install().await?; + if let Some(npm_resolver) = self.npm_resolver.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + } } // add the lockfile redirects to the graph if it's the first time executing @@ -393,10 +395,9 @@ impl ModuleGraphBuilder { if let Some(npm_resolver) = self.npm_resolver.as_managed() { // ensure that the top level package.json is installed if a // specifier was matched in the package.json - self - .resolver - .top_level_package_json_install_if_necessary() - .await?; + if self.resolver.found_package_json_dep() { + npm_resolver.ensure_top_level_package_json_install().await?; + } // resolve the dependencies of any pending dependencies // that were inserted by building the graph diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 1f12fb76b0..8b2560f16c 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -7,7 +7,6 @@ use super::language_server; use super::tsc; use crate::npm::CliNpmResolver; -use crate::npm::NpmResolution; use crate::tools::lint::create_linter; use deno_ast::SourceRange; @@ -162,7 +161,6 @@ fn code_as_string(code: &Option) -> String { pub struct TsResponseImportMapper<'a> { documents: &'a Documents, maybe_import_map: Option<&'a ImportMap>, - npm_resolution: &'a NpmResolution, npm_resolver: &'a dyn CliNpmResolver, } @@ -170,13 +168,11 @@ impl<'a> TsResponseImportMapper<'a> { pub fn new( documents: &'a Documents, maybe_import_map: Option<&'a ImportMap>, - npm_resolution: &'a NpmResolution, npm_resolver: &'a dyn CliNpmResolver, ) -> Self { Self { documents, maybe_import_map, - npm_resolution, npm_resolver, } } @@ -203,8 +199,7 @@ impl<'a> TsResponseImportMapper<'a> { if let Ok(Some(pkg_id)) = npm_resolver.resolve_pkg_id_from_specifier(specifier) { - let pkg_reqs = - self.npm_resolution.resolve_pkg_reqs_from_pkg_id(&pkg_id); + let pkg_reqs = npm_resolver.resolve_pkg_reqs_from_pkg_id(&pkg_id); // check if any pkg reqs match what is found in an import map if !pkg_reqs.is_empty() { let sub_path = self.resolve_package_path(specifier); diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 14c01d13b7..30f419249c 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -17,9 +17,7 @@ use crate::file_fetcher::get_source_from_bytes; use crate::file_fetcher::get_source_from_data_url; use crate::file_fetcher::map_content_type; use crate::lsp::logging::lsp_warn; -use crate::npm::CliNpmRegistryApi; -use crate::npm::NpmResolution; -use crate::npm::PackageJsonDepsInstaller; +use crate::npm::CliNpmResolver; use crate::resolver::CliGraphResolver; use crate::resolver::CliGraphResolverOptions; use crate::util::glob; @@ -860,8 +858,7 @@ pub struct UpdateDocumentConfigOptions<'a> { pub maybe_import_map: Option>, pub maybe_config_file: Option<&'a ConfigFile>, pub maybe_package_json: Option<&'a PackageJson>, - pub npm_registry_api: Arc, - pub npm_resolution: Arc, + pub npm_resolver: Option>, } /// Specify the documents to include on a `documents.documents(...)` call. @@ -917,7 +914,15 @@ impl Documents { file_system_docs: Default::default(), resolver_config_hash: 0, imports: Default::default(), - resolver: Default::default(), + resolver: Arc::new(CliGraphResolver::new( + None, + Arc::new(PackageJsonDepsProvider::default()), + CliGraphResolverOptions { + maybe_jsx_import_source_config: None, + maybe_import_map: None, + maybe_vendor_dir: None, + }, + )), npm_specifier_reqs: Default::default(), has_injected_types_node_package: false, specifier_resolver: Arc::new(SpecifierResolver::new(cache)), @@ -1293,12 +1298,9 @@ impl Documents { ); let deps_provider = Arc::new(PackageJsonDepsProvider::new(maybe_package_json_deps)); - let deps_installer = Arc::new(PackageJsonDepsInstaller::no_op()); self.resolver = Arc::new(CliGraphResolver::new( - options.npm_registry_api, - options.npm_resolution, + options.npm_resolver, deps_provider, - deps_installer, CliGraphResolverOptions { maybe_jsx_import_source_config: maybe_jsx_config, maybe_import_map: options.maybe_import_map, @@ -1306,7 +1308,6 @@ impl Documents { .maybe_config_file .and_then(|c| c.vendor_dir_path()) .as_ref(), - no_npm: false, }, )); self.imports = Arc::new( @@ -1938,7 +1939,6 @@ fn sort_and_remove_non_leaf_dirs(mut dirs: Vec) -> Vec { mod tests { use crate::cache::GlobalHttpCache; use crate::cache::RealDenoCacheEnv; - use crate::npm::NpmResolution; use super::*; use import_map::ImportMap; @@ -2046,13 +2046,6 @@ console.log(b, "hello deno"); #[test] fn test_documents_refresh_dependencies_config_change() { - let npm_registry_api = Arc::new(CliNpmRegistryApi::new_uninitialized()); - let npm_resolution = Arc::new(NpmResolution::from_serialized( - npm_registry_api.clone(), - None, - None, - )); - // it should never happen that a user of this API causes this to happen, // but we'll guard against it anyway let temp_dir = TempDir::new(); @@ -2089,8 +2082,7 @@ console.log(b, "hello deno"); maybe_import_map: Some(Arc::new(import_map)), maybe_config_file: None, maybe_package_json: None, - npm_registry_api: npm_registry_api.clone(), - npm_resolution: npm_resolution.clone(), + npm_resolver: None, }); // open the document @@ -2131,8 +2123,7 @@ console.log(b, "hello deno"); maybe_import_map: Some(Arc::new(import_map)), maybe_config_file: None, maybe_package_json: None, - npm_registry_api, - npm_resolution, + npm_resolver: None, }); // check the document's dependencies diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 9b3500361f..7bed7b1bff 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -109,6 +109,7 @@ use crate::npm::ManagedCliNpmResolver; use crate::npm::NpmCache; use crate::npm::NpmCacheDir; use crate::npm::NpmResolution; +use crate::npm::PackageJsonDepsInstaller; use crate::tools::fmt::format_file; use crate::tools::fmt::format_parsed_source; use crate::util::fs::remove_dir_all_if_exists; @@ -509,7 +510,7 @@ fn create_npm_resolver_and_resolution( maybe_snapshot: Option, ) -> (Arc, Arc) { let resolution = Arc::new(NpmResolution::from_serialized( - api, + api.clone(), maybe_snapshot, // Don't provide the lockfile. We don't want these resolvers // updating it. Only the cache request should update the lockfile. @@ -527,12 +528,14 @@ fn create_npm_resolver_and_resolution( ); ( Arc::new(ManagedCliNpmResolver::new( + api, fs, resolution.clone(), fs_resolver, // Don't provide the lockfile. We don't want these resolvers // updating it. Only the cache request should update the lockfile. None, + Arc::new(PackageJsonDepsInstaller::no_op()), )), resolution, ) @@ -804,6 +807,7 @@ impl Inner { )); let node_fs = Arc::new(deno_fs::RealFs); let npm_resolver = Arc::new(ManagedCliNpmResolver::new( + self.npm.api.clone(), node_fs.clone(), npm_resolution.clone(), create_npm_fs_resolver( @@ -816,6 +820,7 @@ impl Inner { NpmSystemInfo::default(), ), self.config.maybe_lockfile().cloned(), + Arc::new(PackageJsonDepsInstaller::no_op()), )); let node_resolver = Arc::new(NodeResolver::new(node_fs, npm_resolver.clone())); @@ -1366,8 +1371,7 @@ impl Inner { maybe_import_map: self.maybe_import_map.clone(), maybe_config_file: self.config.maybe_config_file(), maybe_package_json: self.maybe_package_json.as_ref(), - npm_registry_api: self.npm.api.clone(), - npm_resolution: self.npm.resolution.clone(), + npm_resolver: Some(self.npm.resolver.clone()), }); // refresh the npm specifiers because it might have discovered @@ -2161,7 +2165,6 @@ impl Inner { TsResponseImportMapper::new( &self.documents, self.maybe_import_map.as_deref(), - &self.npm.resolution, self.npm.resolver.as_ref(), ) } diff --git a/cli/main.rs b/cli/main.rs index 98a2e5d480..1ccd694ee9 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -76,7 +76,10 @@ impl SubcommandOutput for Result<(), std::io::Error> { fn spawn_subcommand + 'static, T: SubcommandOutput>( f: F, ) -> JoinHandle> { - deno_core::unsync::spawn(f.map(|r| r.output())) + // the boxed_local() is important in order to get windows to not blow the stack in debug + deno_core::unsync::spawn( + async move { f.map(|r| r.output()).await }.boxed_local(), + ) } async fn run_subcommand(flags: Flags) -> Result { diff --git a/cli/npm/installer.rs b/cli/npm/managed/installer.rs similarity index 98% rename from cli/npm/installer.rs rename to cli/npm/managed/installer.rs index 8f3db05319..21285c3d7f 100644 --- a/cli/npm/installer.rs +++ b/cli/npm/managed/installer.rs @@ -13,7 +13,7 @@ use deno_semver::package::PackageReq; use crate::args::PackageJsonDepsProvider; use crate::util::sync::AtomicFlag; -use super::CliNpmRegistryApi; +use super::super::CliNpmRegistryApi; use super::NpmResolution; #[derive(Debug)] diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/managed/mod.rs similarity index 77% rename from cli/npm/resolvers/mod.rs rename to cli/npm/managed/mod.rs index 07a122a3e9..c5ba3d3aff 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/managed/mod.rs @@ -1,9 +1,5 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -mod common; -mod global; -mod local; - use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; @@ -14,10 +10,13 @@ use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; use deno_core::serde_json; use deno_core::url::Url; +use deno_graph::NpmPackageReqResolution; +use deno_npm::registry::NpmRegistryApi; use deno_npm::resolution::NpmResolutionSnapshot; use deno_npm::resolution::PackageReqNotFoundError; use deno_npm::resolution::SerializedNpmResolutionSnapshot; use deno_npm::NpmPackageId; +use deno_npm::NpmResolutionPackage; use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; @@ -28,19 +27,24 @@ use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageNv; use deno_semver::package::PackageNvReference; use deno_semver::package::PackageReq; -use global::GlobalNpmPackageResolver; use serde::Deserialize; use serde::Serialize; use crate::args::Lockfile; use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs; -use crate::util::progress_bar::ProgressBar; -use self::local::LocalNpmPackageResolver; -use super::resolution::NpmResolution; -use super::NpmCache; +use super::CliNpmRegistryApi; +use super::CliNpmResolver; +use super::InnerCliNpmResolverRef; -pub use self::common::NpmPackageFsResolver; +pub use self::installer::PackageJsonDepsInstaller; +pub use self::resolution::NpmResolution; +pub use self::resolvers::create_npm_fs_resolver; +pub use self::resolvers::NpmPackageFsResolver; + +mod installer; +mod resolution; +mod resolvers; /// State provided to the process via an environment variable. #[derive(Clone, Debug, Serialize, Deserialize)] @@ -49,97 +53,46 @@ pub struct NpmProcessState { pub local_node_modules_path: Option, } -pub enum InnerCliNpmResolverRef<'a> { - Managed(&'a ManagedCliNpmResolver), - #[allow(dead_code)] - Byonm(&'a ByonmCliNpmResolver), -} - -pub trait CliNpmResolver: NpmResolver { - fn into_npm_resolver(self: Arc) -> Arc; - - fn root_dir_url(&self) -> &Url; - - fn as_inner(&self) -> InnerCliNpmResolverRef; - - fn as_managed(&self) -> Option<&ManagedCliNpmResolver> { - match self.as_inner() { - InnerCliNpmResolverRef::Managed(inner) => Some(inner), - InnerCliNpmResolverRef::Byonm(_) => None, - } - } - - fn node_modules_path(&self) -> Option; - - /// Checks if the provided package req's folder is cached. - fn is_pkg_req_folder_cached(&self, req: &PackageReq) -> bool; - - fn resolve_pkg_nv_ref_from_pkg_req_ref( - &self, - req_ref: &NpmPackageReqReference, - ) -> Result; - - /// Resolve the root folder of the package the provided specifier is in. - /// - /// This will error when the provided specifier is not in an npm package. - fn resolve_pkg_folder_from_specifier( - &self, - specifier: &ModuleSpecifier, - ) -> Result, AnyError>; - - fn resolve_pkg_folder_from_deno_module_req( - &self, - req: &PackageReq, - ) -> Result; - - fn resolve_pkg_folder_from_deno_module( - &self, - nv: &PackageNv, - ) -> Result; - - /// Gets the state of npm for the process. - fn get_npm_process_state(&self) -> String; - - // todo(#18967): should instead return a hash state of the resolver - // or perhaps this could be non-BYONM only and byonm always runs deno check - fn package_reqs(&self) -> HashMap; -} - -// todo(dsherret): implement this -pub struct ByonmCliNpmResolver; - /// 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 { + api: Arc, fs: Arc, fs_resolver: Arc, resolution: Arc, maybe_lockfile: Option>>, + package_json_deps_installer: Arc, } impl std::fmt::Debug for ManagedCliNpmResolver { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("ManagedNpmResolver") + .field("api", &"") .field("fs", &"") .field("fs_resolver", &"") .field("resolution", &"") .field("maybe_lockfile", &"") + .field("package_json_deps_installer", &"") .finish() } } impl ManagedCliNpmResolver { pub fn new( + api: Arc, fs: Arc, resolution: Arc, fs_resolver: Arc, maybe_lockfile: Option>>, + package_json_deps_installer: Arc, ) -> Self { Self { + api, fs, fs_resolver, resolution, maybe_lockfile, + package_json_deps_installer, } } @@ -180,6 +133,13 @@ impl ManagedCliNpmResolver { )) } + pub fn resolve_pkg_reqs_from_pkg_id( + &self, + id: &NpmPackageId, + ) -> Vec { + self.resolution.resolve_pkg_reqs_from_pkg_id(id) + } + /// Attempts to get the package size in bytes. pub fn package_size( &self, @@ -189,6 +149,13 @@ impl ManagedCliNpmResolver { Ok(crate::util::fs::dir_size(&package_folder)?) } + pub fn all_system_packages( + &self, + system_info: &NpmSystemInfo, + ) -> Vec { + self.resolution.all_system_packages(system_info) + } + /// Adds package requirements to the resolver and ensures everything is setup. pub async fn add_package_reqs( &self, @@ -251,6 +218,28 @@ impl ManagedCliNpmResolver { ) -> Result { self.resolution.resolve_pkg_id_from_pkg_req(req) } + + pub async fn ensure_top_level_package_json_install( + &self, + ) -> Result<(), AnyError> { + self + .package_json_deps_installer + .ensure_top_level_install() + .await + } + + pub async fn cache_package_info( + &self, + package_name: &str, + ) -> Result<(), AnyError> { + // this will internally cache the package information + self + .api + .package_info(package_name) + .await + .map(|_| ()) + .map_err(|err| err.into()) + } } impl NpmResolver for ManagedCliNpmResolver { @@ -316,6 +305,24 @@ impl CliNpmResolver for ManagedCliNpmResolver { .unwrap_or(false) } + fn resolve_npm_for_deno_graph( + &self, + pkg_req: &PackageReq, + ) -> NpmPackageReqResolution { + let result = self.resolution.resolve_pkg_req_as_pending(pkg_req); + match result { + Ok(nv) => NpmPackageReqResolution::Ok(nv), + Err(err) => { + if self.api.mark_force_reload() { + log::debug!("Restarting npm specifier resolution to check for new registry information. Error: {:#}", err); + NpmPackageReqResolution::ReloadRegistryInfo(err.into()) + } else { + NpmPackageReqResolution::Err(err.into()) + } + } + } + } + fn resolve_pkg_nv_ref_from_pkg_req_ref( &self, req_ref: &NpmPackageReqReference, @@ -385,32 +392,3 @@ impl CliNpmResolver for ManagedCliNpmResolver { self.resolution.package_reqs() } } - -pub fn create_npm_fs_resolver( - fs: Arc, - cache: Arc, - progress_bar: &ProgressBar, - registry_url: Url, - resolution: Arc, - maybe_node_modules_path: Option, - system_info: NpmSystemInfo, -) -> Arc { - match maybe_node_modules_path { - Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new( - fs, - cache, - progress_bar.clone(), - registry_url, - node_modules_folder, - resolution, - system_info, - )), - None => Arc::new(GlobalNpmPackageResolver::new( - fs, - cache, - registry_url, - resolution, - system_info, - )), - } -} diff --git a/cli/npm/resolution.rs b/cli/npm/managed/resolution.rs similarity index 99% rename from cli/npm/resolution.rs rename to cli/npm/managed/resolution.rs index 3e76d5e85d..05c1227a7c 100644 --- a/cli/npm/resolution.rs +++ b/cli/npm/managed/resolution.rs @@ -34,7 +34,7 @@ use deno_semver::VersionReq; use crate::args::Lockfile; use crate::util::sync::TaskQueue; -use super::registry::CliNpmRegistryApi; +use super::super::registry::CliNpmRegistryApi; /// Handles updating and storing npm resolution in memory where the underlying /// snapshot can be updated concurrently. Additionally handles updating the lockfile @@ -221,6 +221,8 @@ impl NpmResolution { .map(|pkg| pkg.id.clone()) } + // todo: NEXT + /// Resolves a package requirement for deno graph. This should only be /// called by deno_graph's NpmResolver or for resolving packages in /// a package.json @@ -273,10 +275,14 @@ impl NpmResolution { .all_system_packages_partitioned(system_info) } + // todo: NEXT + pub fn has_packages(&self) -> bool { !self.snapshot.read().is_empty() } + // todo: NEXT + pub fn snapshot(&self) -> NpmResolutionSnapshot { self.snapshot.read().clone() } @@ -287,6 +293,8 @@ impl NpmResolution { self.snapshot.read().as_valid_serialized() } + // todo: NEXT + pub fn serialized_valid_snapshot_for_system( &self, system_info: &NpmSystemInfo, diff --git a/cli/npm/resolvers/common.rs b/cli/npm/managed/resolvers/common.rs similarity index 100% rename from cli/npm/resolvers/common.rs rename to cli/npm/managed/resolvers/common.rs diff --git a/cli/npm/resolvers/global.rs b/cli/npm/managed/resolvers/global.rs similarity index 98% rename from cli/npm/resolvers/global.rs rename to cli/npm/managed/resolvers/global.rs index e1144f6102..25db62f732 100644 --- a/cli/npm/resolvers/global.rs +++ b/cli/npm/managed/resolvers/global.rs @@ -20,10 +20,10 @@ use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolutionMode; -use crate::npm::resolution::NpmResolution; -use crate::npm::resolvers::common::cache_packages; use crate::npm::NpmCache; +use super::super::resolution::NpmResolution; +use super::common::cache_packages; use super::common::types_package_name; use super::common::NpmPackageFsResolver; use super::common::RegistryReadPermissionChecker; diff --git a/cli/npm/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs similarity index 99% rename from cli/npm/resolvers/local.rs rename to cli/npm/managed/resolvers/local.rs index afa95e756a..57170eccdf 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -42,11 +42,11 @@ use serde::Deserialize; use serde::Serialize; use crate::npm::cache::mixed_case_package_name_encode; -use crate::npm::resolution::NpmResolution; use crate::npm::NpmCache; use crate::util::fs::copy_dir_recursive; use crate::util::fs::hard_link_dir_recursive; +use super::super::resolution::NpmResolution; use super::common::types_package_name; use super::common::NpmPackageFsResolver; use super::common::RegistryReadPermissionChecker; diff --git a/cli/npm/managed/resolvers/mod.rs b/cli/npm/managed/resolvers/mod.rs new file mode 100644 index 0000000000..b6d96c4af3 --- /dev/null +++ b/cli/npm/managed/resolvers/mod.rs @@ -0,0 +1,50 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + +mod common; +mod global; +mod local; + +use std::path::PathBuf; +use std::sync::Arc; + +use deno_core::url::Url; +use deno_npm::NpmSystemInfo; +use deno_runtime::deno_fs::FileSystem; + +use crate::npm::NpmCache; +use crate::util::progress_bar::ProgressBar; + +pub use self::common::NpmPackageFsResolver; +use self::global::GlobalNpmPackageResolver; +use self::local::LocalNpmPackageResolver; + +use super::NpmResolution; + +pub fn create_npm_fs_resolver( + fs: Arc, + cache: Arc, + progress_bar: &ProgressBar, + registry_url: Url, + resolution: Arc, + maybe_node_modules_path: Option, + system_info: NpmSystemInfo, +) -> Arc { + match maybe_node_modules_path { + Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new( + fs, + cache, + progress_bar.clone(), + registry_url, + node_modules_folder, + resolution, + system_info, + )), + None => Arc::new(GlobalNpmPackageResolver::new( + fs, + cache, + registry_url, + resolution, + system_info, + )), + } +} diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 1b6ec243c4..114bf15f2d 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -1,20 +1,100 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +mod managed; + +// todo(#18967): move the cache, registry, and tarball into the managed folder mod cache; -mod installer; mod registry; -mod resolution; -mod resolvers; mod tarball; +use std::collections::HashMap; +use std::path::PathBuf; +use std::sync::Arc; + +use deno_ast::ModuleSpecifier; +use deno_core::error::AnyError; +use deno_core::url::Url; +use deno_graph::NpmPackageReqResolution; +use deno_npm::resolution::PackageReqNotFoundError; +use deno_runtime::deno_node::NpmResolver; + pub use cache::NpmCache; pub use cache::NpmCacheDir; -pub use installer::PackageJsonDepsInstaller; +use deno_semver::npm::NpmPackageNvReference; +use deno_semver::npm::NpmPackageReqReference; +use deno_semver::package::PackageNv; +use deno_semver::package::PackageReq; +pub use managed::create_npm_fs_resolver; +pub use managed::ManagedCliNpmResolver; +pub use managed::NpmPackageFsResolver; +pub use managed::NpmProcessState; +pub use managed::NpmResolution; +pub use managed::PackageJsonDepsInstaller; pub use registry::CliNpmRegistryApi; -pub use resolution::NpmResolution; -pub use resolvers::create_npm_fs_resolver; -pub use resolvers::CliNpmResolver; -pub use resolvers::InnerCliNpmResolverRef; -pub use resolvers::ManagedCliNpmResolver; -pub use resolvers::NpmPackageFsResolver; -pub use resolvers::NpmProcessState; + +pub enum InnerCliNpmResolverRef<'a> { + Managed(&'a ManagedCliNpmResolver), + #[allow(dead_code)] + Byonm(&'a ByonmCliNpmResolver), +} + +pub trait CliNpmResolver: NpmResolver { + fn into_npm_resolver(self: Arc) -> Arc; + + fn root_dir_url(&self) -> &Url; + + fn as_inner(&self) -> InnerCliNpmResolverRef; + + fn as_managed(&self) -> Option<&ManagedCliNpmResolver> { + match self.as_inner() { + InnerCliNpmResolverRef::Managed(inner) => Some(inner), + InnerCliNpmResolverRef::Byonm(_) => None, + } + } + + fn node_modules_path(&self) -> Option; + + /// Checks if the provided package req's folder is cached. + fn is_pkg_req_folder_cached(&self, req: &PackageReq) -> bool; + + /// Resolves a package requirement for deno graph. This should only be + /// called by deno_graph's NpmResolver or for resolving packages in + /// a package.json + fn resolve_npm_for_deno_graph( + &self, + pkg_req: &PackageReq, + ) -> NpmPackageReqResolution; + + fn resolve_pkg_nv_ref_from_pkg_req_ref( + &self, + req_ref: &NpmPackageReqReference, + ) -> Result; + + /// Resolve the root folder of the package the provided specifier is in. + /// + /// This will error when the provided specifier is not in an npm package. + fn resolve_pkg_folder_from_specifier( + &self, + specifier: &ModuleSpecifier, + ) -> Result, AnyError>; + + fn resolve_pkg_folder_from_deno_module_req( + &self, + req: &PackageReq, + ) -> Result; + + fn resolve_pkg_folder_from_deno_module( + &self, + nv: &PackageNv, + ) -> Result; + + /// Gets the state of npm for the process. + fn get_npm_process_state(&self) -> String; + + // todo(#18967): should instead return a hash state of the resolver + // or perhaps this could be non-BYONM only and byonm always runs deno check + fn package_reqs(&self) -> HashMap; +} + +// todo(#18967): implement this +pub struct ByonmCliNpmResolver; diff --git a/cli/npm/registry.rs b/cli/npm/registry.rs index e960d926f0..61eb4123dd 100644 --- a/cli/npm/registry.rs +++ b/cli/npm/registry.rs @@ -75,12 +75,6 @@ impl CliNpmRegistryApi { }))) } - /// Creates an npm registry API that will be uninitialized. This is - /// useful for tests or for initializing the LSP. - pub fn new_uninitialized() -> Self { - Self(None) - } - /// Clears the internal memory cache. pub fn clear_memory_cache(&self) { self.inner().clear_memory_cache(); diff --git a/cli/resolver.rs b/cli/resolver.rs index 1cbd3356e0..cc1059aed6 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -12,7 +12,6 @@ use deno_graph::source::NpmResolver; use deno_graph::source::Resolver; use deno_graph::source::UnknownBuiltInNodeModuleError; use deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE; -use deno_npm::registry::NpmRegistryApi; use deno_runtime::deno_node::is_builtin_node_module; use deno_semver::package::PackageReq; use import_map::ImportMap; @@ -22,9 +21,7 @@ use std::sync::Arc; use crate::args::package_json::PackageJsonDeps; use crate::args::JsxImportSourceConfig; use crate::args::PackageJsonDepsProvider; -use crate::npm::CliNpmRegistryApi; -use crate::npm::NpmResolution; -use crate::npm::PackageJsonDepsInstaller; +use crate::npm::CliNpmResolver; use crate::util::sync::AtomicFlag; /// Result of checking if a specifier is mapped via @@ -104,53 +101,20 @@ pub struct CliGraphResolver { maybe_default_jsx_import_source: Option, maybe_jsx_import_source_module: Option, maybe_vendor_specifier: Option, - no_npm: bool, - npm_registry_api: Arc, - npm_resolution: Arc, - package_json_deps_installer: Arc, + npm_resolver: Option>, found_package_json_dep_flag: Arc, } -impl Default for CliGraphResolver { - fn default() -> Self { - // This is not ideal, but necessary for the LSP. In the future, we should - // refactor the LSP and force this to be initialized. - let npm_registry_api = Arc::new(CliNpmRegistryApi::new_uninitialized()); - let npm_resolution = Arc::new(NpmResolution::from_serialized( - npm_registry_api.clone(), - None, - None, - )); - Self { - mapped_specifier_resolver: MappedSpecifierResolver { - maybe_import_map: Default::default(), - package_json_deps_provider: Default::default(), - }, - maybe_default_jsx_import_source: None, - maybe_jsx_import_source_module: None, - maybe_vendor_specifier: None, - no_npm: false, - npm_registry_api, - npm_resolution, - package_json_deps_installer: Default::default(), - found_package_json_dep_flag: Default::default(), - } - } -} - pub struct CliGraphResolverOptions<'a> { pub maybe_jsx_import_source_config: Option, pub maybe_import_map: Option>, pub maybe_vendor_dir: Option<&'a PathBuf>, - pub no_npm: bool, } impl CliGraphResolver { pub fn new( - npm_registry_api: Arc, - npm_resolution: Arc, + npm_resolver: Option>, package_json_deps_provider: Arc, - package_json_deps_installer: Arc, options: CliGraphResolverOptions, ) -> Self { Self { @@ -168,10 +132,7 @@ impl CliGraphResolver { maybe_vendor_specifier: options .maybe_vendor_dir .and_then(|v| ModuleSpecifier::from_directory_path(v).ok()), - no_npm: options.no_npm, - npm_registry_api, - npm_resolution, - package_json_deps_installer, + npm_resolver, found_package_json_dep_flag: Default::default(), } } @@ -184,22 +145,8 @@ impl CliGraphResolver { self } - pub async fn force_top_level_package_json_install( - &self, - ) -> Result<(), AnyError> { - self - .package_json_deps_installer - .ensure_top_level_install() - .await - } - - pub async fn top_level_package_json_install_if_necessary( - &self, - ) -> Result<(), AnyError> { - if self.found_package_json_dep_flag.is_raised() { - self.force_top_level_package_json_install().await?; - } - Ok(()) + pub fn found_package_json_dep(&self) -> bool { + self.found_package_json_dep_flag.is_raised() } } @@ -295,41 +242,33 @@ impl NpmResolver for CliGraphResolver { &self, package_name: &str, ) -> LocalBoxFuture<'static, Result<(), AnyError>> { - if self.no_npm { - // return it succeeded and error at the import site below - return Box::pin(future::ready(Ok(()))); + match &self.npm_resolver { + Some(npm_resolver) if npm_resolver.as_managed().is_some() => { + let package_name = package_name.to_string(); + let npm_resolver = npm_resolver.clone(); + async move { + if let Some(managed) = npm_resolver.as_managed() { + managed.cache_package_info(&package_name).await?; + } + Ok(()) + } + .boxed() + } + _ => { + // return it succeeded and error at the import site below + Box::pin(future::ready(Ok(()))) + } } - // this will internally cache the package information - let package_name = package_name.to_string(); - let api = self.npm_registry_api.clone(); - async move { - api - .package_info(&package_name) - .await - .map(|_| ()) - .map_err(|err| err.into()) - } - .boxed() } fn resolve_npm(&self, package_req: &PackageReq) -> NpmPackageReqResolution { - if self.no_npm { - return NpmPackageReqResolution::Err(anyhow!( - "npm specifiers were requested; but --no-npm is specified" - )); - } - - let result = self.npm_resolution.resolve_pkg_req_as_pending(package_req); - match result { - Ok(nv) => NpmPackageReqResolution::Ok(nv), - Err(err) => { - if self.npm_registry_api.mark_force_reload() { - log::debug!("Restarting npm specifier resolution to check for new registry information. Error: {:#}", err); - NpmPackageReqResolution::ReloadRegistryInfo(err.into()) - } else { - NpmPackageReqResolution::Err(err.into()) - } + match &self.npm_resolver { + Some(npm_resolver) => { + npm_resolver.resolve_npm_for_deno_graph(package_req) } + None => NpmPackageReqResolution::Err(anyhow!( + "npm specifiers were requested; but --no-npm is specified" + )), } } } diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 38fb3b08f2..afe5a1b573 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -558,9 +558,7 @@ impl<'a> DenoCompileBinaryWriter<'a> { let registry_url = self.npm_api.base_url(); let root_path = self.npm_cache.registry_folder(registry_url); let mut builder = VfsBuilder::new(root_path)?; - for package in self - .npm_resolution - .all_system_packages(&self.npm_system_info) + for package in npm_resolver.all_system_packages(&self.npm_system_info) { let folder = npm_resolver.resolve_pkg_folder_from_pkg_id(&package.id)?; diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 30bbd7f8b9..442334e398 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -21,6 +21,7 @@ use crate::npm::ManagedCliNpmResolver; use crate::npm::NpmCache; use crate::npm::NpmCacheDir; use crate::npm::NpmResolution; +use crate::npm::PackageJsonDepsInstaller; use crate::resolver::MappedSpecifierResolver; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; @@ -366,11 +367,23 @@ pub async fn run( node_modules_path, NpmSystemInfo::default(), ); + let package_json_deps_provider = Arc::new(PackageJsonDepsProvider::new( + metadata + .package_json_deps + .map(|serialized| serialized.into_deps()), + )); + let package_json_installer = Arc::new(PackageJsonDepsInstaller::new( + package_json_deps_provider.clone(), + npm_api.clone(), + npm_resolution.clone(), + )); let npm_resolver = Arc::new(ManagedCliNpmResolver::new( + npm_api.clone(), fs.clone(), npm_resolution.clone(), npm_fs_resolver, None, + package_json_installer, )) as Arc; let node_resolver = Arc::new(NodeResolver::new( fs.clone(), @@ -387,11 +400,6 @@ pub async fn run( node_resolver.clone(), npm_resolver.clone().into_npm_resolver(), )); - let package_json_deps_provider = Arc::new(PackageJsonDepsProvider::new( - metadata - .package_json_deps - .map(|serialized| serialized.into_deps()), - )); let maybe_import_map = metadata.maybe_import_map.map(|(base, source)| { Arc::new(parse_from_json(&base, &source).unwrap().import_map) }); diff --git a/cli/tools/run.rs b/cli/tools/run.rs index 6ded628ea6..5fb31a4ad7 100644 --- a/cli/tools/run.rs +++ b/cli/tools/run.rs @@ -186,13 +186,11 @@ pub async fn eval_command( async fn maybe_npm_install(factory: &CliFactory) -> Result<(), AnyError> { // ensure an "npm install" is done if the user has explicitly - // opted into using a node_modules directory + // opted into using a managed node_modules directory if factory.cli_options().node_modules_dir_enablement() == Some(true) { - factory - .package_json_deps_installer() - .await? - .ensure_top_level_install() - .await?; + if let Some(npm_resolver) = factory.npm_resolver().await?.as_managed() { + npm_resolver.ensure_top_level_package_json_install().await?; + } } Ok(()) } diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index a8b83bb917..e13b8579b1 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -23,8 +23,6 @@ use import_map::ImportMap; use crate::args::JsxImportSourceConfig; use crate::cache::ParsedSourceCache; -use crate::npm::CliNpmRegistryApi; -use crate::npm::NpmResolution; use crate::resolver::CliGraphResolver; use crate::resolver::CliGraphResolverOptions; @@ -295,22 +293,13 @@ fn build_resolver( maybe_jsx_import_source_config: Option, original_import_map: Option, ) -> CliGraphResolver { - let npm_registry_api = Arc::new(CliNpmRegistryApi::new_uninitialized()); - let npm_resolution = Arc::new(NpmResolution::from_serialized( - npm_registry_api.clone(), - None, - None, - )); CliGraphResolver::new( - npm_registry_api, - npm_resolution, - Default::default(), + None, Default::default(), CliGraphResolverOptions { maybe_jsx_import_source_config, maybe_import_map: original_import_map.map(Arc::new), maybe_vendor_dir: None, - no_npm: false, }, ) }