diff --git a/cli/factory.rs b/cli/factory.rs index 6acf248a88..c56f641394 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -32,6 +32,7 @@ use crate::node::CliNodeCodeTranslator; use crate::npm::create_npm_fs_resolver; use crate::npm::CliNpmRegistryApi; use crate::npm::CliNpmResolver; +use crate::npm::ManagedCliNpmResolver; use crate::npm::NpmCache; use crate::npm::NpmCacheDir; use crate::npm::NpmPackageFsResolver; @@ -158,7 +159,7 @@ struct CliFactoryServices { node_resolver: Deferred>, npm_api: Deferred>, npm_cache: Deferred>, - npm_resolver: Deferred>, + npm_resolver: Deferred>, npm_resolution: Deferred>, package_json_deps_provider: Deferred>, package_json_deps_installer: Deferred>, @@ -334,7 +335,9 @@ impl CliFactory { .await } - pub async fn npm_resolver(&self) -> Result<&Arc, AnyError> { + pub async fn npm_resolver( + &self, + ) -> Result<&Arc, AnyError> { self .services .npm_resolver @@ -350,12 +353,12 @@ impl CliFactory { self.options.node_modules_dir_path(), self.options.npm_system_info(), ); - Ok(Arc::new(CliNpmResolver::new( + Ok(Arc::new(ManagedCliNpmResolver::new( fs.clone(), npm_resolution.clone(), npm_fs_resolver, self.maybe_lockfile().as_ref().cloned(), - ))) + )) as Arc) }) .await } @@ -491,7 +494,7 @@ impl CliFactory { .get_or_try_init_async(async { Ok(Arc::new(NodeResolver::new( self.fs().clone(), - self.npm_resolver().await?.clone(), + self.npm_resolver().await?.clone().into_npm_resolver(), ))) }) .await @@ -514,7 +517,7 @@ impl CliFactory { cjs_esm_analyzer, self.fs().clone(), self.node_resolver().await?.clone(), - self.npm_resolver().await?.clone(), + self.npm_resolver().await?.clone().into_npm_resolver(), ))) }) .await @@ -613,7 +616,7 @@ impl CliFactory { self.npm_api()?, self.npm_cache()?, self.npm_resolution().await?, - self.npm_resolver().await?, + self.npm_resolver().await?.as_ref(), self.options.npm_system_info(), self.package_json_deps_provider(), )) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index e567bebb58..a4a5dcab11 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -171,7 +171,7 @@ pub fn graph_lock_or_exit(graph: &ModuleGraph, lockfile: &mut Lockfile) { pub struct ModuleGraphBuilder { options: Arc, resolver: Arc, - npm_resolver: Arc, + npm_resolver: Arc, parsed_source_cache: Arc, lockfile: Option>>, maybe_file_watcher_reporter: Option, @@ -186,7 +186,7 @@ impl ModuleGraphBuilder { pub fn new( options: Arc, resolver: Arc, - npm_resolver: Arc, + npm_resolver: Arc, parsed_source_cache: Arc, lockfile: Option>>, maybe_file_watcher_reporter: Option, @@ -245,11 +245,10 @@ impl ModuleGraphBuilder { ) .await?; - if graph.has_node_specifier && self.options.type_check_mode().is_true() { - self - .npm_resolver - .inject_synthetic_types_node_package() - .await?; + if let Some(npm_resolver) = self.npm_resolver.as_managed() { + if graph.has_node_specifier && self.options.type_check_mode().is_true() { + npm_resolver.inject_synthetic_types_node_package().await?; + } } Ok(graph) @@ -391,16 +390,18 @@ impl ModuleGraphBuilder { } } - // 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 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?; - // resolve the dependencies of any pending dependencies - // that were inserted by building the graph - self.npm_resolver.resolve_pending().await?; + // resolve the dependencies of any pending dependencies + // that were inserted by building the graph + npm_resolver.resolve_pending().await?; + } Ok(()) } diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 1b11deca89..1f12fb76b0 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -163,7 +163,7 @@ pub struct TsResponseImportMapper<'a> { documents: &'a Documents, maybe_import_map: Option<&'a ImportMap>, npm_resolution: &'a NpmResolution, - npm_resolver: &'a CliNpmResolver, + npm_resolver: &'a dyn CliNpmResolver, } impl<'a> TsResponseImportMapper<'a> { @@ -171,7 +171,7 @@ impl<'a> TsResponseImportMapper<'a> { documents: &'a Documents, maybe_import_map: Option<&'a ImportMap>, npm_resolution: &'a NpmResolution, - npm_resolver: &'a CliNpmResolver, + npm_resolver: &'a dyn CliNpmResolver, ) -> Self { Self { documents, @@ -198,39 +198,41 @@ impl<'a> TsResponseImportMapper<'a> { } } - if self.npm_resolver.in_npm_package(specifier) { - if let Ok(Some(pkg_id)) = - self.npm_resolver.resolve_pkg_id_from_specifier(specifier) - { - let pkg_reqs = - self.npm_resolution.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); - if let Some(import_map) = self.maybe_import_map { - for pkg_req in &pkg_reqs { - let paths = vec![ - concat_npm_specifier("npm:", pkg_req, sub_path.as_deref()), - concat_npm_specifier("npm:/", pkg_req, sub_path.as_deref()), - ]; - for path in paths { - if let Some(mapped_path) = ModuleSpecifier::parse(&path) - .ok() - .and_then(|s| import_map.lookup(&s, referrer)) - { - return Some(mapped_path); + if let Some(npm_resolver) = self.npm_resolver.as_managed() { + if npm_resolver.in_npm_package(specifier) { + 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); + // 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); + if let Some(import_map) = self.maybe_import_map { + for pkg_req in &pkg_reqs { + let paths = vec![ + concat_npm_specifier("npm:", pkg_req, sub_path.as_deref()), + concat_npm_specifier("npm:/", pkg_req, sub_path.as_deref()), + ]; + for path in paths { + if let Some(mapped_path) = ModuleSpecifier::parse(&path) + .ok() + .and_then(|s| import_map.lookup(&s, referrer)) + { + return Some(mapped_path); + } } } } - } - // if not found in the import map, return the first pkg req - if let Some(pkg_req) = pkg_reqs.first() { - return Some(concat_npm_specifier( - "npm:", - pkg_req, - sub_path.as_deref(), - )); + // if not found in the import map, return the first pkg req + if let Some(pkg_req) = pkg_reqs.first() { + return Some(concat_npm_specifier( + "npm:", + pkg_req, + sub_path.as_deref(), + )); + } } } } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 183901fb0d..9ec273d7f2 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -38,7 +38,6 @@ use deno_graph::ResolutionError; use deno_graph::SpecifierError; use deno_lint::rules::LintRule; use deno_runtime::deno_node; -use deno_runtime::deno_node::NpmResolver; use deno_runtime::tokio_util::create_basic_runtime; use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 38d07fb523..9b3500361f 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -105,6 +105,7 @@ use crate::lsp::urls::LspUrlKind; use crate::npm::create_npm_fs_resolver; use crate::npm::CliNpmRegistryApi; use crate::npm::CliNpmResolver; +use crate::npm::ManagedCliNpmResolver; use crate::npm::NpmCache; use crate::npm::NpmCacheDir; use crate::npm::NpmResolution; @@ -137,7 +138,7 @@ struct LspNpmServices { /// Npm resolution that is stored in memory. resolution: Arc, /// Resolver for npm packages. - resolver: Arc, + resolver: Arc, } #[derive(Debug, PartialEq, Eq)] @@ -161,7 +162,7 @@ pub struct LanguageServer(Arc>); #[derive(Debug)] pub struct StateNpmSnapshot { pub node_resolver: Arc, - pub npm_resolver: Arc, + pub npm_resolver: Arc, } /// Snapshot of the state used by TSC. @@ -506,7 +507,7 @@ fn create_npm_resolver_and_resolution( npm_cache: Arc, node_modules_dir_path: Option, maybe_snapshot: Option, -) -> (Arc, Arc) { +) -> (Arc, Arc) { let resolution = Arc::new(NpmResolution::from_serialized( api, maybe_snapshot, @@ -525,7 +526,7 @@ fn create_npm_resolver_and_resolution( NpmSystemInfo::default(), ); ( - Arc::new(CliNpmResolver::new( + Arc::new(ManagedCliNpmResolver::new( fs, resolution.clone(), fs_resolver, @@ -802,7 +803,7 @@ impl Inner { self.config.maybe_lockfile().cloned(), )); let node_fs = Arc::new(deno_fs::RealFs); - let npm_resolver = Arc::new(CliNpmResolver::new( + let npm_resolver = Arc::new(ManagedCliNpmResolver::new( node_fs.clone(), npm_resolution.clone(), create_npm_fs_resolver( @@ -1440,8 +1441,13 @@ impl Inner { let package_reqs = self.documents.npm_package_reqs(); let npm_resolver = self.npm.resolver.clone(); // spawn to avoid the LSP's Send requirements - let handle = - spawn(async move { npm_resolver.set_package_reqs(&package_reqs).await }); + let handle = spawn(async move { + if let Some(npm_resolver) = npm_resolver.as_managed() { + npm_resolver.set_package_reqs(&package_reqs).await + } else { + Ok(()) + } + }); if let Err(err) = handle.await.unwrap() { lsp_warn!("Could not set npm package requirements. {:#}", err); } @@ -2156,7 +2162,7 @@ impl Inner { &self.documents, self.maybe_import_map.as_deref(), &self.npm.resolution, - &self.npm.resolver, + self.npm.resolver.as_ref(), ) } diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index cf809408bb..7999cb1df9 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -49,7 +49,6 @@ use deno_core::JsRuntime; use deno_core::ModuleSpecifier; use deno_core::OpState; use deno_core::RuntimeOptions; -use deno_runtime::deno_node::NpmResolver; use deno_runtime::tokio_util::create_basic_runtime; use lazy_regex::lazy_regex; use log::error; diff --git a/cli/module_loader.rs b/cli/module_loader.rs index f1882d5d7b..4a694e6155 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -647,7 +647,7 @@ pub struct NpmModuleLoader { node_code_translator: Arc, fs: Arc, node_resolver: Arc, - npm_resolver: Arc, + npm_resolver: Arc, } impl NpmModuleLoader { @@ -656,7 +656,7 @@ impl NpmModuleLoader { node_code_translator: Arc, fs: Arc, node_resolver: Arc, - npm_resolver: Arc, + npm_resolver: Arc, ) -> Self { Self { cjs_resolutions, diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 41eb09a57f..1b6ec243c4 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -14,5 +14,7 @@ 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; diff --git a/cli/npm/resolvers/common.rs b/cli/npm/resolvers/common.rs index 1991b2c72f..4076579bfc 100644 --- a/cli/npm/resolvers/common.rs +++ b/cli/npm/resolvers/common.rs @@ -35,6 +35,7 @@ pub trait NpmPackageFsResolver: Send + Sync { &self, package_id: &NpmPackageId, ) -> Result; + fn resolve_package_folder_from_package( &self, name: &str, diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index efaad93ee6..07a122a3e9 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -49,17 +49,77 @@ pub struct NpmProcessState { pub local_node_modules_path: Option, } -/// Brings together the npm resolution with the file system. -pub struct CliNpmResolver { +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 { fs: Arc, fs_resolver: Arc, resolution: Arc, maybe_lockfile: Option>>, } -impl std::fmt::Debug for CliNpmResolver { +impl std::fmt::Debug for ManagedCliNpmResolver { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("NpmPackageResolver") + f.debug_struct("ManagedNpmResolver") .field("fs", &"") .field("fs_resolver", &"") .field("resolution", &"") @@ -68,7 +128,7 @@ impl std::fmt::Debug for CliNpmResolver { } } -impl CliNpmResolver { +impl ManagedCliNpmResolver { pub fn new( fs: Arc, resolution: Arc, @@ -83,44 +143,6 @@ impl CliNpmResolver { } } - pub fn root_dir_url(&self) -> &Url { - self.fs_resolver.root_dir_url() - } - - pub fn node_modules_path(&self) -> Option { - self.fs_resolver.node_modules_path() - } - - /// Checks if the provided package req's folder is cached. - pub fn is_pkg_req_folder_cached(&self, req: &PackageReq) -> bool { - self - .resolve_pkg_id_from_pkg_req(req) - .ok() - .and_then(|id| self.fs_resolver.package_folder(&id).ok()) - .map(|folder| folder.exists()) - .unwrap_or(false) - } - - pub fn resolve_pkg_nv_ref_from_pkg_req_ref( - &self, - req_ref: &NpmPackageReqReference, - ) -> Result { - let pkg_nv = self - .resolve_pkg_id_from_pkg_req(req_ref.req()) - .map(|id| id.nv)?; - Ok(NpmPackageNvReference::new(PackageNvReference { - nv: pkg_nv, - sub_path: req_ref.sub_path().map(|s| s.to_string()), - })) - } - - pub fn resolve_pkg_id_from_pkg_req( - &self, - req: &PackageReq, - ) -> Result { - self.resolution.resolve_pkg_id_from_pkg_req(req) - } - pub fn resolve_pkg_folder_from_pkg_id( &self, pkg_id: &NpmPackageId, @@ -140,43 +162,6 @@ impl CliNpmResolver { Ok(path) } - /// 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. - pub fn resolve_pkg_folder_from_specifier( - &self, - specifier: &ModuleSpecifier, - ) -> Result, AnyError> { - let Some(path) = self - .fs_resolver - .resolve_package_folder_from_specifier(specifier)? - else { - return Ok(None); - }; - log::debug!( - "Resolved package folder of {} to {}", - specifier, - path.display() - ); - Ok(Some(path)) - } - - pub fn resolve_pkg_folder_from_deno_module_req( - &self, - req: &PackageReq, - ) -> Result { - let pkg_id = self.resolve_pkg_id_from_pkg_req(req)?; - self.resolve_pkg_folder_from_pkg_id(&pkg_id) - } - - pub fn resolve_pkg_folder_from_deno_module( - &self, - nv: &PackageNv, - ) -> Result { - let pkg_id = self.resolution.resolve_pkg_id_from_deno_module(nv)?; - self.resolve_pkg_folder_from_pkg_id(&pkg_id) - } - /// Resolves the package nv from the provided specifier. pub fn resolve_pkg_id_from_specifier( &self, @@ -235,25 +220,6 @@ impl CliNpmResolver { self.resolution.set_package_reqs(packages).await } - /// Gets the state of npm for the process. - pub fn get_npm_process_state(&self) -> String { - serde_json::to_string(&NpmProcessState { - snapshot: self - .resolution - .serialized_valid_snapshot() - .into_serialized(), - local_node_modules_path: self - .fs_resolver - .node_modules_path() - .map(|p| p.to_string_lossy().to_string()), - }) - .unwrap() - } - - pub fn package_reqs(&self) -> HashMap { - self.resolution.package_reqs() - } - pub fn snapshot(&self) -> NpmResolutionSnapshot { self.resolution.snapshot() } @@ -278,9 +244,16 @@ impl CliNpmResolver { self.fs_resolver.cache_packages().await?; Ok(()) } + + fn resolve_pkg_id_from_pkg_req( + &self, + req: &PackageReq, + ) -> Result { + self.resolution.resolve_pkg_id_from_pkg_req(req) + } } -impl NpmResolver for CliNpmResolver { +impl NpmResolver for ManagedCliNpmResolver { fn resolve_package_folder_from_package( &self, name: &str, @@ -316,6 +289,103 @@ impl NpmResolver for CliNpmResolver { } } +impl CliNpmResolver for ManagedCliNpmResolver { + fn into_npm_resolver(self: Arc) -> Arc { + self + } + + fn root_dir_url(&self) -> &Url { + self.fs_resolver.root_dir_url() + } + + fn as_inner(&self) -> InnerCliNpmResolverRef { + InnerCliNpmResolverRef::Managed(self) + } + + fn node_modules_path(&self) -> Option { + self.fs_resolver.node_modules_path() + } + + /// Checks if the provided package req's folder is cached. + fn is_pkg_req_folder_cached(&self, req: &PackageReq) -> bool { + self + .resolve_pkg_id_from_pkg_req(req) + .ok() + .and_then(|id| self.fs_resolver.package_folder(&id).ok()) + .map(|folder| folder.exists()) + .unwrap_or(false) + } + + fn resolve_pkg_nv_ref_from_pkg_req_ref( + &self, + req_ref: &NpmPackageReqReference, + ) -> Result { + let pkg_nv = self + .resolve_pkg_id_from_pkg_req(req_ref.req()) + .map(|id| id.nv)?; + Ok(NpmPackageNvReference::new(PackageNvReference { + nv: pkg_nv, + sub_path: req_ref.sub_path().map(|s| s.to_string()), + })) + } + + /// 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> { + let Some(path) = self + .fs_resolver + .resolve_package_folder_from_specifier(specifier)? + else { + return Ok(None); + }; + log::debug!( + "Resolved package folder of {} to {}", + specifier, + path.display() + ); + Ok(Some(path)) + } + + fn resolve_pkg_folder_from_deno_module_req( + &self, + req: &PackageReq, + ) -> Result { + let pkg_id = self.resolve_pkg_id_from_pkg_req(req)?; + self.resolve_pkg_folder_from_pkg_id(&pkg_id) + } + + fn resolve_pkg_folder_from_deno_module( + &self, + nv: &PackageNv, + ) -> Result { + let pkg_id = self.resolution.resolve_pkg_id_from_deno_module(nv)?; + self.resolve_pkg_folder_from_pkg_id(&pkg_id) + } + + /// Gets the state of npm for the process. + fn get_npm_process_state(&self) -> String { + serde_json::to_string(&NpmProcessState { + snapshot: self + .resolution + .serialized_valid_snapshot() + .into_serialized(), + local_node_modules_path: self + .fs_resolver + .node_modules_path() + .map(|p| p.to_string_lossy().to_string()), + }) + .unwrap() + } + + fn package_reqs(&self) -> HashMap { + self.resolution.package_reqs() + } +} + pub fn create_npm_fs_resolver( fs: Arc, cache: Arc, diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs index ab3a554687..eb75dc2725 100644 --- a/cli/ops/mod.rs +++ b/cli/ops/mod.rs @@ -12,7 +12,7 @@ pub mod bench; pub mod jupyter; pub mod testing; -pub fn cli_exts(npm_resolver: Arc) -> Vec { +pub fn cli_exts(npm_resolver: Arc) -> Vec { vec![ #[cfg(not(feature = "__runtime_js_sources"))] cli::init_ops(npm_resolver), @@ -33,7 +33,7 @@ deno_core::extension!(cli, "99_main.js" ], options = { - npm_resolver: Arc, + npm_resolver: Arc, }, state = |state, options| { state.put(options.npm_resolver); @@ -51,6 +51,6 @@ deno_core::extension!(cli, #[op2] #[string] fn op_npm_process_state(state: &mut OpState) -> Result { - let npm_resolver = state.borrow_mut::>(); + let npm_resolver = state.borrow_mut::>(); Ok(npm_resolver.get_npm_process_state()) } diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 48ef043da5..38fb3b08f2 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -38,6 +38,7 @@ use crate::file_fetcher::FileFetcher; use crate::http_util::HttpClient; use crate::npm::CliNpmRegistryApi; use crate::npm::CliNpmResolver; +use crate::npm::InnerCliNpmResolverRef; use crate::npm::NpmCache; use crate::npm::NpmResolution; use crate::util::progress_bar::ProgressBar; @@ -344,7 +345,7 @@ pub struct DenoCompileBinaryWriter<'a> { npm_api: &'a CliNpmRegistryApi, npm_cache: &'a NpmCache, npm_resolution: &'a NpmResolution, - npm_resolver: &'a CliNpmResolver, + npm_resolver: &'a dyn CliNpmResolver, npm_system_info: NpmSystemInfo, package_json_deps_provider: &'a PackageJsonDepsProvider, } @@ -358,7 +359,7 @@ impl<'a> DenoCompileBinaryWriter<'a> { npm_api: &'a CliNpmRegistryApi, npm_cache: &'a NpmCache, npm_resolution: &'a NpmResolution, - npm_resolver: &'a CliNpmResolver, + npm_resolver: &'a dyn CliNpmResolver, npm_system_info: NpmSystemInfo, package_json_deps_provider: &'a PackageJsonDepsProvider, ) -> Self { @@ -545,28 +546,35 @@ impl<'a> DenoCompileBinaryWriter<'a> { } fn build_vfs(&self) -> Result { - if let Some(node_modules_path) = self.npm_resolver.node_modules_path() { - let mut builder = VfsBuilder::new(node_modules_path.clone())?; - builder.add_dir_recursive(&node_modules_path)?; - Ok(builder) - } else { - // DO NOT include the user's registry url as it may contain credentials, - // but also don't make this dependent on the registry url - 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) - { - let folder = self - .npm_resolver - .resolve_pkg_folder_from_pkg_id(&package.id)?; - builder.add_dir_recursive(&folder)?; + match self.npm_resolver.as_inner() { + InnerCliNpmResolverRef::Managed(npm_resolver) => { + if let Some(node_modules_path) = npm_resolver.node_modules_path() { + let mut builder = VfsBuilder::new(node_modules_path.clone())?; + builder.add_dir_recursive(&node_modules_path)?; + Ok(builder) + } else { + // DO NOT include the user's registry url as it may contain credentials, + // but also don't make this dependent on the registry url + 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) + { + let folder = + npm_resolver.resolve_pkg_folder_from_pkg_id(&package.id)?; + builder.add_dir_recursive(&folder)?; + } + // overwrite the root directory's name to obscure the user's registry url + builder.set_root_dir_name("node_modules".to_string()); + Ok(builder) + } + } + InnerCliNpmResolverRef::Byonm(_) => { + // todo(#18967): should use the node_modules directory + todo!() } - // overwrite the root directory's name to obscure the user's registry url - builder.set_root_dir_name("node_modules".to_string()); - Ok(builder) } } } diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index e3ab448e37..30bbd7f8b9 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -17,6 +17,7 @@ use crate::node::CliCjsCodeAnalyzer; use crate::npm::create_npm_fs_resolver; use crate::npm::CliNpmRegistryApi; use crate::npm::CliNpmResolver; +use crate::npm::ManagedCliNpmResolver; use crate::npm::NpmCache; use crate::npm::NpmCacheDir; use crate::npm::NpmResolution; @@ -365,14 +366,16 @@ pub async fn run( node_modules_path, NpmSystemInfo::default(), ); - let npm_resolver = Arc::new(CliNpmResolver::new( + let npm_resolver = Arc::new(ManagedCliNpmResolver::new( fs.clone(), npm_resolution.clone(), npm_fs_resolver, None, + )) as Arc; + let node_resolver = Arc::new(NodeResolver::new( + fs.clone(), + npm_resolver.clone().into_npm_resolver(), )); - let node_resolver = - Arc::new(NodeResolver::new(fs.clone(), npm_resolver.clone())); let cjs_resolutions = Arc::new(CjsResolutionStore::default()); let cache_db = Caches::new(deno_dir_provider.clone()); let node_analysis_cache = NodeAnalysisCache::new(cache_db.node_analysis_db()); @@ -382,7 +385,7 @@ pub async fn run( cjs_esm_code_analyzer, fs.clone(), node_resolver.clone(), - npm_resolver.clone(), + npm_resolver.clone().into_npm_resolver(), )); let package_json_deps_provider = Arc::new(PackageJsonDepsProvider::new( metadata diff --git a/cli/tools/check.rs b/cli/tools/check.rs index a61e3cfe15..0a25518e45 100644 --- a/cli/tools/check.rs +++ b/cli/tools/check.rs @@ -44,7 +44,7 @@ pub struct TypeChecker { caches: Arc, cli_options: Arc, node_resolver: Arc, - npm_resolver: Arc, + npm_resolver: Arc, } impl TypeChecker { @@ -52,7 +52,7 @@ impl TypeChecker { caches: Arc, cli_options: Arc, node_resolver: Arc, - npm_resolver: Arc, + npm_resolver: Arc, ) -> Self { Self { caches, @@ -74,11 +74,10 @@ impl TypeChecker { // node built-in specifiers use the @types/node package to determine // types, so inject that now (the caller should do this after the lockfile // has been written) - if graph.has_node_specifier { - self - .npm_resolver - .inject_synthetic_types_node_package() - .await?; + if let Some(npm_resolver) = self.npm_resolver.as_managed() { + if graph.has_node_specifier { + npm_resolver.inject_synthetic_types_node_package().await?; + } } log::debug!("Type checking."); diff --git a/cli/tools/info.rs b/cli/tools/info.rs index 941ba1cbdc..e1972f08f5 100644 --- a/cli/tools/info.rs +++ b/cli/tools/info.rs @@ -31,6 +31,7 @@ use crate::display; use crate::factory::CliFactory; use crate::graph_util::graph_lock_or_exit; use crate::npm::CliNpmResolver; +use crate::npm::ManagedCliNpmResolver; use crate::util::checksum; pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> { @@ -71,11 +72,11 @@ pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> { if info_flags.json { let mut json_graph = json!(graph); - add_npm_packages_to_json(&mut json_graph, npm_resolver); + add_npm_packages_to_json(&mut json_graph, npm_resolver.as_ref()); display::write_json_to_stdout(&json_graph)?; } else { let mut output = String::new(); - GraphDisplayContext::write(&graph, npm_resolver, &mut output)?; + GraphDisplayContext::write(&graph, npm_resolver.as_ref(), &mut output)?; display::write_to_stdout_ignore_sigpipe(output.as_bytes())?; } } else { @@ -165,8 +166,12 @@ fn print_cache_info( fn add_npm_packages_to_json( json: &mut serde_json::Value, - npm_resolver: &CliNpmResolver, + npm_resolver: &dyn CliNpmResolver, ) { + let Some(npm_resolver) = npm_resolver.as_managed() else { + return; // does not include byonm to deno info's output + }; + // ideally deno_graph could handle this, but for now we just modify the json here let snapshot = npm_resolver.snapshot(); let json = json.as_object_mut().unwrap(); @@ -339,7 +344,7 @@ struct NpmInfo { impl NpmInfo { pub fn build<'a>( graph: &'a ModuleGraph, - npm_resolver: &'a CliNpmResolver, + npm_resolver: &'a ManagedCliNpmResolver, npm_snapshot: &'a NpmResolutionSnapshot, ) -> Self { let mut info = NpmInfo::default(); @@ -365,7 +370,7 @@ impl NpmInfo { fn fill_package_info<'a>( &mut self, package: &NpmResolutionPackage, - npm_resolver: &'a CliNpmResolver, + npm_resolver: &'a ManagedCliNpmResolver, npm_snapshot: &'a NpmResolutionSnapshot, ) { self.packages.insert(package.id.clone(), package.clone()); @@ -399,11 +404,16 @@ struct GraphDisplayContext<'a> { impl<'a> GraphDisplayContext<'a> { pub fn write( graph: &'a ModuleGraph, - npm_resolver: &'a CliNpmResolver, + npm_resolver: &'a dyn CliNpmResolver, writer: &mut TWrite, ) -> fmt::Result { - let npm_snapshot = npm_resolver.snapshot(); - let npm_info = NpmInfo::build(graph, npm_resolver, &npm_snapshot); + let npm_info = match npm_resolver.as_managed() { + Some(npm_resolver) => { + let npm_snapshot = npm_resolver.snapshot(); + NpmInfo::build(graph, npm_resolver, &npm_snapshot) + } + None => NpmInfo::default(), + }; Self { graph, npm_info, diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index a1b602b4b5..f833fbf5d2 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -123,7 +123,7 @@ pub struct TsEvaluateResponse { } pub struct ReplSession { - npm_resolver: Arc, + npm_resolver: Arc, resolver: Arc, pub worker: MainWorker, session: LocalInspectorSession, @@ -136,7 +136,7 @@ pub struct ReplSession { impl ReplSession { pub async fn initialize( cli_options: &CliOptions, - npm_resolver: Arc, + npm_resolver: Arc, resolver: Arc, mut worker: MainWorker, ) -> Result { @@ -508,6 +508,10 @@ impl ReplSession { &mut self, program: &swc_ast::Program, ) -> Result<(), AnyError> { + let Some(npm_resolver) = self.npm_resolver.as_managed() else { + return Ok(()); // don't auto-install for byonm + }; + let mut collector = ImportCollector::new(); program.visit_with(&mut collector); @@ -531,14 +535,11 @@ impl ReplSession { let has_node_specifier = resolved_imports.iter().any(|url| url.scheme() == "node"); if !npm_imports.is_empty() || has_node_specifier { - self.npm_resolver.add_package_reqs(&npm_imports).await?; + npm_resolver.add_package_reqs(&npm_imports).await?; // prevent messages in the repl about @types/node not being cached if has_node_specifier { - self - .npm_resolver - .inject_synthetic_types_node_package() - .await?; + npm_resolver.inject_synthetic_types_node_package().await?; } } Ok(()) diff --git a/cli/tools/task.rs b/cli/tools/task.rs index 6a6c23e39d..d1513072a9 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -5,7 +5,7 @@ use crate::args::Flags; use crate::args::TaskFlags; use crate::colors; use crate::factory::CliFactory; -use crate::npm::CliNpmResolver; +use crate::npm::ManagedCliNpmResolver; use crate::util::fs::canonicalize_path; use deno_core::anyhow::bail; use deno_core::anyhow::Context; @@ -19,6 +19,7 @@ use deno_task_shell::ShellCommand; use deno_task_shell::ShellCommandContext; use indexmap::IndexMap; use std::collections::HashMap; +use std::path::Path; use std::path::PathBuf; use std::rc::Rc; use tokio::task::LocalSet; @@ -67,8 +68,6 @@ pub async fn execute_script( Ok(exit_code) } else if package_json_scripts.contains_key(task_name) { let package_json_deps_provider = factory.package_json_deps_provider(); - let package_json_deps_installer = - factory.package_json_deps_installer().await?; let npm_resolver = factory.npm_resolver().await?; let node_resolver = factory.node_resolver().await?; @@ -85,10 +84,15 @@ pub async fn execute_script( } } - package_json_deps_installer - .ensure_top_level_install() - .await?; - npm_resolver.resolve_pending().await?; + // install the npm packages if we're using a managed resolver + if let Some(npm_resolver) = npm_resolver.as_managed() { + let package_json_deps_installer = + factory.package_json_deps_installer().await?; + package_json_deps_installer + .ensure_top_level_install() + .await?; + npm_resolver.resolve_pending().await?; + } log::info!( "{} Currently only basic package.json `scripts` are supported. Programs like `rimraf` or `cross-env` will not work correctly. This will be fixed in an upcoming release.", @@ -120,8 +124,16 @@ pub async fn execute_script( output_task(&task_name, &script); let seq_list = deno_task_shell::parser::parse(&script) .with_context(|| format!("Error parsing script '{task_name}'."))?; - let npx_commands = resolve_npm_commands(npm_resolver, node_resolver)?; - let env_vars = collect_env_vars(); + let npx_commands = match npm_resolver.as_managed() { + Some(npm_resolver) => { + resolve_npm_commands(npm_resolver, node_resolver)? + } + None => Default::default(), + }; + let env_vars = match npm_resolver.node_modules_path() { + Some(dir_path) => collect_env_vars_with_node_modules_dir(&dir_path), + None => collect_env_vars(), + }; let local = LocalSet::new(); let future = deno_task_shell::execute(seq_list, env_vars, &cwd, npx_commands); @@ -162,6 +174,36 @@ fn output_task(task_name: &str, script: &str) { ); } +fn collect_env_vars_with_node_modules_dir( + node_modules_dir_path: &Path, +) -> HashMap { + let mut env_vars = collect_env_vars(); + prepend_to_path( + &mut env_vars, + node_modules_dir_path + .join(".bin") + .to_string_lossy() + .to_string(), + ); + env_vars +} + +fn prepend_to_path(env_vars: &mut HashMap, value: String) { + match env_vars.get_mut("PATH") { + Some(path) => { + if path.is_empty() { + *path = value; + } else { + *path = + format!("{}{}{}", value, if cfg!(windows) { ";" } else { ":" }, path); + } + } + None => { + env_vars.insert("PATH".to_string(), value); + } + } +} + fn collect_env_vars() -> HashMap { // get the starting env vars (the PWD env var will be set by deno_task_shell) let mut env_vars = std::env::vars().collect::>(); @@ -262,7 +304,7 @@ impl ShellCommand for NpmPackageBinCommand { } fn resolve_npm_commands( - npm_resolver: &CliNpmResolver, + npm_resolver: &ManagedCliNpmResolver, node_resolver: &NodeResolver, ) -> Result>, AnyError> { let mut result = HashMap::new(); @@ -286,3 +328,36 @@ fn resolve_npm_commands( } Ok(result) } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_prepend_to_path() { + let mut env_vars = HashMap::new(); + + prepend_to_path(&mut env_vars, "/example".to_string()); + assert_eq!( + env_vars, + HashMap::from([("PATH".to_string(), "/example".to_string())]) + ); + + prepend_to_path(&mut env_vars, "/example2".to_string()); + let separator = if cfg!(windows) { ";" } else { ":" }; + assert_eq!( + env_vars, + HashMap::from([( + "PATH".to_string(), + format!("/example2{}/example", separator) + )]) + ); + + env_vars.get_mut("PATH").unwrap().clear(); + prepend_to_path(&mut env_vars, "/example".to_string()); + assert_eq!( + env_vars, + HashMap::from([("PATH".to_string(), "/example".to_string())]) + ); + } +} diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 50b7dc9e4d..cd3d9ecae4 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -297,7 +297,7 @@ pub struct EmittedFile { #[derive(Debug)] pub struct RequestNpmState { pub node_resolver: Arc, - pub npm_resolver: Arc, + pub npm_resolver: Arc, } /// A structure representing a request to be sent to the tsc runtime. diff --git a/cli/worker.rs b/cli/worker.rs index 5d80ab6fd7..d4277c618b 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -98,7 +98,7 @@ pub struct CliMainWorkerOptions { struct SharedWorkerState { options: CliMainWorkerOptions, storage_key_resolver: StorageKeyResolver, - npm_resolver: Arc, + npm_resolver: Arc, node_resolver: Arc, blob_store: Arc, broadcast_channel: InMemoryBroadcastChannel, @@ -305,7 +305,7 @@ impl CliMainWorkerFactory { #[allow(clippy::too_many_arguments)] pub fn new( storage_key_resolver: StorageKeyResolver, - npm_resolver: Arc, + npm_resolver: Arc, node_resolver: Arc, blob_store: Arc, module_loader_factory: Box, @@ -383,10 +383,11 @@ impl CliMainWorkerFactory { } else { package_ref }; - shared - .npm_resolver - .add_package_reqs(&[package_ref.req().clone()]) - .await?; + if let Some(npm_resolver) = shared.npm_resolver.as_managed() { + npm_resolver + .add_package_reqs(&[package_ref.req().clone()]) + .await?; + } let package_ref = shared .npm_resolver .resolve_pkg_nv_ref_from_pkg_req_ref(&package_ref)?; @@ -486,7 +487,7 @@ impl CliMainWorkerFactory { should_wait_for_inspector_session: shared.options.inspect_wait, module_loader, fs: shared.fs.clone(), - npm_resolver: Some(shared.npm_resolver.clone()), + npm_resolver: Some(shared.npm_resolver.clone().into_npm_resolver()), get_error_class_fn: Some(&errors::get_error_class_name), cache_storage_dir, origin_storage_dir, @@ -652,7 +653,7 @@ fn create_web_worker_callback( source_map_getter: maybe_source_map_getter, module_loader, fs: shared.fs.clone(), - npm_resolver: Some(shared.npm_resolver.clone()), + npm_resolver: Some(shared.npm_resolver.clone().into_npm_resolver()), worker_type: args.worker_type, maybe_inspector_server, get_error_class_fn: Some(&errors::get_error_class_name),