diff options
55 files changed, 1335 insertions, 1726 deletions
diff --git a/Cargo.lock b/Cargo.lock index d15f5e7c4..38b81fc24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1009,9 +1009,9 @@ dependencies = [ [[package]] name = "deno_doc" -version = "0.55.0" +version = "0.57.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d51f565fc87d1b8754482a49acf48254c9413392696a109d0f7464fb68da17d4" +checksum = "717302b02dd63e4e16a5e7f6339a74708a981b2addc5cbc944b9a4c74857120c" dependencies = [ "cfg-if", "deno_ast", @@ -1027,9 +1027,9 @@ dependencies = [ [[package]] name = "deno_emit" -version = "0.15.0" +version = "0.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1462b4d37e1d301512505231729b39993d601ffc3c5c8121cd3a829952bd99ad" +checksum = "068712ed5abcae4c109c4e08d7b8f959fc974726c922a6c3c71aabb613249c92" dependencies = [ "anyhow", "base64 0.13.1", @@ -1093,14 +1093,15 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.43.3" +version = "0.44.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b654f093ae79ca93715d6df840f45890dc61fc93e7f63ab0a7442c2a494ecac" +checksum = "f03c7b4d4990cc1f09c09c6ae94ee0dc039c9fcf66a97f8e5954ee102ba0eedf" dependencies = [ "anyhow", "data-url", "deno_ast", "futures", + "indexmap", "monch", "once_cell", "parking_lot 0.12.1", @@ -1710,9 +1711,9 @@ dependencies = [ [[package]] name = "eszip" -version = "0.35.0" +version = "0.37.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bbe59a9f33f0b4c2ac2b630e1769225fc1d7419bef195e27084be24b996afd9" +checksum = "a1a9f2547787db6a0f5998a91dc54a3b5031190d11edcc62739473c0299bf36b" dependencies = [ "anyhow", "base64 0.13.1", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 970a015cf..3903d32f1 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -44,9 +44,9 @@ winres.workspace = true [dependencies] deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] } deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } -deno_doc = "0.55.0" -deno_emit = "0.15.0" -deno_graph = "0.43.3" +deno_doc = "0.57.0" +deno_emit = "0.16.0" +deno_graph = "0.44.0" deno_lint = { version = "0.40.0", features = ["docs"] } deno_lockfile.workspace = true deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] } @@ -70,7 +70,7 @@ dprint-plugin-markdown = "=0.15.2" dprint-plugin-typescript = "=0.83.0" encoding_rs.workspace = true env_logger = "=0.9.0" -eszip = "=0.35.0" +eszip = "=0.37.0" fancy-regex = "=0.10.0" flate2.workspace = true http.workspace = true diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index 5a65bd0a2..eead9c419 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -3,7 +3,6 @@ use crate::errors::get_error_class_name; use crate::file_fetcher::FileFetcher; -use deno_core::futures; use deno_core::futures::FutureExt; use deno_core::ModuleSpecifier; use deno_graph::source::CacheInfo; @@ -76,10 +75,6 @@ impl Loader for FetchCacher { return None; } - if matches!(specifier.scheme(), "npm" | "node") { - return None; - } - let local = self.file_fetcher.get_local_path(specifier)?; if local.is_file() { let emit = self @@ -101,40 +96,13 @@ impl Loader for FetchCacher { specifier: &ModuleSpecifier, is_dynamic: bool, ) -> LoadFuture { - if specifier.scheme() == "npm" { - return Box::pin(futures::future::ready( - match deno_graph::npm::NpmPackageReqReference::from_specifier(specifier) - { - Ok(_) => Ok(Some(deno_graph::source::LoadResponse::External { - specifier: specifier.clone(), - })), - Err(err) => Err(err.into()), - }, - )); - } - - let specifier = - if let Some(module_name) = specifier.as_str().strip_prefix("node:") { - // Built-in Node modules are embedded in the Deno binary (in V8 snapshot) - // so we don't want them to be loaded by the "deno graph". - match crate::node::resolve_builtin_node_module(module_name) { - Ok(specifier) => { - return Box::pin(futures::future::ready(Ok(Some( - deno_graph::source::LoadResponse::External { specifier }, - )))) - } - Err(err) => return Box::pin(futures::future::ready(Err(err))), - } - } else { - specifier.clone() - }; - let permissions = if is_dynamic { self.dynamic_permissions.clone() } else { self.root_permissions.clone() }; let file_fetcher = self.file_fetcher.clone(); + let specifier = specifier.clone(); async move { file_fetcher diff --git a/cli/cache/parsed_source.rs b/cli/cache/parsed_source.rs index fc1b54ddb..b6a80e82e 100644 --- a/cli/cache/parsed_source.rs +++ b/cli/cache/parsed_source.rs @@ -75,19 +75,15 @@ impl ParsedSourceCache { } } - pub fn get_parsed_source_from_module( + pub fn get_parsed_source_from_esm_module( &self, - module: &deno_graph::Module, - ) -> Result<Option<ParsedSource>, AnyError> { - if let Some(source) = &module.maybe_source { - Ok(Some(self.get_or_parse_module( - &module.specifier, - source.clone(), - module.media_type, - )?)) - } else { - Ok(None) - } + module: &deno_graph::EsmModule, + ) -> Result<ParsedSource, deno_ast::Diagnostic> { + self.get_or_parse_module( + &module.specifier, + module.source.clone(), + module.media_type, + ) } /// Gets the matching `ParsedSource` from the cache diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 77dc3011c..ef1e0f59a 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -8,7 +8,7 @@ use crate::cache; use crate::cache::TypeCheckCache; use crate::colors; use crate::errors::get_error_class_name; -use crate::npm::resolve_graph_npm_info; +use crate::npm::NpmPackageResolver; use crate::proc_state::ProcState; use crate::resolver::CliGraphResolver; use crate::tools::check; @@ -17,6 +17,7 @@ use deno_core::anyhow::bail; use deno_core::error::custom_error; use deno_core::error::AnyError; use deno_core::ModuleSpecifier; +use deno_graph::Module; use deno_graph::ModuleGraph; use deno_graph::ModuleGraphError; use deno_graph::ResolutionError; @@ -121,20 +122,23 @@ pub fn graph_valid( /// Checks the lockfile against the graph and and exits on errors. pub fn graph_lock_or_exit(graph: &ModuleGraph, lockfile: &mut Lockfile) { for module in graph.modules() { - if let Some(source) = &module.maybe_source { - if !lockfile.check_or_insert_remote(module.specifier.as_str(), source) { - let err = format!( - concat!( - "The source code is invalid, as it does not match the expected hash in the lock file.\n", - " Specifier: {}\n", - " Lock file: {}", - ), - module.specifier, - lockfile.filename.display(), - ); - log::error!("{} {}", colors::red("error:"), err); - std::process::exit(10); - } + let source = match module { + Module::Esm(module) => &module.source, + Module::Json(module) => &module.source, + Module::Node(_) | Module::Npm(_) | Module::External(_) => continue, + }; + if !lockfile.check_or_insert_remote(module.specifier().as_str(), source) { + let err = format!( + concat!( + "The source code is invalid, as it does not match the expected hash in the lock file.\n", + " Specifier: {}\n", + " Lock file: {}", + ), + module.specifier(), + lockfile.filename.display(), + ); + log::error!("{} {}", colors::red("error:"), err); + std::process::exit(10); } } } @@ -154,30 +158,33 @@ pub async fn create_graph_and_maybe_check( let cli_resolver = CliGraphResolver::new( ps.options.to_maybe_jsx_import_source_config(), ps.maybe_import_map.clone(), + ps.options.no_npm(), + ps.npm_resolver.api().clone(), + ps.npm_resolver.resolution().clone(), maybe_package_json_deps, ); let graph_resolver = cli_resolver.as_graph_resolver(); + let graph_npm_resolver = cli_resolver.as_graph_npm_resolver(); let analyzer = ps.parsed_source_cache.as_analyzer(); let mut graph = ModuleGraph::default(); - graph - .build( - vec![root], - &mut cache, - deno_graph::BuildOptions { - is_dynamic: false, - imports: maybe_imports, - resolver: Some(graph_resolver), - module_analyzer: Some(&*analyzer), - reporter: None, - }, - ) - .await; + build_graph_with_npm_resolution( + &mut graph, + &ps.npm_resolver, + vec![root], + &mut cache, + deno_graph::BuildOptions { + is_dynamic: false, + imports: maybe_imports, + resolver: Some(graph_resolver), + npm_resolver: Some(graph_npm_resolver), + module_analyzer: Some(&*analyzer), + reporter: None, + }, + ) + .await?; + graph_valid_with_cli_options(&graph, &graph.roots, &ps.options)?; let graph = Arc::new(graph); - let npm_graph_info = resolve_graph_npm_info(&graph); - ps.npm_resolver - .add_package_reqs(npm_graph_info.package_reqs) - .await?; if let Some(lockfile) = &ps.lockfile { graph_lock_or_exit(&graph, &mut lockfile.lock()); } @@ -185,7 +192,7 @@ pub async fn create_graph_and_maybe_check( if ps.options.type_check_mode() != TypeCheckMode::None { // node built-in specifiers use the @types/node package to determine // types, so inject that now after the lockfile has been written - if npm_graph_info.has_node_builtin_specifier { + if graph.has_node_specifier { ps.npm_resolver .inject_synthetic_types_node_package() .await?; @@ -211,7 +218,6 @@ pub async fn create_graph_and_maybe_check( ts_config: ts_config_result.ts_config, log_checks: true, reload: ps.options.reload_flag(), - has_node_builtin_specifier: npm_graph_info.has_node_builtin_specifier, }, )?; log::debug!("{}", check_result.stats); @@ -223,23 +229,37 @@ pub async fn create_graph_and_maybe_check( Ok(graph) } +pub async fn build_graph_with_npm_resolution<'a>( + graph: &mut ModuleGraph, + npm_resolver: &NpmPackageResolver, + roots: Vec<ModuleSpecifier>, + loader: &mut dyn deno_graph::source::Loader, + options: deno_graph::BuildOptions<'a>, +) -> Result<(), AnyError> { + graph.build(roots, loader, options).await; + + // resolve the dependencies of any pending dependencies + // that were inserted by building the graph + npm_resolver.resolve_pending().await?; + + Ok(()) +} + pub fn error_for_any_npm_specifier( - graph: &deno_graph::ModuleGraph, + graph: &ModuleGraph, ) -> Result<(), AnyError> { - let first_npm_specifier = graph - .specifiers() - .filter_map(|(_, r)| match r { - Ok(module) if module.kind == deno_graph::ModuleKind::External => { - Some(&module.specifier) + for module in graph.modules() { + match module { + Module::Npm(module) => { + bail!("npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: {}", module.specifier) } - _ => None, - }) - .next(); - if let Some(npm_specifier) = first_npm_specifier { - bail!("npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: {}", npm_specifier) - } else { - Ok(()) + Module::Node(module) => { + bail!("Node specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: node:{}", module.module_name) + } + Module::Esm(_) | Module::Json(_) | Module::External(_) => {} + } } + Ok(()) } /// Adds more explanatory information to a resolution error. diff --git a/cli/lsp/cache.rs b/cli/lsp/cache.rs index bad932176..f047e5fd4 100644 --- a/cli/lsp/cache.rs +++ b/cli/lsp/cache.rs @@ -84,7 +84,7 @@ impl CacheMetadata { } fn refresh(&self, specifier: &ModuleSpecifier) -> Option<Metadata> { - if specifier.scheme() == "file" || specifier.scheme() == "npm" { + if matches!(specifier.scheme(), "file" | "npm" | "node") { return None; } let cache_filename = self.cache.get_cache_filename(specifier)?; diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index f8f3aa371..f938ba6f4 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -911,7 +911,8 @@ fn diagnose_resolution( if let Some(npm_resolver) = &snapshot.maybe_npm_resolver { // show diagnostics for npm package references that aren't cached if npm_resolver - .resolve_package_folder_from_deno_module(&pkg_ref.req) + .resolution() + .resolve_pkg_id_from_pkg_req(&pkg_ref.req) .is_err() { diagnostics.push( @@ -932,7 +933,8 @@ fn diagnose_resolution( let types_node_ref = NpmPackageReqReference::from_str("npm:@types/node").unwrap(); if npm_resolver - .resolve_package_folder_from_deno_module(&types_node_ref.req) + .resolution() + .resolve_pkg_id_from_pkg_req(&types_node_ref.req) .is_err() { diagnostics.push( diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index c97555911..fa8b62306 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -17,6 +17,8 @@ use crate::node; use crate::node::node_resolve_npm_reference; use crate::node::NodeResolution; use crate::npm::NpmPackageResolver; +use crate::npm::NpmRegistryApi; +use crate::npm::NpmResolution; use crate::resolver::CliGraphResolver; use crate::util::path::specifier_to_file_path; use crate::util::text_encoding; @@ -36,8 +38,8 @@ use deno_graph::GraphImport; use deno_graph::Resolution; use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::permissions::PermissionsContainer; +use indexmap::IndexMap; use once_cell::sync::Lazy; -use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; @@ -242,7 +244,7 @@ impl AssetOrDocument { #[derive(Debug, Default)] struct DocumentDependencies { - deps: BTreeMap<String, deno_graph::Dependency>, + deps: IndexMap<String, deno_graph::Dependency>, maybe_types_dependency: Option<deno_graph::TypesDependency>, } @@ -255,7 +257,7 @@ impl DocumentDependencies { } } - pub fn from_module(module: &deno_graph::Module) -> Self { + pub fn from_module(module: &deno_graph::EsmModule) -> Self { Self { deps: module.dependencies.clone(), maybe_types_dependency: module.maybe_types_dependency.clone(), @@ -263,7 +265,7 @@ impl DocumentDependencies { } } -type ModuleResult = Result<deno_graph::Module, deno_graph::ModuleGraphError>; +type ModuleResult = Result<deno_graph::EsmModule, deno_graph::ModuleGraphError>; type ParsedSourceResult = Result<ParsedSource, deno_ast::Diagnostic>; #[derive(Debug)] @@ -542,9 +544,7 @@ impl Document { self.0.maybe_lsp_version } - fn maybe_module( - &self, - ) -> Option<&Result<deno_graph::Module, deno_graph::ModuleGraphError>> { + fn maybe_esm_module(&self) -> Option<&ModuleResult> { self.0.maybe_module.as_ref() } @@ -572,7 +572,7 @@ impl Document { } } - pub fn dependencies(&self) -> &BTreeMap<String, deno_graph::Dependency> { + pub fn dependencies(&self) -> &IndexMap<String, deno_graph::Dependency> { &self.0.dependencies.deps } @@ -583,7 +583,7 @@ impl Document { &self, position: &lsp::Position, ) -> Option<(String, deno_graph::Dependency, deno_graph::Range)> { - let module = self.maybe_module()?.as_ref().ok()?; + let module = self.maybe_esm_module()?.as_ref().ok()?; let position = deno_graph::Position { line: position.line as usize, character: position.character as usize, @@ -1103,19 +1103,10 @@ impl Documents { .and_then(|r| r.maybe_specifier()) { results.push(self.resolve_dependency(specifier, maybe_npm_resolver)); - } else if let Ok(npm_ref) = NpmPackageReqReference::from_str(&specifier) { - results.push(maybe_npm_resolver.map(|npm_resolver| { - NodeResolution::into_specifier_and_media_type( - node_resolve_npm_reference( - &npm_ref, - NodeResolutionMode::Types, - npm_resolver, - &mut PermissionsContainer::allow_all(), - ) - .ok() - .flatten(), - ) - })); + } else if let Ok(npm_req_ref) = + NpmPackageReqReference::from_str(&specifier) + { + results.push(node_resolve_npm_req_ref(npm_req_ref, maybe_npm_resolver)); } else { results.push(None); } @@ -1159,6 +1150,8 @@ impl Documents { &mut self, maybe_import_map: Option<Arc<import_map::ImportMap>>, maybe_config_file: Option<&ConfigFile>, + npm_registry_api: NpmRegistryApi, + npm_resolution: NpmResolution, ) { fn calculate_resolver_config_hash( maybe_import_map: Option<&import_map::ImportMap>, @@ -1182,8 +1175,14 @@ impl Documents { maybe_jsx_config.as_ref(), ); // TODO(bartlomieju): handle package.json dependencies here - self.resolver = - CliGraphResolver::new(maybe_jsx_config, maybe_import_map, None); + self.resolver = CliGraphResolver::new( + maybe_jsx_config, + maybe_import_map, + false, + npm_registry_api, + npm_resolution, + None, + ); self.imports = Arc::new( if let Some(Ok(imports)) = maybe_config_file.map(|cf| cf.to_maybe_imports()) @@ -1322,21 +1321,10 @@ impl Documents { maybe_npm_resolver: Option<&NpmPackageResolver>, ) -> Option<(ModuleSpecifier, MediaType)> { if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) { - return maybe_npm_resolver.map(|npm_resolver| { - NodeResolution::into_specifier_and_media_type( - node_resolve_npm_reference( - &npm_ref, - NodeResolutionMode::Types, - npm_resolver, - &mut PermissionsContainer::allow_all(), - ) - .ok() - .flatten(), - ) - }); + return node_resolve_npm_req_ref(npm_ref, maybe_npm_resolver); } let doc = self.get(specifier)?; - let maybe_module = doc.maybe_module().and_then(|r| r.as_ref().ok()); + let maybe_module = doc.maybe_esm_module().and_then(|r| r.as_ref().ok()); let maybe_types_dependency = maybe_module .and_then(|m| m.maybe_types_dependency.as_ref().map(|d| &d.dependency)); if let Some(specifier) = @@ -1363,6 +1351,30 @@ impl Documents { } } +fn node_resolve_npm_req_ref( + npm_req_ref: NpmPackageReqReference, + maybe_npm_resolver: Option<&NpmPackageResolver>, +) -> Option<(ModuleSpecifier, MediaType)> { + maybe_npm_resolver.map(|npm_resolver| { + NodeResolution::into_specifier_and_media_type( + npm_resolver + .resolution() + .pkg_req_ref_to_nv_ref(npm_req_ref) + .ok() + .and_then(|pkg_id_ref| { + node_resolve_npm_reference( + &pkg_id_ref, + NodeResolutionMode::Types, + npm_resolver, + &mut PermissionsContainer::allow_all(), + ) + .ok() + .flatten() + }), + ) + }) +} + /// Loader that will look at the open documents. pub struct OpenDocumentsGraphLoader<'a> { pub inner_loader: &'a mut dyn deno_graph::source::Loader, @@ -1426,7 +1438,6 @@ fn analyze_module( match parsed_source_result { Ok(parsed_source) => Ok(deno_graph::parse_module_from_ast( specifier, - deno_graph::ModuleKind::Esm, maybe_headers, parsed_source, Some(resolver), @@ -1440,6 +1451,8 @@ fn analyze_module( #[cfg(test)] mod tests { + use crate::npm::NpmResolution; + use super::*; use import_map::ImportMap; use test_util::TempDir; @@ -1540,6 +1553,10 @@ console.log(b, "hello deno"); #[test] fn test_documents_refresh_dependencies_config_change() { + let npm_registry_api = NpmRegistryApi::new_uninitialized(); + let npm_resolution = + NpmResolution::new(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(); @@ -1569,7 +1586,12 @@ console.log(b, "hello deno"); .append("test".to_string(), "./file2.ts".to_string()) .unwrap(); - documents.update_config(Some(Arc::new(import_map)), None); + documents.update_config( + Some(Arc::new(import_map)), + None, + npm_registry_api.clone(), + npm_resolution.clone(), + ); // open the document let document = documents.open( @@ -1602,7 +1624,12 @@ console.log(b, "hello deno"); .append("test".to_string(), "./file3.ts".to_string()) .unwrap(); - documents.update_config(Some(Arc::new(import_map)), None); + documents.update_config( + Some(Arc::new(import_map)), + None, + npm_registry_api, + npm_resolution, + ); // check the document's dependencies let document = documents.get(&file1_specifier).unwrap(); diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 33b3379a2..37c36e228 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -937,6 +937,8 @@ impl Inner { self.documents.update_config( self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), + self.npm_resolver.api().clone(), + self.npm_resolver.resolution().clone(), ); self.assets.intitialize(self.snapshot()).await; @@ -1124,6 +1126,8 @@ impl Inner { self.documents.update_config( self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), + self.npm_resolver.api().clone(), + self.npm_resolver.resolution().clone(), ); self.send_diagnostics_update(); @@ -1170,6 +1174,8 @@ impl Inner { self.documents.update_config( self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), + self.npm_resolver.api().clone(), + self.npm_resolver.resolution().clone(), ); self.refresh_npm_specifiers().await; self.diagnostics_server.invalidate_all(); diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 462b41cbb..4ddb297a5 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -21,6 +21,8 @@ use deno_core::ModuleType; use deno_core::OpState; use deno_core::ResolutionKind; use deno_core::SourceMapGetter; +use deno_graph::EsmModule; +use deno_graph::JsonModule; use deno_runtime::permissions::PermissionsContainer; use std::cell::RefCell; use std::pin::Pin; @@ -78,23 +80,33 @@ impl CliModuleLoader { maybe_referrer: Option<ModuleSpecifier>, ) -> Result<ModuleCodeSource, AnyError> { if specifier.scheme() == "node" { - unreachable!("Node built-in modules should be handled internally."); + unreachable!(); // Node built-in modules should be handled internally. } let graph = self.ps.graph(); match graph.get(specifier) { - Some(deno_graph::Module { - maybe_source: Some(code), + Some(deno_graph::Module::Json(JsonModule { + source, media_type, specifier, .. - }) => { + })) => Ok(ModuleCodeSource { + code: source.to_string(), + found_url: specifier.clone(), + media_type: *media_type, + }), + Some(deno_graph::Module::Esm(EsmModule { + source, + media_type, + specifier, + .. + })) => { let code = match media_type { MediaType::JavaScript | MediaType::Unknown | MediaType::Cjs | MediaType::Mjs - | MediaType::Json => code.to_string(), + | MediaType::Json => source.to_string(), MediaType::Dts | MediaType::Dcts | MediaType::Dmts => "".to_string(), MediaType::TypeScript | MediaType::Mts @@ -107,7 +119,7 @@ impl CliModuleLoader { &self.ps.parsed_source_cache, specifier, *media_type, - code, + source, &self.ps.emit_options, self.ps.emit_options_hash, )? @@ -295,10 +307,8 @@ impl SourceMapGetter for CliModuleLoader { ) -> Option<String> { let graph = self.ps.graph(); let code = match graph.get(&resolve_url(file_name).ok()?) { - Some(deno_graph::Module { - maybe_source: Some(code), - .. - }) => code, + Some(deno_graph::Module::Esm(module)) => &module.source, + Some(deno_graph::Module::Json(module)) => &module.source, _ => return None, }; // Do NOT use .lines(): it skips the terminating empty line. diff --git a/cli/node/mod.rs b/cli/node/mod.rs index 9286400d4..a6f40c99e 100644 --- a/cli/node/mod.rs +++ b/cli/node/mod.rs @@ -15,8 +15,8 @@ use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::serde_json::Value; use deno_core::url::Url; -use deno_graph::npm::NpmPackageReq; -use deno_graph::npm::NpmPackageReqReference; +use deno_graph::npm::NpmPackageNv; +use deno_graph::npm::NpmPackageNvReference; use deno_runtime::deno_node; use deno_runtime::deno_node::errors; use deno_runtime::deno_node::find_builtin_node_module; @@ -105,13 +105,13 @@ impl NodeResolution { } // TODO(bartlomieju): seems super wasteful to parse specified each time -pub fn resolve_builtin_node_module(specifier: &str) -> Result<Url, AnyError> { - if let Some(module) = find_builtin_node_module(specifier) { +pub fn resolve_builtin_node_module(module_name: &str) -> Result<Url, AnyError> { + if let Some(module) = find_builtin_node_module(module_name) { return Ok(ModuleSpecifier::parse(module.specifier).unwrap()); } Err(generic_error(format!( - "Unknown built-in \"node:\" module: {specifier}" + "Unknown built-in \"node:\" module: {module_name}" ))) } @@ -241,13 +241,13 @@ pub fn node_resolve( } pub fn node_resolve_npm_reference( - reference: &NpmPackageReqReference, + reference: &NpmPackageNvReference, mode: NodeResolutionMode, npm_resolver: &NpmPackageResolver, permissions: &mut dyn NodePermissions, ) -> Result<Option<NodeResolution>, AnyError> { let package_folder = - npm_resolver.resolve_package_folder_from_deno_module(&reference.req)?; + npm_resolver.resolve_package_folder_from_deno_module(&reference.nv)?; let node_module_kind = NodeModuleKind::Esm; let maybe_resolved_path = package_config_resolve( &reference @@ -286,13 +286,13 @@ pub fn node_resolve_npm_reference( } pub fn node_resolve_binary_export( - pkg_req: &NpmPackageReq, + pkg_nv: &NpmPackageNv, bin_name: Option<&str>, npm_resolver: &NpmPackageResolver, permissions: &mut dyn NodePermissions, ) -> Result<NodeResolution, AnyError> { let package_folder = - npm_resolver.resolve_package_folder_from_deno_module(pkg_req)?; + npm_resolver.resolve_package_folder_from_deno_module(pkg_nv)?; let package_json_path = package_folder.join("package.json"); let package_json = PackageJson::load(npm_resolver, permissions, package_json_path)?; @@ -300,10 +300,10 @@ pub fn node_resolve_binary_export( Some(bin) => bin, None => bail!( "package '{}' did not have a bin property in its package.json", - &pkg_req.name, + &pkg_nv.name, ), }; - let bin_entry = resolve_bin_entry_value(pkg_req, bin_name, bin)?; + let bin_entry = resolve_bin_entry_value(pkg_nv, bin_name, bin)?; let url = ModuleSpecifier::from_file_path(package_folder.join(bin_entry)).unwrap(); @@ -314,13 +314,13 @@ pub fn node_resolve_binary_export( } fn resolve_bin_entry_value<'a>( - pkg_req: &NpmPackageReq, + pkg_nv: &NpmPackageNv, bin_name: Option<&str>, bin: &'a Value, ) -> Result<&'a str, AnyError> { let bin_entry = match bin { Value::String(_) => { - if bin_name.is_some() && bin_name.unwrap() != pkg_req.name { + if bin_name.is_some() && bin_name.unwrap() != pkg_nv.name { None } else { Some(bin) @@ -332,10 +332,10 @@ fn resolve_bin_entry_value<'a>( } else if o.len() == 1 || o.len() > 1 && o.values().all(|v| v == o.values().next().unwrap()) { o.values().next() } else { - o.get(&pkg_req.name) + o.get(&pkg_nv.name) } }, - _ => bail!("package '{}' did not have a bin property with a string or object value in its package.json", pkg_req.name), + _ => bail!("package '{}' did not have a bin property with a string or object value in its package.json", pkg_nv), }; let bin_entry = match bin_entry { Some(e) => e, @@ -345,14 +345,14 @@ fn resolve_bin_entry_value<'a>( .map(|o| { o.keys() .into_iter() - .map(|k| format!(" * npm:{pkg_req}/{k}")) + .map(|k| format!(" * npm:{pkg_nv}/{k}")) .collect::<Vec<_>>() }) .unwrap_or_default(); bail!( "package '{}' did not have a bin entry for '{}' in its package.json{}", - pkg_req.name, - bin_name.unwrap_or(&pkg_req.name), + pkg_nv, + bin_name.unwrap_or(&pkg_nv.name), if keys.is_empty() { "".to_string() } else { @@ -365,7 +365,7 @@ fn resolve_bin_entry_value<'a>( Value::String(s) => Ok(s), _ => bail!( "package '{}' had a non-string sub property of bin in its package.json", - pkg_req.name, + pkg_nv, ), } } @@ -982,7 +982,7 @@ mod tests { }); assert_eq!( resolve_bin_entry_value( - &NpmPackageReq::from_str("test").unwrap(), + &NpmPackageNv::from_str("test@1.1.1").unwrap(), Some("bin1"), &value ) @@ -993,7 +993,7 @@ mod tests { // should resolve the value with the same name when not specified assert_eq!( resolve_bin_entry_value( - &NpmPackageReq::from_str("test").unwrap(), + &NpmPackageNv::from_str("test@1.1.1").unwrap(), None, &value ) @@ -1004,7 +1004,7 @@ mod tests { // should not resolve when specified value does not exist assert_eq!( resolve_bin_entry_value( - &NpmPackageReq::from_str("test").unwrap(), + &NpmPackageNv::from_str("test@1.1.1").unwrap(), Some("other"), &value ) @@ -1012,19 +1012,19 @@ mod tests { .unwrap() .to_string(), concat!( - "package 'test' did not have a bin entry for 'other' in its package.json\n", + "package 'test@1.1.1' did not have a bin entry for 'other' in its package.json\n", "\n", "Possibilities:\n", - " * npm:test/bin1\n", - " * npm:test/bin2\n", - " * npm:test/test" + " * npm:test@1.1.1/bin1\n", + " * npm:test@1.1.1/bin2\n", + " * npm:test@1.1.1/test" ) ); // should not resolve when default value can't be determined assert_eq!( resolve_bin_entry_value( - &NpmPackageReq::from_str("asdf@1.2").unwrap(), + &NpmPackageNv::from_str("asdf@1.2.3").unwrap(), None, &value ) @@ -1032,12 +1032,12 @@ mod tests { .unwrap() .to_string(), concat!( - "package 'asdf' did not have a bin entry for 'asdf' in its package.json\n", + "package 'asdf@1.2.3' did not have a bin entry for 'asdf' in its package.json\n", "\n", "Possibilities:\n", - " * npm:asdf@1.2/bin1\n", - " * npm:asdf@1.2/bin2\n", - " * npm:asdf@1.2/test" + " * npm:asdf@1.2.3/bin1\n", + " * npm:asdf@1.2.3/bin2\n", + " * npm:asdf@1.2.3/test" ) ); @@ -1048,7 +1048,7 @@ mod tests { }); assert_eq!( resolve_bin_entry_value( - &NpmPackageReq::from_str("test").unwrap(), + &NpmPackageNv::from_str("test@1.2.3").unwrap(), None, &value ) @@ -1060,14 +1060,14 @@ mod tests { let value = json!("./value"); assert_eq!( resolve_bin_entry_value( - &NpmPackageReq::from_str("test").unwrap(), + &NpmPackageNv::from_str("test@1.2.3").unwrap(), Some("path"), &value ) .err() .unwrap() .to_string(), - "package 'test' did not have a bin entry for 'path' in its package.json" + "package 'test@1.2.3' did not have a bin entry for 'path' in its package.json" ); } } diff --git a/cli/npm/cache.rs b/cli/npm/cache.rs index b2d932911..9f58bcd0d 100644 --- a/cli/npm/cache.rs +++ b/cli/npm/cache.rs @@ -36,7 +36,7 @@ pub fn should_sync_download() -> bool { const NPM_PACKAGE_SYNC_LOCK_FILENAME: &str = ".deno_sync_lock"; pub fn with_folder_sync_lock( - package: (&str, &Version), + package: &NpmPackageNv, output_folder: &Path, action: impl FnOnce() -> Result<(), AnyError>, ) -> Result<(), AnyError> { @@ -88,14 +88,13 @@ pub fn with_folder_sync_lock( if remove_err.kind() != std::io::ErrorKind::NotFound { bail!( concat!( - "Failed setting up package cache directory for {}@{}, then ", + "Failed setting up package cache directory for {}, then ", "failed cleaning it up.\n\nOriginal error:\n\n{}\n\n", "Remove error:\n\n{}\n\nPlease manually ", "delete this folder or you will run into issues using this ", "package in the future:\n\n{}" ), - package.0, - package.1, + package, err, remove_err, output_folder.display(), @@ -182,31 +181,26 @@ impl ReadonlyNpmCache { pub fn package_folder_for_id( &self, - id: &NpmPackageCacheFolderId, + folder_id: &NpmPackageCacheFolderId, registry_url: &Url, ) -> PathBuf { - if id.copy_index == 0 { - self.package_folder_for_name_and_version( - &id.nv.name, - &id.nv.version, - registry_url, - ) + if folder_id.copy_index == 0 { + self.package_folder_for_name_and_version(&folder_id.nv, registry_url) } else { self - .package_name_folder(&id.nv.name, registry_url) - .join(format!("{}_{}", id.nv.version, id.copy_index)) + .package_name_folder(&folder_id.nv.name, registry_url) + .join(format!("{}_{}", folder_id.nv.version, folder_id.copy_index)) } } pub fn package_folder_for_name_and_version( &self, - name: &str, - version: &Version, + package: &NpmPackageNv, registry_url: &Url, ) -> PathBuf { self - .package_name_folder(name, registry_url) - .join(version.to_string()) + .package_name_folder(&package.name, registry_url) + .join(package.version.to_string()) } pub fn package_name_folder(&self, name: &str, registry_url: &Url) -> PathBuf { @@ -324,7 +318,7 @@ pub struct NpmCache { http_client: HttpClient, progress_bar: ProgressBar, /// ensures a package is only downloaded once per run - previously_reloaded_packages: Arc<Mutex<HashSet<String>>>, + previously_reloaded_packages: Arc<Mutex<HashSet<NpmPackageNv>>>, } impl NpmCache { @@ -358,40 +352,36 @@ impl NpmCache { /// and imports a dynamic import that imports the same package again for example. fn should_use_global_cache_for_package( &self, - package: (&str, &Version), + package: &NpmPackageNv, ) -> bool { - self.cache_setting.should_use_for_npm_package(package.0) + self.cache_setting.should_use_for_npm_package(&package.name) || !self .previously_reloaded_packages .lock() - .insert(format!("{}@{}", package.0, package.1)) + .insert(package.clone()) } pub async fn ensure_package( &self, - package: (&str, &Version), + package: &NpmPackageNv, dist: &NpmPackageVersionDistInfo, registry_url: &Url, ) -> Result<(), AnyError> { self .ensure_package_inner(package, dist, registry_url) .await - .with_context(|| { - format!("Failed caching npm package '{}@{}'.", package.0, package.1) - }) + .with_context(|| format!("Failed caching npm package '{package}'.")) } async fn ensure_package_inner( &self, - package: (&str, &Version), + package: &NpmPackageNv, dist: &NpmPackageVersionDistInfo, registry_url: &Url, ) -> Result<(), AnyError> { - let package_folder = self.readonly.package_folder_for_name_and_version( - package.0, - package.1, - registry_url, - ); + let package_folder = self + .readonly + .package_folder_for_name_and_version(package, registry_url); if self.should_use_global_cache_for_package(package) && package_folder.exists() // if this file exists, then the package didn't successfully extract @@ -404,7 +394,7 @@ impl NpmCache { "NotCached", format!( "An npm specifier not found in cache: \"{}\", --cached-only is specified.", - &package.0 + &package.name ) ) ); @@ -431,32 +421,28 @@ impl NpmCache { /// from exists before this is called. pub fn ensure_copy_package( &self, - id: &NpmPackageCacheFolderId, + folder_id: &NpmPackageCacheFolderId, registry_url: &Url, ) -> Result<(), AnyError> { - assert_ne!(id.copy_index, 0); - let package_folder = self.readonly.package_folder_for_id(id, registry_url); + assert_ne!(folder_id.copy_index, 0); + let package_folder = + self.readonly.package_folder_for_id(folder_id, registry_url); if package_folder.exists() // if this file exists, then the package didn't successfully extract // the first time, or another process is currently extracting the zip file && !package_folder.join(NPM_PACKAGE_SYNC_LOCK_FILENAME).exists() - && self.cache_setting.should_use_for_npm_package(&id.nv.name) + && self.cache_setting.should_use_for_npm_package(&folder_id.nv.name) { return Ok(()); } - let original_package_folder = - self.readonly.package_folder_for_name_and_version( - &id.nv.name, - &id.nv.version, - registry_url, - ); - with_folder_sync_lock( - (id.nv.name.as_str(), &id.nv.version), - &package_folder, - || hard_link_dir_recursive(&original_package_folder, &package_folder), - )?; + let original_package_folder = self + .readonly + .package_folder_for_name_and_version(&folder_id.nv, registry_url); + with_folder_sync_lock(&folder_id.nv, &package_folder, || { + hard_link_dir_recursive(&original_package_folder, &package_folder) + })?; Ok(()) } @@ -470,15 +456,12 @@ impl NpmCache { pub fn package_folder_for_name_and_version( &self, - name: &str, - version: &Version, + package: &NpmPackageNv, registry_url: &Url, ) -> PathBuf { - self.readonly.package_folder_for_name_and_version( - name, - version, - registry_url, - ) + self + .readonly + .package_folder_for_name_and_version(package, registry_url) } pub fn package_name_folder(&self, name: &str, registry_url: &Url) -> PathBuf { diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 2a58bb01a..602b4ad44 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -6,12 +6,15 @@ mod resolution; mod resolvers; mod tarball; +pub use cache::should_sync_download; pub use cache::NpmCache; #[cfg(test)] pub use registry::NpmPackageVersionDistInfo; pub use registry::NpmRegistryApi; -pub use resolution::resolve_graph_npm_info; +#[cfg(test)] +pub use registry::TestNpmRegistryApiInner; pub use resolution::NpmPackageId; +pub use resolution::NpmResolution; pub use resolution::NpmResolutionPackage; pub use resolution::NpmResolutionSnapshot; pub use resolvers::NpmPackageResolver; diff --git a/cli/npm/registry.rs b/cli/npm/registry.rs index bcdada30d..75760c171 100644 --- a/cli/npm/registry.rs +++ b/cli/npm/registry.rs @@ -232,6 +232,13 @@ impl NpmRegistryApi { })) } + /// Creates an npm registry API that will be uninitialized + /// and error for every request. This is useful for tests + /// or for initializing the LSP. + pub fn new_uninitialized() -> Self { + Self(Arc::new(NullNpmRegistryApiInner)) + } + #[cfg(test)] pub fn new_for_test(api: TestNpmRegistryApiInner) -> NpmRegistryApi { Self(Arc::new(api)) @@ -294,6 +301,13 @@ impl NpmRegistryApi { self.0.clear_memory_cache(); } + pub fn get_cached_package_info( + &self, + name: &str, + ) -> Option<Arc<NpmPackageInfo>> { + self.0.get_cached_package_info(name) + } + pub fn base_url(&self) -> &Url { self.0.base_url() } diff --git a/cli/npm/resolution/graph.rs b/cli/npm/resolution/graph.rs index 966e1f010..87579dad3 100644 --- a/cli/npm/resolution/graph.rs +++ b/cli/npm/resolution/graph.rs @@ -159,6 +159,9 @@ impl ResolvedNodeIds { } } +// todo(dsherret): for some reason the lsp errors when using an Rc<RefCell<NodeId>> here +// instead of an Arc<Mutex<NodeId>>. We should investigate and fix. + /// A pointer to a specific node in a graph path. The underlying node id /// may change as peer dependencies are created. #[derive(Clone, Debug)] @@ -297,6 +300,8 @@ pub struct Graph { // This will be set when creating from a snapshot, then // inform the final snapshot creation. packages_to_copy_index: HashMap<NpmPackageId, usize>, + /// Packages that the resolver should resolve first. + pending_unresolved_packages: Vec<NpmPackageNv>, } impl Graph { @@ -359,6 +364,7 @@ impl Graph { .map(|(id, p)| (id.clone(), p.copy_index)) .collect(), package_reqs: snapshot.package_reqs, + pending_unresolved_packages: snapshot.pending_unresolved_packages, ..Default::default() }; let mut created_package_ids = @@ -375,10 +381,18 @@ impl Graph { Ok(graph) } + pub fn take_pending_unresolved(&mut self) -> Vec<NpmPackageNv> { + std::mem::take(&mut self.pending_unresolved_packages) + } + pub fn has_package_req(&self, req: &NpmPackageReq) -> bool { self.package_reqs.contains_key(req) } + pub fn has_root_package(&self, id: &NpmPackageNv) -> bool { + self.root_packages.contains_key(id) + } + fn get_npm_pkg_id(&self, node_id: NodeId) -> NpmPackageId { let resolved_id = self.resolved_node_ids.get(node_id).unwrap(); self.get_npm_pkg_id_from_resolved_id(resolved_id, HashSet::new()) @@ -596,6 +610,7 @@ impl Graph { .collect(), packages, package_reqs: self.package_reqs, + pending_unresolved_packages: self.pending_unresolved_packages, }) } @@ -714,11 +729,43 @@ impl<'a> GraphDependencyResolver<'a> { } } + pub fn add_root_package( + &mut self, + package_nv: &NpmPackageNv, + package_info: &NpmPackageInfo, + ) -> Result<(), AnyError> { + if self.graph.root_packages.contains_key(package_nv) { + return Ok(()); // already added + } + + // todo(dsherret): using a version requirement here is a temporary hack + // to reuse code in a large refactor. We should resolve the node directly + // from the package name and version + let version_req = + VersionReq::parse_from_specifier(&format!("{}", package_nv.version)) + .unwrap(); + let (pkg_id, node_id) = self.resolve_node_from_info( + &package_nv.name, + &version_req, + package_info, + None, + )?; + self.graph.root_packages.insert(pkg_id.clone(), node_id); + self + .pending_unresolved_nodes + .push_back(GraphPath::for_root(node_id, pkg_id)); + Ok(()) + } + pub fn add_package_req( &mut self, package_req: &NpmPackageReq, package_info: &NpmPackageInfo, ) -> Result<(), AnyError> { + if self.graph.package_reqs.contains_key(package_req) { + return Ok(()); // already added + } + let (pkg_id, node_id) = self.resolve_node_from_info( &package_req.name, package_req diff --git a/cli/npm/resolution/mod.rs b/cli/npm/resolution/mod.rs index c95124b61..8584958b5 100644 --- a/cli/npm/resolution/mod.rs +++ b/cli/npm/resolution/mod.rs @@ -4,19 +4,26 @@ use std::cmp::Ordering; use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; +use std::sync::Arc; use deno_core::anyhow::Context; use deno_core::error::AnyError; +use deno_core::parking_lot::Mutex; use deno_core::parking_lot::RwLock; use deno_graph::npm::NpmPackageNv; +use deno_graph::npm::NpmPackageNvReference; use deno_graph::npm::NpmPackageReq; +use deno_graph::npm::NpmPackageReqReference; use deno_graph::semver::Version; +use log::debug; use serde::Deserialize; use serde::Serialize; use thiserror::Error; use crate::args::Lockfile; +use crate::npm::resolution::common::LATEST_VERSION_REQ; +use self::common::resolve_best_package_version_and_info; use self::graph::GraphDependencyResolver; use self::snapshot::NpmPackagesPartitioned; @@ -27,11 +34,9 @@ use super::registry::NpmRegistryApi; mod common; mod graph; mod snapshot; -mod specifier; use graph::Graph; pub use snapshot::NpmResolutionSnapshot; -pub use specifier::resolve_graph_npm_info; #[derive(Debug, Error)] #[error("Invalid npm package id '{text}'. {message}")] @@ -230,15 +235,19 @@ impl NpmResolutionPackage { } } -pub struct NpmResolution { +#[derive(Clone)] +pub struct NpmResolution(Arc<NpmResolutionInner>); + +struct NpmResolutionInner { api: NpmRegistryApi, snapshot: RwLock<NpmResolutionSnapshot>, update_semaphore: tokio::sync::Semaphore, + maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, } impl std::fmt::Debug for NpmResolution { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let snapshot = self.snapshot.read(); + let snapshot = self.0.snapshot.read(); f.debug_struct("NpmResolution") .field("snapshot", &snapshot) .finish() @@ -249,26 +258,35 @@ impl NpmResolution { pub fn new( api: NpmRegistryApi, initial_snapshot: Option<NpmResolutionSnapshot>, + maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, ) -> Self { - Self { + Self(Arc::new(NpmResolutionInner { api, snapshot: RwLock::new(initial_snapshot.unwrap_or_default()), update_semaphore: tokio::sync::Semaphore::new(1), - } + maybe_lockfile, + })) } pub async fn add_package_reqs( &self, package_reqs: Vec<NpmPackageReq>, ) -> Result<(), AnyError> { + let inner = &self.0; + // only allow one thread in here at a time - let _permit = self.update_semaphore.acquire().await?; - let snapshot = self.snapshot.read().clone(); + let _permit = inner.update_semaphore.acquire().await?; + let snapshot = inner.snapshot.read().clone(); - let snapshot = - add_package_reqs_to_snapshot(&self.api, package_reqs, snapshot).await?; + let snapshot = add_package_reqs_to_snapshot( + &inner.api, + package_reqs, + snapshot, + self.0.maybe_lockfile.clone(), + ) + .await?; - *self.snapshot.write() = snapshot; + *inner.snapshot.write() = snapshot; Ok(()) } @@ -276,9 +294,10 @@ impl NpmResolution { &self, package_reqs: HashSet<NpmPackageReq>, ) -> Result<(), AnyError> { + let inner = &self.0; // only allow one thread in here at a time - let _permit = self.update_semaphore.acquire().await?; - let snapshot = self.snapshot.read().clone(); + let _permit = inner.update_semaphore.acquire().await?; + let snapshot = inner.snapshot.read().clone(); let has_removed_package = !snapshot .package_reqs @@ -291,22 +310,46 @@ impl NpmResolution { snapshot }; let snapshot = add_package_reqs_to_snapshot( - &self.api, + &inner.api, package_reqs.into_iter().collect(), snapshot, + self.0.maybe_lockfile.clone(), ) .await?; - *self.snapshot.write() = snapshot; + *inner.snapshot.write() = snapshot; Ok(()) } - pub fn resolve_package_from_id( + pub async fn resolve_pending(&self) -> Result<(), AnyError> { + let inner = &self.0; + // only allow one thread in here at a time + let _permit = inner.update_semaphore.acquire().await?; + let snapshot = inner.snapshot.read().clone(); + + let snapshot = add_package_reqs_to_snapshot( + &inner.api, + Vec::new(), + snapshot, + self.0.maybe_lockfile.clone(), + ) + .await?; + + *inner.snapshot.write() = snapshot; + + Ok(()) + } + + pub fn pkg_req_ref_to_nv_ref( &self, - id: &NpmPackageId, - ) -> Option<NpmResolutionPackage> { - self.snapshot.read().package_from_id(id).cloned() + req_ref: NpmPackageReqReference, + ) -> Result<NpmPackageNvReference, AnyError> { + let node_id = self.resolve_pkg_id_from_pkg_req(&req_ref.req)?; + Ok(NpmPackageNvReference { + nv: node_id.nv, + sub_path: req_ref.sub_path, + }) } pub fn resolve_package_cache_folder_id_from_id( @@ -314,6 +357,7 @@ impl NpmResolution { id: &NpmPackageId, ) -> Option<NpmPackageCacheFolderId> { self + .0 .snapshot .read() .package_from_id(id) @@ -326,6 +370,7 @@ impl NpmResolution { referrer: &NpmPackageCacheFolderId, ) -> Result<NpmResolutionPackage, AnyError> { self + .0 .snapshot .read() .resolve_package_from_package(name, referrer) @@ -333,36 +378,100 @@ impl NpmResolution { } /// Resolve a node package from a deno module. - pub fn resolve_package_from_deno_module( + pub fn resolve_pkg_id_from_pkg_req( &self, - package: &NpmPackageReq, - ) -> Result<NpmResolutionPackage, AnyError> { + req: &NpmPackageReq, + ) -> Result<NpmPackageId, AnyError> { self + .0 .snapshot .read() - .resolve_package_from_deno_module(package) - .cloned() + .resolve_pkg_from_pkg_req(req) + .map(|pkg| pkg.pkg_id.clone()) + } + + pub fn resolve_pkg_id_from_deno_module( + &self, + id: &NpmPackageNv, + ) -> Result<NpmPackageId, AnyError> { + self + .0 + .snapshot + .read() + .resolve_package_from_deno_module(id) + .map(|pkg| pkg.pkg_id.clone()) + } + + /// Resolves a package requirement for deno graph. This should only be + /// called by deno_graph's NpmResolver. + pub fn resolve_package_req_for_deno_graph( + &self, + pkg_req: &NpmPackageReq, + ) -> Result<NpmPackageNv, AnyError> { + let inner = &self.0; + // we should always have this because it should have been cached before here + let package_info = + inner.api.get_cached_package_info(&pkg_req.name).unwrap(); + + let mut snapshot = inner.snapshot.write(); + let version_req = + pkg_req.version_req.as_ref().unwrap_or(&*LATEST_VERSION_REQ); + let version_and_info = + match snapshot.packages_by_name.get(&package_info.name) { + Some(existing_versions) => resolve_best_package_version_and_info( + version_req, + &package_info, + existing_versions.iter().map(|p| &p.nv.version), + )?, + None => resolve_best_package_version_and_info( + version_req, + &package_info, + Vec::new().iter(), + )?, + }; + let id = NpmPackageNv { + name: package_info.name.to_string(), + version: version_and_info.version, + }; + debug!( + "Resolved {}@{} to {}", + pkg_req.name, + version_req.version_text(), + id.to_string(), + ); + snapshot.package_reqs.insert(pkg_req.clone(), id.clone()); + let packages_with_name = snapshot + .packages_by_name + .entry(package_info.name.clone()) + .or_default(); + if !packages_with_name.iter().any(|p| p.nv == id) { + packages_with_name.push(NpmPackageId { + nv: id.clone(), + peer_dependencies: Vec::new(), + }); + } + snapshot.pending_unresolved_packages.push(id.clone()); + Ok(id) } pub fn all_packages_partitioned(&self) -> NpmPackagesPartitioned { - self.snapshot.read().all_packages_partitioned() + self.0.snapshot.read().all_packages_partitioned() } pub fn has_packages(&self) -> bool { - !self.snapshot.read().packages.is_empty() + !self.0.snapshot.read().packages.is_empty() } pub fn snapshot(&self) -> NpmResolutionSnapshot { - self.snapshot.read().clone() + self.0.snapshot.read().clone() } pub fn lock(&self, lockfile: &mut Lockfile) -> Result<(), AnyError> { - let snapshot = self.snapshot.read(); + let snapshot = self.0.snapshot.read(); for (package_req, nv) in snapshot.package_reqs.iter() { - let package_id = snapshot.root_packages.get(nv).unwrap(); lockfile.insert_npm_specifier( package_req.to_string(), - package_id.as_serialized(), + snapshot.root_packages.get(nv).unwrap().as_serialized(), ); } for package in snapshot.all_packages() { @@ -376,10 +485,12 @@ async fn add_package_reqs_to_snapshot( api: &NpmRegistryApi, package_reqs: Vec<NpmPackageReq>, snapshot: NpmResolutionSnapshot, + maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, ) -> Result<NpmResolutionSnapshot, AnyError> { - if package_reqs - .iter() - .all(|req| snapshot.package_reqs.contains_key(req)) + if snapshot.pending_unresolved_packages.is_empty() + && package_reqs + .iter() + .all(|req| snapshot.package_reqs.contains_key(req)) { return Ok(snapshot); // already up to date } @@ -390,39 +501,72 @@ async fn add_package_reqs_to_snapshot( "Failed creating npm state. Try recreating your lockfile." ) })?; + let pending_unresolved = graph.take_pending_unresolved(); // avoid loading the info if this is already in the graph let package_reqs = package_reqs .into_iter() .filter(|r| !graph.has_package_req(r)) .collect::<Vec<_>>(); + let pending_unresolved = pending_unresolved + .into_iter() + .filter(|p| !graph.has_root_package(p)) + .collect::<Vec<_>>(); - // go over the top level package names first, then down the tree - // one level at a time through all the branches + // cache the packages in parallel api .cache_in_parallel( package_reqs .iter() - .map(|r| r.name.clone()) + .map(|req| req.name.clone()) + .chain(pending_unresolved.iter().map(|id| id.name.clone())) + .collect::<HashSet<_>>() .into_iter() .collect::<Vec<_>>(), ) .await?; + // go over the top level package names first (npm package reqs and pending unresolved), + // then down the tree one level at a time through all the branches let mut resolver = GraphDependencyResolver::new(&mut graph, api); - // The package reqs should already be sorted + // The package reqs and ids should already be sorted // in the order they should be resolved in. for package_req in package_reqs { let info = api.package_info(&package_req.name).await?; resolver.add_package_req(&package_req, &info)?; } + for pkg_id in pending_unresolved { + let info = api.package_info(&pkg_id.name).await?; + resolver.add_root_package(&pkg_id, &info)?; + } + resolver.resolve_pending().await?; let result = graph.into_snapshot(api).await; api.clear_memory_cache(); - result + + if let Some(lockfile_mutex) = maybe_lockfile { + let mut lockfile = lockfile_mutex.lock(); + match result { + Ok(snapshot) => { + for (package_req, nv) in snapshot.package_reqs.iter() { + lockfile.insert_npm_specifier( + package_req.to_string(), + snapshot.root_packages.get(nv).unwrap().as_serialized(), + ); + } + for package in snapshot.all_packages() { + lockfile.check_or_insert_npm_package(package.into())?; + } + Ok(snapshot) + } + Err(err) => Err(err), + } + } else { + result + } } #[cfg(test)] diff --git a/cli/npm/resolution/snapshot.rs b/cli/npm/resolution/snapshot.rs index 3fc82cbb8..e986294ec 100644 --- a/cli/npm/resolution/snapshot.rs +++ b/cli/npm/resolution/snapshot.rs @@ -54,6 +54,9 @@ pub struct NpmResolutionSnapshot { pub(super) packages_by_name: HashMap<String, Vec<NpmPackageId>>, #[serde(with = "map_to_vec")] pub(super) packages: HashMap<NpmPackageId, NpmResolutionPackage>, + /// Ordered list based on resolution of packages whose dependencies + /// have not yet been resolved + pub(super) pending_unresolved_packages: Vec<NpmPackageNv>, } impl std::fmt::Debug for NpmResolutionSnapshot { @@ -76,6 +79,10 @@ impl std::fmt::Debug for NpmResolutionSnapshot { "packages", &self.packages.iter().collect::<BTreeMap<_, _>>(), ) + .field( + "pending_unresolved_packages", + &self.pending_unresolved_packages, + ) .finish() } } @@ -120,22 +127,28 @@ mod map_to_vec { } impl NpmResolutionSnapshot { - /// Resolve a node package from a deno module. - pub fn resolve_package_from_deno_module( + /// Resolve a package from a package requirement. + pub fn resolve_pkg_from_pkg_req( &self, req: &NpmPackageReq, ) -> Result<&NpmResolutionPackage, AnyError> { - match self - .package_reqs - .get(req) - .and_then(|nv| self.root_packages.get(nv)) - .and_then(|id| self.packages.get(id)) - { - Some(id) => Ok(id), + match self.package_reqs.get(req) { + Some(id) => self.resolve_package_from_deno_module(id), None => bail!("could not find npm package directory for '{}'", req), } } + /// Resolve a package from a deno module. + pub fn resolve_package_from_deno_module( + &self, + id: &NpmPackageNv, + ) -> Result<&NpmResolutionPackage, AnyError> { + match self.root_packages.get(id) { + Some(id) => Ok(self.packages.get(id).unwrap()), + None => bail!("could not find npm package directory for '{}'", id), + } + } + pub fn top_level_packages(&self) -> Vec<NpmPackageId> { self.root_packages.values().cloned().collect::<Vec<_>>() } @@ -342,6 +355,7 @@ impl NpmResolutionSnapshot { root_packages, packages_by_name, packages, + pending_unresolved_packages: Default::default(), }) } } diff --git a/cli/npm/resolution/specifier.rs b/cli/npm/resolution/specifier.rs deleted file mode 100644 index 29d65c747..000000000 --- a/cli/npm/resolution/specifier.rs +++ /dev/null @@ -1,666 +0,0 @@ -// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. - -use std::cmp::Ordering; -use std::collections::HashMap; -use std::collections::HashSet; -use std::collections::VecDeque; - -use deno_ast::ModuleSpecifier; -use deno_graph::npm::NpmPackageReq; -use deno_graph::npm::NpmPackageReqReference; -use deno_graph::ModuleGraph; - -pub struct GraphNpmInfo { - /// The order of these package requirements is the order they - /// should be resolved in. - pub package_reqs: Vec<NpmPackageReq>, - /// Gets if the graph had a built-in node specifier (ex. `node:fs`). - pub has_node_builtin_specifier: bool, -} - -/// Resolves npm specific information from the graph. -/// -/// This function will analyze the module graph for parent-most folder -/// specifiers of all modules, then group npm specifiers together as found in -/// those descendant modules and return them in the order found spreading out -/// from the root of the graph. -/// -/// For example, given the following module graph: -/// -/// file:///dev/local_module_a/mod.ts -/// ├── npm:package-a@1 -/// ├─┬ https://deno.land/x/module_d/mod.ts -/// │ └─┬ https://deno.land/x/module_d/other.ts -/// │ └── npm:package-a@3 -/// ├─┬ file:///dev/local_module_a/other.ts -/// │ └── npm:package-b@2 -/// ├─┬ file:///dev/local_module_b/mod.ts -/// │ └── npm:package-b@2 -/// └─┬ https://deno.land/x/module_a/mod.ts -/// ├── npm:package-a@4 -/// ├── npm:package-c@5 -/// ├─┬ https://deno.land/x/module_c/sub_folder/mod.ts -/// │ ├── https://deno.land/x/module_c/mod.ts -/// │ ├─┬ https://deno.land/x/module_d/sub_folder/mod.ts -/// │ │ └── npm:package-other@2 -/// │ └── npm:package-c@5 -/// └── https://deno.land/x/module_b/mod.ts -/// -/// The graph above would be grouped down to the topmost specifier folders like -/// so and npm specifiers under each path would be resolved for that group -/// prioritizing file specifiers and sorting by end folder name alphabetically: -/// -/// file:///dev/local_module_a/ -/// ├── file:///dev/local_module_b/ -/// ├─┬ https://deno.land/x/module_a/ -/// │ ├── https://deno.land/x/module_b/ -/// │ └─┬ https://deno.land/x/module_c/ -/// │ └── https://deno.land/x/module_d/ -/// └── https://deno.land/x/module_d/ -/// -/// Then it would resolve the npm specifiers in each of those groups according -/// to that tree going by tree depth. -pub fn resolve_graph_npm_info(graph: &ModuleGraph) -> GraphNpmInfo { - fn collect_specifiers<'a>( - graph: &'a ModuleGraph, - module: &'a deno_graph::Module, - ) -> Vec<&'a ModuleSpecifier> { - let mut specifiers = Vec::with_capacity(module.dependencies.len() * 2 + 1); - let maybe_types = module - .maybe_types_dependency - .as_ref() - .map(|d| &d.dependency); - if let Some(specifier) = maybe_types.and_then(|d| d.maybe_specifier()) { - specifiers.push(specifier); - } - for dep in module.dependencies.values() { - #[allow(clippy::manual_flatten)] - for resolved in [&dep.maybe_code, &dep.maybe_type] { - if let Some(specifier) = resolved.maybe_specifier() { - specifiers.push(specifier); - } - } - } - - // flatten any data urls into this list of specifiers - for i in (0..specifiers.len()).rev() { - if specifiers[i].scheme() == "data" { - let data_specifier = specifiers.swap_remove(i); - if let Some(module) = graph.get(data_specifier) { - specifiers.extend(collect_specifiers(graph, module)); - } - } - } - - specifiers - } - - fn analyze_module( - module: &deno_graph::Module, - graph: &ModuleGraph, - specifier_graph: &mut SpecifierTree, - seen: &mut HashSet<ModuleSpecifier>, - has_node_builtin_specifier: &mut bool, - ) { - if !seen.insert(module.specifier.clone()) { - return; // already visited - } - - let parent_specifier = get_folder_path_specifier(&module.specifier); - let leaf = specifier_graph.get_leaf(&parent_specifier); - - let specifiers = collect_specifiers(graph, module); - - // fill this leaf's information - for specifier in &specifiers { - if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) { - leaf.reqs.insert(npm_ref.req); - } else if !specifier.as_str().starts_with(parent_specifier.as_str()) { - leaf - .dependencies - .insert(get_folder_path_specifier(specifier)); - } - - if !*has_node_builtin_specifier && specifier.scheme() == "node" { - *has_node_builtin_specifier = true; - } - } - - // now visit all the dependencies - for specifier in &specifiers { - if let Some(module) = graph.get(specifier) { - analyze_module( - module, - graph, - specifier_graph, - seen, - has_node_builtin_specifier, - ); - } - } - } - - let root_specifiers = graph - .roots - .iter() - .map(|url| graph.resolve(url)) - .collect::<Vec<_>>(); - let mut seen = HashSet::new(); - let mut specifier_graph = SpecifierTree::default(); - let mut has_node_builtin_specifier = false; - for root in &root_specifiers { - if let Some(module) = graph.get(root) { - analyze_module( - module, - graph, - &mut specifier_graph, - &mut seen, - &mut has_node_builtin_specifier, - ); - } - } - - let mut seen = HashSet::new(); - let mut pending_specifiers = VecDeque::new(); - let mut result = Vec::new(); - - for specifier in &root_specifiers { - match NpmPackageReqReference::from_specifier(specifier) { - Ok(npm_ref) => result.push(npm_ref.req), - Err(_) => { - pending_specifiers.push_back(get_folder_path_specifier(specifier)) - } - } - } - - while let Some(specifier) = pending_specifiers.pop_front() { - let leaf = specifier_graph.get_leaf(&specifier); - if !seen.insert(leaf.specifier.clone()) { - continue; // already seen - } - - let reqs = std::mem::take(&mut leaf.reqs); - let mut reqs = reqs.into_iter().collect::<Vec<_>>(); - reqs.sort(); - result.extend(reqs); - - let mut deps = std::mem::take(&mut leaf.dependencies) - .into_iter() - .collect::<Vec<_>>(); - deps.sort_by(cmp_folder_specifiers); - - for dep in deps { - pending_specifiers.push_back(dep); - } - } - - GraphNpmInfo { - has_node_builtin_specifier, - package_reqs: result, - } -} - -fn get_folder_path_specifier(specifier: &ModuleSpecifier) -> ModuleSpecifier { - let mut specifier = specifier.clone(); - specifier.set_query(None); - specifier.set_fragment(None); - if !specifier.path().ends_with('/') { - // remove the last path part, but keep the trailing slash - let mut path_parts = specifier.path().split('/').collect::<Vec<_>>(); - let path_parts_len = path_parts.len(); // make borrow checker happy for some reason - if path_parts_len > 0 { - path_parts[path_parts_len - 1] = ""; - } - specifier.set_path(&path_parts.join("/")); - } - specifier -} - -#[derive(Debug)] -enum SpecifierTreeNode { - Parent(SpecifierTreeParentNode), - Leaf(SpecifierTreeLeafNode), -} - -impl SpecifierTreeNode { - pub fn mut_to_leaf(&mut self) { - if let SpecifierTreeNode::Parent(node) = self { - let node = std::mem::replace( - node, - SpecifierTreeParentNode { - specifier: node.specifier.clone(), - dependencies: Default::default(), - }, - ); - *self = SpecifierTreeNode::Leaf(node.into_leaf()); - } - } -} - -#[derive(Debug)] -struct SpecifierTreeParentNode { - specifier: ModuleSpecifier, - dependencies: HashMap<String, SpecifierTreeNode>, -} - -impl SpecifierTreeParentNode { - pub fn into_leaf(self) -> SpecifierTreeLeafNode { - fn fill_new_leaf( - deps: HashMap<String, SpecifierTreeNode>, - new_leaf: &mut SpecifierTreeLeafNode, - ) { - for node in deps.into_values() { - match node { - SpecifierTreeNode::Parent(node) => { - fill_new_leaf(node.dependencies, new_leaf) - } - SpecifierTreeNode::Leaf(leaf) => { - for dep in leaf.dependencies { - // don't insert if the dependency is found within the new leaf - if !dep.as_str().starts_with(new_leaf.specifier.as_str()) { - new_leaf.dependencies.insert(dep); - } - } - new_leaf.reqs.extend(leaf.reqs); - } - } - } - } - - let mut new_leaf = SpecifierTreeLeafNode { - specifier: self.specifier, - reqs: Default::default(), - dependencies: Default::default(), - }; - fill_new_leaf(self.dependencies, &mut new_leaf); - new_leaf - } -} - -#[derive(Debug)] -struct SpecifierTreeLeafNode { - specifier: ModuleSpecifier, - reqs: HashSet<NpmPackageReq>, - dependencies: HashSet<ModuleSpecifier>, -} - -#[derive(Default)] -struct SpecifierTree { - root_nodes: HashMap<ModuleSpecifier, SpecifierTreeNode>, -} - -impl SpecifierTree { - pub fn get_leaf( - &mut self, - specifier: &ModuleSpecifier, - ) -> &mut SpecifierTreeLeafNode { - let root_specifier = { - let mut specifier = specifier.clone(); - specifier.set_path(""); - specifier - }; - let root_node = self - .root_nodes - .entry(root_specifier.clone()) - .or_insert_with(|| { - SpecifierTreeNode::Parent(SpecifierTreeParentNode { - specifier: root_specifier.clone(), - dependencies: Default::default(), - }) - }); - let mut current_node = root_node; - if !matches!(specifier.path(), "" | "/") { - let mut current_parts = Vec::new(); - let path = specifier.path(); - for part in path[1..path.len() - 1].split('/') { - current_parts.push(part); - match current_node { - SpecifierTreeNode::Leaf(leaf) => return leaf, - SpecifierTreeNode::Parent(node) => { - current_node = node - .dependencies - .entry(part.to_string()) - .or_insert_with(|| { - SpecifierTreeNode::Parent(SpecifierTreeParentNode { - specifier: { - let mut specifier = root_specifier.clone(); - specifier.set_path(¤t_parts.join("/")); - specifier - }, - dependencies: Default::default(), - }) - }); - } - } - } - } - current_node.mut_to_leaf(); - match current_node { - SpecifierTreeNode::Leaf(leaf) => leaf, - _ => unreachable!(), - } - } -} - -// prefer file: specifiers, then sort by folder name, then by specifier -fn cmp_folder_specifiers(a: &ModuleSpecifier, b: &ModuleSpecifier) -> Ordering { - fn order_folder_name(path_a: &str, path_b: &str) -> Option<Ordering> { - let path_a = path_a.trim_end_matches('/'); - let path_b = path_b.trim_end_matches('/'); - match path_a.rfind('/') { - Some(a_index) => match path_b.rfind('/') { - Some(b_index) => match path_a[a_index..].cmp(&path_b[b_index..]) { - Ordering::Equal => None, - ordering => Some(ordering), - }, - None => None, - }, - None => None, - } - } - - fn order_specifiers(a: &ModuleSpecifier, b: &ModuleSpecifier) -> Ordering { - match order_folder_name(a.path(), b.path()) { - Some(ordering) => ordering, - None => a.as_str().cmp(b.as_str()), // fallback to just comparing the entire url - } - } - - if a.scheme() == "file" { - if b.scheme() == "file" { - order_specifiers(a, b) - } else { - Ordering::Less - } - } else if b.scheme() == "file" { - Ordering::Greater - } else { - order_specifiers(a, b) - } -} - -#[cfg(test)] -mod tests { - use pretty_assertions::assert_eq; - - use super::*; - - #[test] - fn sorting_folder_specifiers() { - fn cmp(a: &str, b: &str) -> Ordering { - let a = ModuleSpecifier::parse(a).unwrap(); - let b = ModuleSpecifier::parse(b).unwrap(); - cmp_folder_specifiers(&a, &b) - } - - // prefer file urls - assert_eq!( - cmp("file:///test/", "https://deno.land/x/module/"), - Ordering::Less - ); - assert_eq!( - cmp("https://deno.land/x/module/", "file:///test/"), - Ordering::Greater - ); - - // sort by folder name - assert_eq!( - cmp( - "https://deno.land/x/module_a/", - "https://deno.land/x/module_b/" - ), - Ordering::Less - ); - assert_eq!( - cmp( - "https://deno.land/x/module_b/", - "https://deno.land/x/module_a/" - ), - Ordering::Greater - ); - assert_eq!( - cmp( - "https://deno.land/x/module_a/", - "https://deno.land/std/module_b/" - ), - Ordering::Less - ); - assert_eq!( - cmp( - "https://deno.land/std/module_b/", - "https://deno.land/x/module_a/" - ), - Ordering::Greater - ); - - // by specifier, since folder names match - assert_eq!( - cmp( - "https://deno.land/std/module_a/", - "https://deno.land/x/module_a/" - ), - Ordering::Less - ); - } - - #[test] - fn test_get_folder_path_specifier() { - fn get(a: &str) -> String { - get_folder_path_specifier(&ModuleSpecifier::parse(a).unwrap()).to_string() - } - - assert_eq!(get("https://deno.land/"), "https://deno.land/"); - assert_eq!(get("https://deno.land"), "https://deno.land/"); - assert_eq!(get("https://deno.land/test"), "https://deno.land/"); - assert_eq!(get("https://deno.land/test/"), "https://deno.land/test/"); - assert_eq!( - get("https://deno.land/test/other"), - "https://deno.land/test/" - ); - assert_eq!( - get("https://deno.land/test/other/"), - "https://deno.land/test/other/" - ); - assert_eq!( - get("https://deno.land/test/other/test?test#other"), - "https://deno.land/test/other/" - ); - } - - #[tokio::test] - async fn test_resolve_npm_package_reqs() { - let mut loader = deno_graph::source::MemoryLoader::new( - vec![ - ( - "file:///dev/local_module_a/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "file:///dev/local_module_a/mod.ts".to_string(), - content: concat!( - "import 'https://deno.land/x/module_d/mod.ts';", - "import 'file:///dev/local_module_a/other.ts';", - "import 'file:///dev/local_module_b/mod.ts';", - "import 'https://deno.land/x/module_a/mod.ts';", - "import 'npm:package-a@local_module_a';", - "import 'https://deno.land/x/module_e/';", - ) - .to_string(), - maybe_headers: None, - }, - ), - ( - "file:///dev/local_module_a/other.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "file:///dev/local_module_a/other.ts".to_string(), - content: "import 'npm:package-b@local_module_a';".to_string(), - maybe_headers: None, - }, - ), - ( - "file:///dev/local_module_b/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "file:///dev/local_module_b/mod.ts".to_string(), - content: concat!( - "export * from 'npm:package-b@local_module_b';", - "import * as test from 'data:application/typescript,export%20*%20from%20%22npm:package-data%40local_module_b%22;';", - ).to_string(), - maybe_headers: None, - }, - ), - ( - "https://deno.land/x/module_d/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_d/mod.ts".to_string(), - content: concat!( - "import './other.ts';", - "import 'npm:package-a@module_d';", - ) - .to_string(), - maybe_headers: None, - }, - ), - ( - "https://deno.land/x/module_d/other.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_d/other.ts".to_string(), - content: "import 'npm:package-c@module_d'".to_string(), - maybe_headers: None, - }, - ), - ( - "https://deno.land/x/module_a/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_a/mod.ts".to_string(), - content: concat!( - "import 'npm:package-a@module_a';", - "import 'npm:package-b@module_a';", - "import '../module_c/sub/sub/mod.ts';", - "import '../module_b/mod.ts';", - ) - .to_string(), - maybe_headers: None, - }, - ), - ( - "https://deno.land/x/module_b/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_b/mod.ts".to_string(), - content: "import 'npm:package-a@module_b'".to_string(), - maybe_headers: None, - }, - ), - ( - "https://deno.land/x/module_c/sub/sub/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_c/sub/sub/mod.ts" - .to_string(), - content: concat!( - "import 'npm:package-a@module_c';", - "import '../../mod.ts';", - ) - .to_string(), - maybe_headers: None, - }, - ), - ( - "https://deno.land/x/module_c/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_c/mod.ts".to_string(), - content: concat!( - "import 'npm:package-b@module_c';", - "import '../module_d/sub_folder/mod.ts';", - ) - .to_string(), - maybe_headers: None, - }, - ), - ( - "https://deno.land/x/module_d/sub_folder/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_d/sub_folder/mod.ts" - .to_string(), - content: "import 'npm:package-b@module_d';".to_string(), - maybe_headers: None, - }, - ), - ( - // ensure a module at a directory is treated as being at a directory - "https://deno.land/x/module_e/".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_e/" - .to_string(), - content: "import 'npm:package-a@module_e';".to_string(), - maybe_headers: Some(vec![( - "content-type".to_string(), - "application/javascript".to_string(), - )]), - }, - ), - // redirect module - ( - "https://deno.land/x/module_redirect/mod.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_redirect@0.0.1/mod.ts".to_string(), - content: concat!( - "import 'npm:package-a@module_redirect';", - // try another redirect here - "import 'https://deno.land/x/module_redirect/other.ts';", - ).to_string(), - maybe_headers: None, - } - ), - ( - "https://deno.land/x/module_redirect/other.ts".to_string(), - deno_graph::source::Source::Module { - specifier: "https://deno.land/x/module_redirect@0.0.1/other.ts".to_string(), - content: "import 'npm:package-b@module_redirect';".to_string(), - maybe_headers: None, - } - ), - ], - Vec::new(), - ); - let analyzer = deno_graph::CapturingModuleAnalyzer::default(); - let mut graph = deno_graph::ModuleGraph::default(); - graph - .build( - vec![ - ModuleSpecifier::parse("file:///dev/local_module_a/mod.ts").unwrap(), - // test redirect at root - ModuleSpecifier::parse("https://deno.land/x/module_redirect/mod.ts") - .unwrap(), - ], - &mut loader, - deno_graph::BuildOptions { - module_analyzer: Some(&analyzer), - ..Default::default() - }, - ) - .await; - let reqs = resolve_graph_npm_info(&graph) - .package_reqs - .into_iter() - .map(|r| r.to_string()) - .collect::<Vec<_>>(); - - assert_eq!( - reqs, - vec![ - "package-a@local_module_a", - "package-b@local_module_a", - "package-a@module_redirect", - "package-b@module_redirect", - "package-b@local_module_b", - "package-data@local_module_b", - "package-a@module_a", - "package-b@module_a", - "package-a@module_d", - "package-b@module_d", - "package-c@module_d", - "package-a@module_e", - "package-a@module_b", - "package-a@module_c", - "package-b@module_c", - ] - ); - } -} diff --git a/cli/npm/resolvers/common.rs b/cli/npm/resolvers/common.rs index 2b02e7721..8c1ecd892 100644 --- a/cli/npm/resolvers/common.rs +++ b/cli/npm/resolvers/common.rs @@ -1,30 +1,28 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -use std::collections::HashSet; use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; +use async_trait::async_trait; use deno_ast::ModuleSpecifier; use deno_core::error::AnyError; use deno_core::futures; -use deno_core::futures::future::BoxFuture; use deno_core::url::Url; -use deno_graph::npm::NpmPackageReq; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolutionMode; -use crate::args::Lockfile; use crate::npm::cache::should_sync_download; -use crate::npm::resolution::NpmResolutionSnapshot; use crate::npm::NpmCache; use crate::npm::NpmPackageId; use crate::npm::NpmResolutionPackage; -pub trait InnerNpmPackageResolver: Send + Sync { +/// Part of the resolution that interacts with the file system. +#[async_trait] +pub trait NpmPackageFsResolver: Send + Sync { fn resolve_package_folder_from_deno_module( &self, - pkg_req: &NpmPackageReq, + id: &NpmPackageId, ) -> Result<PathBuf, AnyError>; fn resolve_package_folder_from_package( @@ -41,29 +39,13 @@ pub trait InnerNpmPackageResolver: Send + Sync { fn package_size(&self, package_id: &NpmPackageId) -> Result<u64, AnyError>; - fn has_packages(&self) -> bool; - - fn add_package_reqs( - &self, - packages: Vec<NpmPackageReq>, - ) -> BoxFuture<'static, Result<(), AnyError>>; - - fn set_package_reqs( - &self, - packages: HashSet<NpmPackageReq>, - ) -> BoxFuture<'static, Result<(), AnyError>>; - - fn cache_packages(&self) -> BoxFuture<'static, Result<(), AnyError>>; + async fn cache_packages(&self) -> Result<(), AnyError>; fn ensure_read_permission( &self, permissions: &mut dyn NodePermissions, path: &Path, ) -> Result<(), AnyError>; - - fn snapshot(&self) -> NpmResolutionSnapshot; - - fn lock(&self, lockfile: &mut Lockfile) -> Result<(), AnyError>; } /// Caches all the packages in parallel. @@ -86,11 +68,7 @@ pub async fn cache_packages( let registry_url = registry_url.clone(); let handle = tokio::task::spawn(async move { cache - .ensure_package( - (package.pkg_id.nv.name.as_str(), &package.pkg_id.nv.version), - &package.dist, - ®istry_url, - ) + .ensure_package(&package.pkg_id.nv, &package.dist, ®istry_url) .await }); if sync_download { diff --git a/cli/npm/resolvers/global.rs b/cli/npm/resolvers/global.rs index e7bdbb1b4..1d4d14ac8 100644 --- a/cli/npm/resolvers/global.rs +++ b/cli/npm/resolvers/global.rs @@ -2,51 +2,41 @@ //! Code for global npm cache resolution. -use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; -use std::sync::Arc; +use async_trait::async_trait; use deno_ast::ModuleSpecifier; use deno_core::error::AnyError; -use deno_core::futures::future::BoxFuture; -use deno_core::futures::FutureExt; use deno_core::url::Url; -use deno_graph::npm::NpmPackageReq; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolutionMode; -use crate::args::Lockfile; use crate::npm::cache::NpmPackageCacheFolderId; use crate::npm::resolution::NpmResolution; -use crate::npm::resolution::NpmResolutionSnapshot; use crate::npm::resolvers::common::cache_packages; use crate::npm::NpmCache; use crate::npm::NpmPackageId; -use crate::npm::NpmRegistryApi; use crate::npm::NpmResolutionPackage; use super::common::ensure_registry_read_permission; use super::common::types_package_name; -use super::common::InnerNpmPackageResolver; +use super::common::NpmPackageFsResolver; /// Resolves packages from the global npm cache. #[derive(Debug, Clone)] pub struct GlobalNpmPackageResolver { cache: NpmCache, - resolution: Arc<NpmResolution>, + resolution: NpmResolution, registry_url: Url, } impl GlobalNpmPackageResolver { pub fn new( cache: NpmCache, - api: NpmRegistryApi, - initial_snapshot: Option<NpmResolutionSnapshot>, + registry_url: Url, + resolution: NpmResolution, ) -> Self { - let registry_url = api.base_url().to_owned(); - let resolution = Arc::new(NpmResolution::new(api, initial_snapshot)); - Self { cache, resolution, @@ -76,13 +66,13 @@ impl GlobalNpmPackageResolver { } } -impl InnerNpmPackageResolver for GlobalNpmPackageResolver { +#[async_trait] +impl NpmPackageFsResolver for GlobalNpmPackageResolver { fn resolve_package_folder_from_deno_module( &self, - pkg_req: &NpmPackageReq, + id: &NpmPackageId, ) -> Result<PathBuf, AnyError> { - let pkg = self.resolution.resolve_package_from_deno_module(pkg_req)?; - Ok(self.package_folder(&pkg.pkg_id)) + Ok(self.package_folder(id)) } fn resolve_package_folder_from_package( @@ -125,34 +115,13 @@ impl InnerNpmPackageResolver for GlobalNpmPackageResolver { ) } - fn package_size(&self, package_id: &NpmPackageId) -> Result<u64, AnyError> { - let package_folder = self.package_folder(package_id); + fn package_size(&self, id: &NpmPackageId) -> Result<u64, AnyError> { + let package_folder = self.package_folder(id); Ok(crate::util::fs::dir_size(&package_folder)?) } - fn has_packages(&self) -> bool { - self.resolution.has_packages() - } - - fn add_package_reqs( - &self, - packages: Vec<NpmPackageReq>, - ) -> BoxFuture<'static, Result<(), AnyError>> { - let resolver = self.clone(); - async move { resolver.resolution.add_package_reqs(packages).await }.boxed() - } - - fn set_package_reqs( - &self, - packages: HashSet<NpmPackageReq>, - ) -> BoxFuture<'static, Result<(), AnyError>> { - let resolver = self.clone(); - async move { resolver.resolution.set_package_reqs(packages).await }.boxed() - } - - fn cache_packages(&self) -> BoxFuture<'static, Result<(), AnyError>> { - let resolver = self.clone(); - async move { cache_packages_in_resolver(&resolver).await }.boxed() + async fn cache_packages(&self) -> Result<(), AnyError> { + cache_packages_in_resolver(self).await } fn ensure_read_permission( @@ -163,14 +132,6 @@ impl InnerNpmPackageResolver for GlobalNpmPackageResolver { let registry_path = self.cache.registry_folder(&self.registry_url); ensure_registry_read_permission(permissions, ®istry_path, path) } - - fn snapshot(&self) -> NpmResolutionSnapshot { - self.resolution.snapshot() - } - - fn lock(&self, lockfile: &mut Lockfile) -> Result<(), AnyError> { - self.resolution.lock(lockfile) - } } async fn cache_packages_in_resolver( diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index aa6233d61..ba395d1b6 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -8,24 +8,20 @@ use std::collections::VecDeque; use std::fs; use std::path::Path; use std::path::PathBuf; -use std::sync::Arc; use crate::util::fs::symlink_dir; +use async_trait::async_trait; use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; -use deno_core::futures::future::BoxFuture; -use deno_core::futures::FutureExt; use deno_core::url::Url; -use deno_graph::npm::NpmPackageReq; use deno_runtime::deno_core::futures; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::deno_node::PackageJson; use tokio::task::JoinHandle; -use crate::args::Lockfile; use crate::npm::cache::mixed_case_package_name_encode; use crate::npm::cache::should_sync_download; use crate::npm::cache::NpmPackageCacheFolderId; @@ -33,21 +29,19 @@ use crate::npm::resolution::NpmResolution; use crate::npm::resolution::NpmResolutionSnapshot; use crate::npm::NpmCache; use crate::npm::NpmPackageId; -use crate::npm::NpmRegistryApi; -use crate::npm::NpmResolutionPackage; use crate::util::fs::copy_dir_recursive; use crate::util::fs::hard_link_dir_recursive; use super::common::ensure_registry_read_permission; use super::common::types_package_name; -use super::common::InnerNpmPackageResolver; +use super::common::NpmPackageFsResolver; /// Resolver that creates a local node_modules directory /// and resolves packages from it. #[derive(Debug, Clone)] pub struct LocalNpmPackageResolver { cache: NpmCache, - resolution: Arc<NpmResolution>, + resolution: NpmResolution, registry_url: Url, root_node_modules_path: PathBuf, root_node_modules_specifier: ModuleSpecifier, @@ -56,13 +50,10 @@ pub struct LocalNpmPackageResolver { impl LocalNpmPackageResolver { pub fn new( cache: NpmCache, - api: NpmRegistryApi, + registry_url: Url, node_modules_folder: PathBuf, - initial_snapshot: Option<NpmResolutionSnapshot>, + resolution: NpmResolution, ) -> Self { - let registry_url = api.base_url().to_owned(); - let resolution = Arc::new(NpmResolution::new(api, initial_snapshot)); - Self { cache, resolution, @@ -112,41 +103,34 @@ impl LocalNpmPackageResolver { fn get_package_id_folder( &self, - package_id: &NpmPackageId, + id: &NpmPackageId, ) -> Result<PathBuf, AnyError> { - match self.resolution.resolve_package_from_id(package_id) { - Some(package) => Ok(self.get_package_id_folder_from_package(&package)), + match self.resolution.resolve_package_cache_folder_id_from_id(id) { + // package is stored at: + // node_modules/.deno/<package_cache_folder_id_folder_name>/node_modules/<package_name> + Some(cache_folder_id) => Ok( + self + .root_node_modules_path + .join(".deno") + .join(get_package_folder_id_folder_name(&cache_folder_id)) + .join("node_modules") + .join(&cache_folder_id.nv.name), + ), None => bail!( "Could not find package information for '{}'", - package_id.as_serialized() + id.as_serialized() ), } } - - fn get_package_id_folder_from_package( - &self, - package: &NpmResolutionPackage, - ) -> PathBuf { - // package is stored at: - // node_modules/.deno/<package_cache_folder_id_folder_name>/node_modules/<package_name> - self - .root_node_modules_path - .join(".deno") - .join(get_package_folder_id_folder_name( - &package.get_package_cache_folder_id(), - )) - .join("node_modules") - .join(&package.pkg_id.nv.name) - } } -impl InnerNpmPackageResolver for LocalNpmPackageResolver { +#[async_trait] +impl NpmPackageFsResolver for LocalNpmPackageResolver { fn resolve_package_folder_from_deno_module( &self, - pkg_req: &NpmPackageReq, + node_id: &NpmPackageId, ) -> Result<PathBuf, AnyError> { - let package = self.resolution.resolve_package_from_deno_module(pkg_req)?; - Ok(self.get_package_id_folder_from_package(&package)) + self.get_package_id_folder(node_id) } fn resolve_package_folder_from_package( @@ -203,47 +187,15 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { Ok(package_root_path) } - fn package_size(&self, package_id: &NpmPackageId) -> Result<u64, AnyError> { - let package_folder_path = self.get_package_id_folder(package_id)?; + fn package_size(&self, id: &NpmPackageId) -> Result<u64, AnyError> { + let package_folder_path = self.get_package_id_folder(id)?; Ok(crate::util::fs::dir_size(&package_folder_path)?) } - fn has_packages(&self) -> bool { - self.resolution.has_packages() - } - - fn add_package_reqs( - &self, - packages: Vec<NpmPackageReq>, - ) -> BoxFuture<'static, Result<(), AnyError>> { - let resolver = self.clone(); - async move { - resolver.resolution.add_package_reqs(packages).await?; - Ok(()) - } - .boxed() - } - - fn set_package_reqs( - &self, - packages: HashSet<NpmPackageReq>, - ) -> BoxFuture<'static, Result<(), AnyError>> { - let resolver = self.clone(); - async move { - resolver.resolution.set_package_reqs(packages).await?; - Ok(()) - } - .boxed() - } - - fn cache_packages(&self) -> BoxFuture<'static, Result<(), AnyError>> { - let resolver = self.clone(); - async move { - sync_resolver_with_fs(&resolver).await?; - Ok(()) - } - .boxed() + async fn cache_packages(&self) -> Result<(), AnyError> { + sync_resolver_with_fs(self).await?; + Ok(()) } fn ensure_read_permission( @@ -257,14 +209,6 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { path, ) } - - fn snapshot(&self) -> NpmResolutionSnapshot { - self.resolution.snapshot() - } - - fn lock(&self, lockfile: &mut Lockfile) -> Result<(), AnyError> { - self.resolution.lock(lockfile) - } } async fn sync_resolver_with_fs( @@ -321,11 +265,7 @@ async fn sync_resolution_with_fs( let package = package.clone(); let handle = tokio::task::spawn(async move { cache - .ensure_package( - (&package.pkg_id.nv.name, &package.pkg_id.nv.version), - &package.dist, - ®istry_url, - ) + .ensure_package(&package.pkg_id.nv, &package.dist, ®istry_url) .await?; let sub_node_modules = folder_path.join("node_modules"); let package_path = @@ -333,8 +273,7 @@ async fn sync_resolution_with_fs( fs::create_dir_all(&package_path) .with_context(|| format!("Creating '{}'", folder_path.display()))?; let cache_folder = cache.package_folder_for_name_and_version( - &package.pkg_id.nv.name, - &package.pkg_id.nv.version, + &package.pkg_id.nv, ®istry_url, ); // for now copy, but in the future consider hard linking @@ -427,22 +366,22 @@ async fn sync_resolution_with_fs( .into_iter() .map(|id| (id, true)), ); - while let Some((package_id, is_top_level)) = pending_packages.pop_front() { - let root_folder_name = if found_names.insert(package_id.nv.name.clone()) { - package_id.nv.name.clone() + while let Some((id, is_top_level)) = pending_packages.pop_front() { + let root_folder_name = if found_names.insert(id.nv.name.clone()) { + id.nv.name.clone() } else if is_top_level { - format!("{}@{}", package_id.nv.name, package_id.nv.version) + id.nv.to_string() } else { continue; // skip, already handled }; - let package = snapshot.package_from_id(&package_id).unwrap(); + let package = snapshot.package_from_id(&id).unwrap(); let local_registry_package_path = join_package_name( &deno_local_registry_dir .join(get_package_folder_id_folder_name( &package.get_package_cache_folder_id(), )) .join("node_modules"), - &package_id.nv.name, + &id.nv.name, ); symlink_package_dir( diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 3ac373a54..2450638bf 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -7,10 +7,10 @@ mod local; use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; use deno_core::anyhow::Context; -use deno_core::error::custom_error; use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; use deno_core::serde_json; +use deno_graph::npm::NpmPackageNv; use deno_graph::npm::NpmPackageReq; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolutionMode; @@ -27,8 +27,9 @@ use std::sync::Arc; use crate::args::Lockfile; use crate::util::fs::canonicalize_path_maybe_not_exists; -use self::common::InnerNpmPackageResolver; +use self::common::NpmPackageFsResolver; use self::local::LocalNpmPackageResolver; +use super::resolution::NpmResolution; use super::NpmCache; use super::NpmPackageId; use super::NpmRegistryApi; @@ -43,10 +44,10 @@ pub struct NpmProcessState { #[derive(Clone)] pub struct NpmPackageResolver { - no_npm: bool, - inner: Arc<dyn InnerNpmPackageResolver>, + fs_resolver: Arc<dyn NpmPackageFsResolver>, local_node_modules_path: Option<PathBuf>, api: NpmRegistryApi, + resolution: NpmResolution, cache: NpmCache, maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, } @@ -54,22 +55,24 @@ pub struct NpmPackageResolver { impl std::fmt::Debug for NpmPackageResolver { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("NpmPackageResolver") - .field("no_npm", &self.no_npm) - .field("inner", &"<omitted>") + .field("fs_resolver", &"<omitted>") .field("local_node_modules_path", &self.local_node_modules_path) + .field("api", &"<omitted>") + .field("resolution", &"<omitted>") + .field("cache", &"<omitted>") + .field("maybe_lockfile", &"<omitted>") .finish() } } impl NpmPackageResolver { pub fn new(cache: NpmCache, api: NpmRegistryApi) -> Self { - Self::new_inner(cache, api, false, None, None, None) + Self::new_inner(cache, api, None, None, None) } pub async fn new_with_maybe_lockfile( cache: NpmCache, api: NpmRegistryApi, - no_npm: bool, local_node_modules_path: Option<PathBuf>, initial_snapshot: Option<NpmResolutionSnapshot>, maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, @@ -96,7 +99,6 @@ impl NpmPackageResolver { Ok(Self::new_inner( cache, api, - no_npm, local_node_modules_path, initial_snapshot, maybe_lockfile, @@ -106,47 +108,67 @@ impl NpmPackageResolver { fn new_inner( cache: NpmCache, api: NpmRegistryApi, - no_npm: bool, local_node_modules_path: Option<PathBuf>, maybe_snapshot: Option<NpmResolutionSnapshot>, maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, ) -> Self { - let inner: Arc<dyn InnerNpmPackageResolver> = match &local_node_modules_path - { - Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new( - cache.clone(), - api.clone(), - node_modules_folder.clone(), - maybe_snapshot, - )), - None => Arc::new(GlobalNpmPackageResolver::new( - cache.clone(), - api.clone(), - maybe_snapshot, - )), - }; + let registry_url = api.base_url().to_owned(); + let resolution = + NpmResolution::new(api.clone(), maybe_snapshot, maybe_lockfile.clone()); + let fs_resolver: Arc<dyn NpmPackageFsResolver> = + match &local_node_modules_path { + Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new( + cache.clone(), + registry_url, + node_modules_folder.clone(), + resolution.clone(), + )), + None => Arc::new(GlobalNpmPackageResolver::new( + cache.clone(), + registry_url, + resolution.clone(), + )), + }; Self { - no_npm, - inner, + fs_resolver, local_node_modules_path, api, + resolution, cache, maybe_lockfile, } } + pub fn api(&self) -> &NpmRegistryApi { + &self.api + } + + pub fn resolution(&self) -> &NpmResolution { + &self.resolution + } + /// Resolves an npm package folder path from a Deno module. pub fn resolve_package_folder_from_deno_module( &self, - pkg_req: &NpmPackageReq, + package_id: &NpmPackageNv, + ) -> Result<PathBuf, AnyError> { + let node_id = self + .resolution + .resolve_pkg_id_from_deno_module(package_id)?; + self.resolve_pkg_folder_from_deno_module_at_node_id(&node_id) + } + + fn resolve_pkg_folder_from_deno_module_at_node_id( + &self, + package_id: &NpmPackageId, ) -> Result<PathBuf, AnyError> { let path = self - .inner - .resolve_package_folder_from_deno_module(pkg_req)?; + .fs_resolver + .resolve_package_folder_from_deno_module(package_id)?; let path = canonicalize_path_maybe_not_exists(&path)?; log::debug!( "Resolved package folder of {} to {}", - pkg_req, + package_id.as_serialized(), path.display() ); Ok(path) @@ -160,7 +182,7 @@ impl NpmPackageResolver { mode: NodeResolutionMode, ) -> Result<PathBuf, AnyError> { let path = self - .inner + .fs_resolver .resolve_package_folder_from_package(name, referrer, mode)?; log::debug!("Resolved {} from {} to {}", name, referrer, path.display()); Ok(path) @@ -174,7 +196,7 @@ impl NpmPackageResolver { specifier: &ModuleSpecifier, ) -> Result<PathBuf, AnyError> { let path = self - .inner + .fs_resolver .resolve_package_folder_from_specifier(specifier)?; log::debug!( "Resolved package folder of {} to {}", @@ -189,7 +211,7 @@ impl NpmPackageResolver { &self, package_id: &NpmPackageId, ) -> Result<u64, AnyError> { - self.inner.package_size(package_id) + self.fs_resolver.package_size(package_id) } /// Gets if the provided specifier is in an npm package. @@ -201,7 +223,7 @@ impl NpmPackageResolver { /// If the resolver has resolved any npm packages. pub fn has_packages(&self) -> bool { - self.inner.has_packages() + self.resolution.has_packages() } /// Adds package requirements to the resolver and ensures everything is setup. @@ -213,24 +235,8 @@ impl NpmPackageResolver { return Ok(()); } - if self.no_npm { - let fmt_reqs = packages - .iter() - .collect::<HashSet<_>>() // prevent duplicates - .iter() - .map(|p| format!("\"{p}\"")) - .collect::<Vec<_>>() - .join(", "); - return Err(custom_error( - "NoNpm", - format!( - "Following npm specifiers were requested: {fmt_reqs}; but --no-npm is specified." - ), - )); - } - - self.inner.add_package_reqs(packages).await?; - self.inner.cache_packages().await?; + self.resolution.add_package_reqs(packages).await?; + self.fs_resolver.cache_packages().await?; // If there's a lock file, update it with all discovered npm packages if let Some(lockfile_mutex) = &self.maybe_lockfile { @@ -248,13 +254,13 @@ impl NpmPackageResolver { &self, packages: HashSet<NpmPackageReq>, ) -> Result<(), AnyError> { - self.inner.set_package_reqs(packages).await + 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.inner.snapshot(), + snapshot: self.snapshot(), local_node_modules_path: self .local_node_modules_path .as_ref() @@ -268,7 +274,6 @@ impl NpmPackageResolver { Self::new_inner( self.cache.clone(), self.api.clone(), - self.no_npm, self.local_node_modules_path.clone(), Some(self.snapshot()), None, @@ -276,11 +281,11 @@ impl NpmPackageResolver { } pub fn snapshot(&self) -> NpmResolutionSnapshot { - self.inner.snapshot() + self.resolution.snapshot() } pub fn lock(&self, lockfile: &mut Lockfile) -> Result<(), AnyError> { - self.inner.lock(lockfile) + self.resolution.lock(lockfile) } pub async fn inject_synthetic_types_node_package( @@ -288,11 +293,17 @@ impl NpmPackageResolver { ) -> Result<(), AnyError> { // add and ensure this isn't added to the lockfile self - .inner + .resolution .add_package_reqs(vec![NpmPackageReq::from_str("@types/node").unwrap()]) .await?; - self.inner.cache_packages().await?; + self.fs_resolver.cache_packages().await?; + + Ok(()) + } + pub async fn resolve_pending(&self) -> Result<(), AnyError> { + self.resolution.resolve_pending().await?; + self.fs_resolver.cache_packages().await?; Ok(()) } } @@ -332,7 +343,7 @@ impl RequireNpmResolver for NpmPackageResolver { permissions: &mut dyn NodePermissions, path: &Path, ) -> Result<(), AnyError> { - self.inner.ensure_read_permission(permissions, path) + self.fs_resolver.ensure_read_permission(permissions, path) } } diff --git a/cli/npm/tarball.rs b/cli/npm/tarball.rs index 302c6308a..1f804a9aa 100644 --- a/cli/npm/tarball.rs +++ b/cli/npm/tarball.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; use deno_core::anyhow::bail; use deno_core::error::AnyError; -use deno_graph::semver::Version; +use deno_graph::npm::NpmPackageNv; use flate2::read::GzDecoder; use tar::Archive; use tar::EntryType; @@ -16,7 +16,7 @@ use super::cache::with_folder_sync_lock; use super::registry::NpmPackageVersionDistInfo; pub fn verify_and_extract_tarball( - package: (&str, &Version), + package: &NpmPackageNv, data: &[u8], dist_info: &NpmPackageVersionDistInfo, output_folder: &Path, @@ -29,7 +29,7 @@ pub fn verify_and_extract_tarball( } fn verify_tarball_integrity( - package: (&str, &Version), + package: &NpmPackageNv, data: &[u8], npm_integrity: &str, ) -> Result<(), AnyError> { @@ -40,18 +40,16 @@ fn verify_tarball_integrity( "sha512" => &ring::digest::SHA512, "sha1" => &ring::digest::SHA1_FOR_LEGACY_USE_ONLY, hash_kind => bail!( - "Not implemented hash function for {}@{}: {}", - package.0, - package.1, + "Not implemented hash function for {}: {}", + package, hash_kind ), }; (algo, checksum.to_lowercase()) } None => bail!( - "Not implemented integrity kind for {}@{}: {}", - package.0, - package.1, + "Not implemented integrity kind for {}: {}", + package, npm_integrity ), }; @@ -62,9 +60,8 @@ fn verify_tarball_integrity( let tarball_checksum = base64::encode(digest.as_ref()).to_lowercase(); if tarball_checksum != expected_checksum { bail!( - "Tarball checksum did not match what was provided by npm registry for {}@{}.\n\nExpected: {}\nActual: {}", - package.0, - package.1, + "Tarball checksum did not match what was provided by npm registry for {}.\n\nExpected: {}\nActual: {}", + package, expected_checksum, tarball_checksum, ) @@ -119,29 +116,32 @@ fn extract_tarball(data: &[u8], output_folder: &Path) -> Result<(), AnyError> { #[cfg(test)] mod test { + use deno_graph::semver::Version; + use super::*; #[test] pub fn test_verify_tarball() { - let package_name = "package".to_string(); - let package_version = Version::parse_from_npm("1.0.0").unwrap(); - let package = (package_name.as_str(), &package_version); + let package = NpmPackageNv { + name: "package".to_string(), + version: Version::parse_from_npm("1.0.0").unwrap(), + }; let actual_checksum = "z4phnx7vul3xvchq1m2ab9yg5aulvxxcg/spidns6c5h0ne8xyxysp+dgnkhfuwvy7kxvudbeoglodj6+sfapg=="; assert_eq!( - verify_tarball_integrity(package, &Vec::new(), "test") + verify_tarball_integrity(&package, &Vec::new(), "test") .unwrap_err() .to_string(), "Not implemented integrity kind for package@1.0.0: test", ); assert_eq!( - verify_tarball_integrity(package, &Vec::new(), "notimplemented-test") + verify_tarball_integrity(&package, &Vec::new(), "notimplemented-test") .unwrap_err() .to_string(), "Not implemented hash function for package@1.0.0: notimplemented", ); assert_eq!( - verify_tarball_integrity(package, &Vec::new(), "sha1-test") + verify_tarball_integrity(&package, &Vec::new(), "sha1-test") .unwrap_err() .to_string(), concat!( @@ -150,13 +150,13 @@ mod test { ), ); assert_eq!( - verify_tarball_integrity(package, &Vec::new(), "sha512-test") + verify_tarball_integrity(&package, &Vec::new(), "sha512-test") .unwrap_err() .to_string(), format!("Tarball checksum did not match what was provided by npm registry for package@1.0.0.\n\nExpected: test\nActual: {actual_checksum}"), ); assert!(verify_tarball_integrity( - package, + &package, &Vec::new(), &format!("sha512-{actual_checksum}") ) diff --git a/cli/proc_state.rs b/cli/proc_state.rs index edd59427c..86af11ec3 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -17,12 +17,12 @@ use crate::cache::ParsedSourceCache; use crate::cache::TypeCheckCache; use crate::emit::emit_parsed_source; use crate::file_fetcher::FileFetcher; +use crate::graph_util::build_graph_with_npm_resolution; use crate::graph_util::graph_lock_or_exit; use crate::graph_util::graph_valid_with_cli_options; use crate::http_util::HttpClient; use crate::node; use crate::node::NodeResolution; -use crate::npm::resolve_graph_npm_info; use crate::npm::NpmCache; use crate::npm::NpmPackageResolver; use crate::npm::NpmRegistryApi; @@ -43,12 +43,11 @@ use deno_core::resolve_url_or_path; use deno_core::CompiledWasmModuleStore; use deno_core::ModuleSpecifier; use deno_core::SharedArrayBufferStore; -use deno_graph::npm::NpmPackageReq; use deno_graph::npm::NpmPackageReqReference; use deno_graph::source::Loader; use deno_graph::source::Resolver; +use deno_graph::Module; use deno_graph::ModuleGraph; -use deno_graph::ModuleKind; use deno_graph::Resolution; use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel; use deno_runtime::deno_node::NodeResolutionMode; @@ -209,6 +208,29 @@ impl ProcState { let lockfile = cli_options.maybe_lock_file(); + let registry_url = NpmRegistryApi::default_url().to_owned(); + let npm_cache = NpmCache::from_deno_dir( + &dir, + cli_options.cache_setting(), + http_client.clone(), + progress_bar.clone(), + ); + let api = NpmRegistryApi::new( + registry_url, + npm_cache.clone(), + http_client.clone(), + progress_bar.clone(), + ); + let npm_resolver = NpmPackageResolver::new_with_maybe_lockfile( + npm_cache.clone(), + api, + cli_options + .resolve_local_node_modules_folder() + .with_context(|| "Resolving local node_modules folder.")?, + cli_options.get_npm_resolution_snapshot(), + lockfile.as_ref().cloned(), + ) + .await?; let maybe_import_map = cli_options .resolve_import_map(&file_fetcher) .await? @@ -217,16 +239,18 @@ impl ProcState { cli_options.resolve_inspector_server().map(Arc::new); let maybe_package_json_deps = cli_options.maybe_package_json_deps()?; - let package_json_reqs = if let Some(deps) = &maybe_package_json_deps { + if let Some(deps) = &maybe_package_json_deps { + // resolve the package.json npm requirements ahead of time let mut package_reqs = deps.values().cloned().collect::<Vec<_>>(); package_reqs.sort(); // deterministic resolution - package_reqs - } else { - Vec::new() - }; + npm_resolver.add_package_reqs(package_reqs).await?; + } let resolver = Arc::new(CliGraphResolver::new( cli_options.to_maybe_jsx_import_source_config(), maybe_import_map.clone(), + cli_options.no_npm(), + npm_resolver.api().clone(), + npm_resolver.resolution().clone(), maybe_package_json_deps, )); @@ -244,31 +268,12 @@ impl ProcState { let emit_cache = EmitCache::new(dir.gen_cache.clone()); let parsed_source_cache = ParsedSourceCache::new(Some(dir.dep_analysis_db_file_path())); - let registry_url = NpmRegistryApi::default_url(); let npm_cache = NpmCache::from_deno_dir( &dir, cli_options.cache_setting(), http_client.clone(), progress_bar.clone(), ); - let api = NpmRegistryApi::new( - registry_url.clone(), - npm_cache.clone(), - http_client.clone(), - progress_bar.clone(), - ); - let npm_resolver = NpmPackageResolver::new_with_maybe_lockfile( - npm_cache.clone(), - api, - cli_options.no_npm(), - cli_options - .resolve_local_node_modules_folder() - .with_context(|| "Resolving local node_modules folder.")?, - cli_options.get_npm_resolution_snapshot(), - lockfile.as_ref().cloned(), - ) - .await?; - npm_resolver.add_package_reqs(package_json_reqs).await?; let node_analysis_cache = NodeAnalysisCache::new(Some(dir.node_analysis_db_file_path())); @@ -326,7 +331,8 @@ impl ProcState { dynamic_permissions, ); let maybe_imports = self.options.to_maybe_imports()?; - let resolver = self.resolver.as_graph_resolver(); + let graph_resolver = self.resolver.as_graph_resolver(); + let graph_npm_resolver = self.resolver.as_graph_npm_resolver(); let maybe_file_watcher_reporter: Option<&dyn deno_graph::source::Reporter> = if let Some(reporter) = &self.maybe_file_watcher_reporter { Some(reporter) @@ -344,40 +350,35 @@ impl ProcState { let reload_exclusions: HashSet<ModuleSpecifier> = graph.specifiers().map(|(s, _)| s.clone()).collect(); - graph - .build( - roots.clone(), - &mut cache, - deno_graph::BuildOptions { - is_dynamic, - imports: maybe_imports, - resolver: Some(resolver), - module_analyzer: Some(&*analyzer), - reporter: maybe_file_watcher_reporter, - }, - ) - .await; + build_graph_with_npm_resolution( + &mut graph, + &self.npm_resolver, + roots.clone(), + &mut cache, + deno_graph::BuildOptions { + is_dynamic, + imports: maybe_imports, + resolver: Some(graph_resolver), + npm_resolver: Some(graph_npm_resolver), + module_analyzer: Some(&*analyzer), + reporter: maybe_file_watcher_reporter, + }, + ) + .await?; // If there is a lockfile, validate the integrity of all the modules. if let Some(lockfile) = &self.lockfile { graph_lock_or_exit(&graph, &mut lockfile.lock()); } - let (npm_package_reqs, has_node_builtin_specifier) = { + let graph = { graph_valid_with_cli_options(&graph, &roots, &self.options)?; let mut graph_data = self.graph_data.write(); graph_data.update_graph(Arc::new(graph)); - ( - graph_data.npm_packages.clone(), - graph_data.has_node_builtin_specifier, - ) + graph_data.graph.clone() }; - if !npm_package_reqs.is_empty() { - self.npm_resolver.add_package_reqs(npm_package_reqs).await?; - } - - if has_node_builtin_specifier + if graph.has_node_specifier && self.options.type_check_mode() != TypeCheckMode::None { self @@ -394,12 +395,9 @@ impl ProcState { { log::debug!("Type checking."); let maybe_config_specifier = self.options.maybe_config_file_specifier(); - let (graph, has_node_builtin_specifier) = { + let graph = { let graph_data = self.graph_data.read(); - ( - Arc::new(graph_data.graph.segment(&roots)), - graph_data.has_node_builtin_specifier, - ) + Arc::new(graph_data.graph.segment(&roots)) }; let options = check::CheckOptions { type_check_mode: self.options.type_check_mode(), @@ -412,7 +410,6 @@ impl ProcState { log_checks: true, reload: self.options.reload_flag() && !roots.iter().all(|r| reload_exclusions.contains(r)), - has_node_builtin_specifier, }; let check_cache = TypeCheckCache::new(&self.dir.type_checking_cache_db_file_path()); @@ -501,7 +498,7 @@ impl ProcState { let graph_data = self.graph_data.read(); let graph = &graph_data.graph; let maybe_resolved = match graph.get(&referrer) { - Some(module) => module + Some(Module::Esm(module)) => module .dependencies .get(specifier) .map(|d| (&module.specifier, &d.maybe_code)), @@ -512,33 +509,36 @@ impl ProcState { Some((found_referrer, Resolution::Ok(resolved))) => { let specifier = &resolved.specifier; - if specifier.scheme() == "node" { - return node::resolve_builtin_node_module(specifier.path()); - } - - if let Ok(reference) = - NpmPackageReqReference::from_specifier(specifier) - { - if !self.options.unstable() - && matches!(found_referrer.scheme(), "http" | "https") - { - return Err(custom_error( + return match graph.get(specifier) { + Some(Module::Npm(module)) => { + if !self.options.unstable() + && matches!(found_referrer.scheme(), "http" | "https") + { + return Err(custom_error( "NotSupported", format!("importing npm specifiers in remote modules requires the --unstable flag (referrer: {found_referrer})"), )); + } + + self + .handle_node_resolve_result(node::node_resolve_npm_reference( + &module.nv_reference, + NodeResolutionMode::Execution, + &self.npm_resolver, + permissions, + )) + .with_context(|| { + format!("Could not resolve '{}'.", module.nv_reference) + }) } - - return self - .handle_node_resolve_result(node::node_resolve_npm_reference( - &reference, - NodeResolutionMode::Execution, - &self.npm_resolver, - permissions, - )) - .with_context(|| format!("Could not resolve '{reference}'.")); - } else { - return Ok(specifier.clone()); - } + Some(Module::Node(module)) => { + node::resolve_builtin_node_module(&module.module_name) + } + Some(Module::Esm(module)) => Ok(module.specifier.clone()), + Some(Module::External(module)) => Ok(module.specifier.clone()), + Some(Module::Json(module)) => Ok(module.specifier.clone()), + None => Ok(specifier.clone()), + }; } Some((_, Resolution::Err(err))) => { return Err(custom_error( @@ -579,6 +579,10 @@ impl ProcState { if let Ok(reference) = NpmPackageReqReference::from_specifier(&specifier) { + let reference = self + .npm_resolver + .resolution() + .pkg_req_ref_to_nv_ref(reference)?; return self .handle_node_resolve_result(node::node_resolve_npm_reference( &reference, @@ -597,8 +601,8 @@ impl ProcState { pub fn cache_module_emits(&self) -> Result<(), AnyError> { let graph = self.graph(); for module in graph.modules() { - let is_emittable = module.kind != ModuleKind::External - && matches!( + if let Module::Esm(module) = module { + let is_emittable = matches!( module.media_type, MediaType::TypeScript | MediaType::Mts @@ -606,14 +610,13 @@ impl ProcState { | MediaType::Jsx | MediaType::Tsx ); - if is_emittable { - if let Some(code) = &module.maybe_source { + if is_emittable { emit_parsed_source( &self.emit_cache, &self.parsed_source_cache, &module.specifier, module.media_type, - code, + &module.source, &self.emit_options, self.emit_options_hash, )?; @@ -651,36 +654,34 @@ impl ProcState { let cli_resolver = CliGraphResolver::new( self.options.to_maybe_jsx_import_source_config(), self.maybe_import_map.clone(), + self.options.no_npm(), + self.npm_resolver.api().clone(), + self.npm_resolver.resolution().clone(), // TODO(bartlomieju): this should use dependencies from `package.json`? None, ); let graph_resolver = cli_resolver.as_graph_resolver(); + let graph_npm_resolver = cli_resolver.as_graph_npm_resolver(); let analyzer = self.parsed_source_cache.as_analyzer(); let mut graph = ModuleGraph::default(); - graph - .build( - roots, - loader, - deno_graph::BuildOptions { - is_dynamic: false, - imports: maybe_imports, - resolver: Some(graph_resolver), - module_analyzer: Some(&*analyzer), - reporter: None, - }, - ) - .await; + build_graph_with_npm_resolution( + &mut graph, + &self.npm_resolver, + roots, + loader, + deno_graph::BuildOptions { + is_dynamic: false, + imports: maybe_imports, + resolver: Some(graph_resolver), + npm_resolver: Some(graph_npm_resolver), + module_analyzer: Some(&*analyzer), + reporter: None, + }, + ) + .await?; - // add the found npm package requirements to the npm resolver and cache them - let graph_npm_info = resolve_graph_npm_info(&graph); - if !graph_npm_info.package_reqs.is_empty() { - self - .npm_resolver - .add_package_reqs(graph_npm_info.package_reqs) - .await?; - } - if graph_npm_info.has_node_builtin_specifier + if graph.has_node_specifier && self.options.type_check_mode() != TypeCheckMode::None { self @@ -697,7 +698,7 @@ impl ProcState { } pub fn has_node_builtin_specifier(&self) -> bool { - self.graph_data.read().has_node_builtin_specifier + self.graph_data.read().graph.has_node_specifier } } @@ -728,44 +729,12 @@ impl deno_graph::source::Reporter for FileWatcherReporter { #[derive(Debug, Default)] struct GraphData { graph: Arc<ModuleGraph>, - /// The npm package requirements from all the encountered graphs - /// in the order that they should be resolved. - npm_packages: Vec<NpmPackageReq>, - /// If the graph had a "node:" specifier. - has_node_builtin_specifier: bool, checked_libs: HashMap<TsTypeLib, HashSet<ModuleSpecifier>>, } impl GraphData { /// Store data from `graph` into `self`. pub fn update_graph(&mut self, graph: Arc<ModuleGraph>) { - let mut has_npm_specifier_in_graph = false; - - for (specifier, _) in graph.specifiers() { - match specifier.scheme() { - "node" => { - // We don't ever set this back to false because once it's - // on then it's on globally. - self.has_node_builtin_specifier = true; - } - "npm" => { - if !has_npm_specifier_in_graph - && NpmPackageReqReference::from_specifier(specifier).is_ok() - { - has_npm_specifier_in_graph = true; - } - } - _ => {} - } - - if has_npm_specifier_in_graph && self.has_node_builtin_specifier { - break; // exit early - } - } - - if has_npm_specifier_in_graph { - self.npm_packages = resolve_graph_npm_info(&graph).package_reqs; - } self.graph = graph; } diff --git a/cli/resolver.rs b/cli/resolver.rs index aa58549a7..76e1715b5 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -1,19 +1,29 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use deno_core::anyhow::bail; use deno_core::error::AnyError; +use deno_core::futures::future; +use deno_core::futures::future::LocalBoxFuture; +use deno_core::futures::FutureExt; use deno_core::ModuleSpecifier; +use deno_graph::npm::NpmPackageNv; +use deno_graph::npm::NpmPackageReq; +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_runtime::deno_node::is_builtin_node_module; use import_map::ImportMap; use std::collections::HashMap; use std::sync::Arc; use crate::args::JsxImportSourceConfig; -use deno_graph::npm::NpmPackageReq; +use crate::npm::NpmRegistryApi; +use crate::npm::NpmResolution; /// A resolver that takes care of resolution, taking into account loaded /// import map, JSX settings. -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub struct CliGraphResolver { maybe_import_map: Option<Arc<ImportMap>>, // TODO(bartlomieju): actually use in `resolver`, once @@ -22,12 +32,39 @@ pub struct CliGraphResolver { maybe_package_json_deps: Option<HashMap<String, NpmPackageReq>>, maybe_default_jsx_import_source: Option<String>, maybe_jsx_import_source_module: Option<String>, + no_npm: bool, + npm_registry_api: NpmRegistryApi, + npm_resolution: NpmResolution, + sync_download_semaphore: Option<Arc<tokio::sync::Semaphore>>, +} + +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 = NpmRegistryApi::new_uninitialized(); + let npm_resolution = + NpmResolution::new(npm_registry_api.clone(), None, None); + Self { + maybe_import_map: Default::default(), + maybe_default_jsx_import_source: Default::default(), + maybe_jsx_import_source_module: Default::default(), + no_npm: false, + npm_registry_api, + npm_resolution, + maybe_package_json_deps: Default::default(), + sync_download_semaphore: Self::create_sync_download_semaphore(), + } + } } impl CliGraphResolver { pub fn new( maybe_jsx_import_source_config: Option<JsxImportSourceConfig>, maybe_import_map: Option<Arc<ImportMap>>, + no_npm: bool, + npm_registry_api: NpmRegistryApi, + npm_resolution: NpmResolution, maybe_package_json_deps: Option<HashMap<String, NpmPackageReq>>, ) -> Self { Self { @@ -37,13 +74,29 @@ impl CliGraphResolver { .and_then(|c| c.default_specifier.clone()), maybe_jsx_import_source_module: maybe_jsx_import_source_config .map(|c| c.module), + no_npm, + npm_registry_api, + npm_resolution, maybe_package_json_deps, + sync_download_semaphore: Self::create_sync_download_semaphore(), + } + } + + fn create_sync_download_semaphore() -> Option<Arc<tokio::sync::Semaphore>> { + if crate::npm::should_sync_download() { + Some(Arc::new(tokio::sync::Semaphore::new(1))) + } else { + None } } pub fn as_graph_resolver(&self) -> &dyn Resolver { self } + + pub fn as_graph_npm_resolver(&self) -> &dyn NpmResolver { + self + } } impl Resolver for CliGraphResolver { @@ -74,3 +127,59 @@ impl Resolver for CliGraphResolver { } } } + +impl NpmResolver for CliGraphResolver { + fn resolve_builtin_node_module( + &self, + specifier: &ModuleSpecifier, + ) -> Result<Option<String>, UnknownBuiltInNodeModuleError> { + if specifier.scheme() != "node" { + return Ok(None); + } + + let module_name = specifier.path().to_string(); + if is_builtin_node_module(&module_name) { + Ok(Some(module_name)) + } else { + Err(UnknownBuiltInNodeModuleError { module_name }) + } + } + + fn load_and_cache_npm_package_info( + &self, + package_name: &str, + ) -> LocalBoxFuture<'static, Result<(), String>> { + if self.no_npm { + // return it succeeded and error at the import site below + return 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(); + let mut maybe_sync_download_semaphore = + self.sync_download_semaphore.clone(); + async move { + let result = if let Some(semaphore) = maybe_sync_download_semaphore.take() + { + let _permit = semaphore.acquire().await.unwrap(); + api.package_info(&package_name).await + } else { + api.package_info(&package_name).await + }; + result.map(|_| ()).map_err(|err| format!("{err:#}")) + } + .boxed() + } + + fn resolve_npm( + &self, + package_req: &NpmPackageReq, + ) -> Result<NpmPackageNv, AnyError> { + if self.no_npm { + bail!("npm specifiers were requested; but --no-npm is specified") + } + self + .npm_resolution + .resolve_package_req_for_deno_graph(package_req) + } +} diff --git a/cli/standalone.rs b/cli/standalone.rs index 7da246fa9..f5483a9f8 100644 --- a/cli/standalone.rs +++ b/cli/standalone.rs @@ -240,6 +240,9 @@ pub async fn run( Some(Arc::new( parse_from_json(&base, &source).unwrap().import_map, )), + false, + ps.npm_resolver.api().clone(), + ps.npm_resolver.resolution().clone(), None, ) }, diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index 107bab613..3d21b57c3 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -581,7 +581,7 @@ fn no_npm_after_first_run() { let stdout = String::from_utf8_lossy(&output.stdout); assert_contains!( stderr, - "Following npm specifiers were requested: \"chalk@5\"; but --no-npm is specified." + "error: npm specifiers were requested; but --no-npm is specified\n at file:///" ); assert!(stdout.is_empty()); assert!(!output.status.success()); @@ -623,7 +623,7 @@ fn no_npm_after_first_run() { let stdout = String::from_utf8_lossy(&output.stdout); assert_contains!( stderr, - "Following npm specifiers were requested: \"chalk@5\"; but --no-npm is specified." + "error: npm specifiers were requested; but --no-npm is specified\n at file:///" ); assert!(stdout.is_empty()); assert!(!output.status.success()); @@ -820,7 +820,7 @@ fn ensure_registry_files_local() { itest!(compile_errors { args: "compile -A --quiet npm/cached_only/main.ts", - output_str: Some("error: npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: npm:chalk@5\n"), + output_str: Some("error: npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: npm:chalk@5.0.1\n"), exit_code: 1, envs: env_vars_for_npm_tests(), http_server: true, @@ -828,7 +828,7 @@ itest!(compile_errors { itest!(bundle_errors { args: "bundle --quiet npm/esm/main.js", - output_str: Some("error: npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: npm:chalk@5\n"), + output_str: Some("error: npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: npm:chalk@5.0.1\n"), exit_code: 1, envs: env_vars_for_npm_tests(), http_server: true, diff --git a/cli/tests/testdata/cert/cafile_info.ts.out b/cli/tests/testdata/cert/cafile_info.ts.out index ddece3019..279453f88 100644 --- a/cli/tests/testdata/cert/cafile_info.ts.out +++ b/cli/tests/testdata/cert/cafile_info.ts.out @@ -4,11 +4,11 @@ dependencies: 8 unique size: [WILDCARD] https://localhost:5545/cert/cafile_info.ts ([WILDCARD]) -├── https://localhost:5545/subdir/mt_application_ecmascript.j2.js ([WILDCARD]) -├── https://localhost:5545/subdir/mt_application_x_javascript.j4.js ([WILDCARD]) -├── https://localhost:5545/subdir/mt_application_x_typescript.t4.ts ([WILDCARD]) -├── https://localhost:5545/subdir/mt_text_ecmascript.j3.js ([WILDCARD]) -├── https://localhost:5545/subdir/mt_text_javascript.j1.js ([WILDCARD]) ├── https://localhost:5545/subdir/mt_text_typescript.t1.ts ([WILDCARD]) +├── https://localhost:5545/subdir/mt_video_vdn.t2.ts ([WILDCARD]) ├── https://localhost:5545/subdir/mt_video_mp2t.t3.ts ([WILDCARD]) -└── https://localhost:5545/subdir/mt_video_vdn.t2.ts ([WILDCARD]) +├── https://localhost:5545/subdir/mt_application_x_typescript.t4.ts ([WILDCARD]) +├── https://localhost:5545/subdir/mt_text_javascript.j1.js ([WILDCARD]) +├── https://localhost:5545/subdir/mt_application_ecmascript.j2.js ([WILDCARD]) +├── https://localhost:5545/subdir/mt_text_ecmascript.j3.js ([WILDCARD]) +└── https://localhost:5545/subdir/mt_application_x_javascript.j4.js ([WILDCARD]) diff --git a/cli/tests/testdata/info/049_info_flag_script_jsx.out b/cli/tests/testdata/info/049_info_flag_script_jsx.out index 244541696..f49fc2356 100644 --- a/cli/tests/testdata/info/049_info_flag_script_jsx.out +++ b/cli/tests/testdata/info/049_info_flag_script_jsx.out @@ -5,11 +5,11 @@ dependencies: 8 unique size: [WILDCARD] http://127.0.0.1:4545/run/048_media_types_jsx.ts ([WILDCARD]) -├── http://localhost:4545/subdir/mt_application_ecmascript_jsx.j2.jsx ([WILDCARD]) -├── http://localhost:4545/subdir/mt_application_x_javascript_jsx.j4.jsx ([WILDCARD]) -├── http://localhost:4545/subdir/mt_application_x_typescript_tsx.t4.tsx ([WILDCARD]) -├── http://localhost:4545/subdir/mt_text_ecmascript_jsx.j3.jsx ([WILDCARD]) -├── http://localhost:4545/subdir/mt_text_javascript_jsx.j1.jsx ([WILDCARD]) ├── http://localhost:4545/subdir/mt_text_typescript_tsx.t1.tsx ([WILDCARD]) +├── http://localhost:4545/subdir/mt_video_vdn_tsx.t2.tsx ([WILDCARD]) ├── http://localhost:4545/subdir/mt_video_mp2t_tsx.t3.tsx ([WILDCARD]) -└── http://localhost:4545/subdir/mt_video_vdn_tsx.t2.tsx ([WILDCARD]) +├── http://localhost:4545/subdir/mt_application_x_typescript_tsx.t4.tsx ([WILDCARD]) +├── http://localhost:4545/subdir/mt_text_javascript_jsx.j1.jsx ([WILDCARD]) +├── http://localhost:4545/subdir/mt_application_ecmascript_jsx.j2.jsx ([WILDCARD]) +├── http://localhost:4545/subdir/mt_text_ecmascript_jsx.j3.jsx ([WILDCARD]) +└── http://localhost:4545/subdir/mt_application_x_javascript_jsx.j4.jsx ([WILDCARD]) diff --git a/cli/tests/testdata/info/076_info_json_deps_order.out b/cli/tests/testdata/info/076_info_json_deps_order.out index 98b5d5d50..a1b15e00c 100644 --- a/cli/tests/testdata/info/076_info_json_deps_order.out +++ b/cli/tests/testdata/info/076_info_json_deps_order.out @@ -4,6 +4,7 @@ ], "modules": [ { + "kind": "esm", "dependencies": [ { "specifier": "./recursive_imports/A.ts", @@ -22,13 +23,13 @@ } } ], - "kind": "esm", "local": "[WILDCARD]076_info_json_deps_order.ts", [WILDCARD] "mediaType": "TypeScript", "specifier": "file://[WILDCARD]/076_info_json_deps_order.ts" }, { + "kind": "esm", "dependencies": [ { "specifier": "./B.ts", @@ -63,13 +64,13 @@ } } ], - "kind": "esm", "local": "[WILDCARD]A.ts", [WILDCARD] "mediaType": "TypeScript", "specifier": "file://[WILDCARD]/recursive_imports/A.ts" }, { + "kind": "esm", "dependencies": [ { "specifier": "./C.ts", @@ -104,13 +105,13 @@ } } ], - "kind": "esm", "local": "[WILDCARD]B.ts", [WILDCARD] "mediaType": "TypeScript", "specifier": "file://[WILDCARD]/recursive_imports/B.ts" }, { + "kind": "esm", "dependencies": [ { "specifier": "./A.ts", @@ -145,7 +146,6 @@ } } ], - "kind": "esm", "local": "[WILDCARD]C.ts", [WILDCARD] "mediaType": "TypeScript", diff --git a/cli/tests/testdata/info/json_output/main.out b/cli/tests/testdata/info/json_output/main.out index aaef028c0..5a89d5cab 100644 --- a/cli/tests/testdata/info/json_output/main.out +++ b/cli/tests/testdata/info/json_output/main.out @@ -4,6 +4,7 @@ ], "modules": [ { + "kind": "esm", "dependencies": [ { "specifier": "../../subdir/mod1.ts", @@ -22,13 +23,13 @@ } } ], - "kind": "esm", "local": "[WILDCARD]main.ts", [WILDCARD] "mediaType": "TypeScript", "specifier": "file://[WILDCARD]/json_output/main.ts" }, { + "kind": "esm", "dependencies": [ { "specifier": "./subdir2/mod2.ts", @@ -47,7 +48,6 @@ } } ], - "kind": "esm", "local": "[WILDCARD]mod1.ts", [WILDCARD] "mediaType": "TypeScript", @@ -61,6 +61,7 @@ "specifier": "file://[WILDCARD]/subdir/print_hello.ts" }, { + "kind": "esm", "dependencies": [ { "specifier": "../print_hello.ts", @@ -79,7 +80,6 @@ } } ], - "kind": "esm", "local": "[WILDCARD]mod2.ts", [WILDCARD] "mediaType": "TypeScript", diff --git a/cli/tests/testdata/info/multiple_imports.out b/cli/tests/testdata/info/multiple_imports.out index ea35e69c8..cb13318ca 100644 --- a/cli/tests/testdata/info/multiple_imports.out +++ b/cli/tests/testdata/info/multiple_imports.out @@ -5,11 +5,11 @@ dependencies: 8 unique size: [WILDCARD] http://127.0.0.1:4545/run/019_media_types.ts ([WILDCARD]) -├── http://localhost:4545/subdir/mt_application_ecmascript.j2.js ([WILDCARD]) -├── http://localhost:4545/subdir/mt_application_x_javascript.j4.js ([WILDCARD]) -├── http://localhost:4545/subdir/mt_application_x_typescript.t4.ts ([WILDCARD]) -├── http://localhost:4545/subdir/mt_text_ecmascript.j3.js ([WILDCARD]) -├── http://localhost:4545/subdir/mt_text_javascript.j1.js ([WILDCARD]) ├── http://localhost:4545/subdir/mt_text_typescript.t1.ts ([WILDCARD]) +├── http://localhost:4545/subdir/mt_video_vdn.t2.ts ([WILDCARD]) ├── http://localhost:4545/subdir/mt_video_mp2t.t3.ts ([WILDCARD]) -└── http://localhost:4545/subdir/mt_video_vdn.t2.ts ([WILDCARD]) +├── http://localhost:4545/subdir/mt_application_x_typescript.t4.ts ([WILDCARD]) +├── http://localhost:4545/subdir/mt_text_javascript.j1.js ([WILDCARD]) +├── http://localhost:4545/subdir/mt_application_ecmascript.j2.js ([WILDCARD]) +├── http://localhost:4545/subdir/mt_text_ecmascript.j3.js ([WILDCARD]) +└── http://localhost:4545/subdir/mt_application_x_javascript.j4.js ([WILDCARD]) diff --git a/cli/tests/testdata/npm/cached_only/main.out b/cli/tests/testdata/npm/cached_only/main.out index f49494839..d03420fee 100644 --- a/cli/tests/testdata/npm/cached_only/main.out +++ b/cli/tests/testdata/npm/cached_only/main.out @@ -1,4 +1,2 @@ -error: Error getting response at http://localhost:4545/npm/registry/chalk for package "chalk" - -Caused by: - An npm specifier not found in cache: "chalk", --cached-only is specified. +error: Error getting response at http://localhost:4545/npm/registry/chalk for package "chalk": An npm specifier not found in cache: "chalk", --cached-only is specified. + at file:///[WILDCARD]/testdata/npm/cached_only/main.ts:1:19 diff --git a/cli/tests/testdata/npm/cjs_with_deps/main.out b/cli/tests/testdata/npm/cjs_with_deps/main.out index 23c217f7a..3a16ff467 100644 --- a/cli/tests/testdata/npm/cjs_with_deps/main.out +++ b/cli/tests/testdata/npm/cjs_with_deps/main.out @@ -1,5 +1,7 @@ -Download http://localhost:4545/npm/registry/chai Download http://localhost:4545/npm/registry/chalk +Download http://localhost:4545/npm/registry/chai +Download http://localhost:4545/npm/registry/ansi-styles +Download http://localhost:4545/npm/registry/supports-color Download http://localhost:4545/npm/registry/assertion-error Download http://localhost:4545/npm/registry/check-error Download http://localhost:4545/npm/registry/deep-eql @@ -7,8 +9,6 @@ Download http://localhost:4545/npm/registry/get-func-name Download http://localhost:4545/npm/registry/loupe Download http://localhost:4545/npm/registry/pathval Download http://localhost:4545/npm/registry/type-detect -Download http://localhost:4545/npm/registry/ansi-styles -Download http://localhost:4545/npm/registry/supports-color Download http://localhost:4545/npm/registry/color-convert Download http://localhost:4545/npm/registry/has-flag Download http://localhost:4545/npm/registry/color-name diff --git a/cli/tests/testdata/npm/cjs_with_deps/main_info.out b/cli/tests/testdata/npm/cjs_with_deps/main_info.out index 345583a90..cf84197e1 100644 --- a/cli/tests/testdata/npm/cjs_with_deps/main_info.out +++ b/cli/tests/testdata/npm/cjs_with_deps/main_info.out @@ -4,19 +4,19 @@ dependencies: 14 unique size: [WILDCARD] file:///[WILDCARD]/npm/cjs_with_deps/main.js ([WILDCARD]) -├─┬ npm:chai@4.3 - 4.3.6 ([WILDCARD]) -│ ├── npm:assertion-error@1.1.0 ([WILDCARD]) -│ ├── npm:check-error@1.0.2 ([WILDCARD]) -│ ├─┬ npm:deep-eql@3.0.1 ([WILDCARD]) -│ │ └── npm:type-detect@4.0.8 ([WILDCARD]) -│ ├── npm:get-func-name@2.0.0 ([WILDCARD]) -│ ├─┬ npm:loupe@2.3.4 ([WILDCARD]) -│ │ └── npm:get-func-name@2.0.0 ([WILDCARD]) -│ ├── npm:pathval@1.1.1 ([WILDCARD]) -│ └── npm:type-detect@4.0.8 ([WILDCARD]) -└─┬ npm:chalk@4 - 4.1.2 ([WILDCARD]) - ├─┬ npm:ansi-styles@4.3.0 ([WILDCARD]) - │ └─┬ npm:color-convert@2.0.1 ([WILDCARD]) - │ └── npm:color-name@1.1.4 ([WILDCARD]) - └─┬ npm:supports-color@7.2.0 ([WILDCARD]) - └── npm:has-flag@4.0.0 ([WILDCARD]) +├─┬ npm:chalk@4.1.2 ([WILDCARD]) +│ ├─┬ npm:ansi-styles@4.3.0 ([WILDCARD]) +│ │ └─┬ npm:color-convert@2.0.1 ([WILDCARD]) +│ │ └── npm:color-name@1.1.4 ([WILDCARD]) +│ └─┬ npm:supports-color@7.2.0 ([WILDCARD]) +│ └── npm:has-flag@4.0.0 ([WILDCARD]) +└─┬ npm:chai@4.3.6 ([WILDCARD]) + ├── npm:assertion-error@1.1.0 ([WILDCARD]) + ├── npm:check-error@1.0.2 ([WILDCARD]) + ├─┬ npm:deep-eql@3.0.1 ([WILDCARD]) + │ └── npm:type-detect@4.0.8 ([WILDCARD]) + ├── npm:get-func-name@2.0.0 ([WILDCARD]) + ├─┬ npm:loupe@2.3.4 ([WILDCARD]) + │ └── npm:get-func-name@2.0.0 ([WILDCARD]) + ├── npm:pathval@1.1.1 ([WILDCARD]) + └── npm:type-detect@4.0.8 ([WILDCARD]) diff --git a/cli/tests/testdata/npm/cjs_with_deps/main_info_json.out b/cli/tests/testdata/npm/cjs_with_deps/main_info_json.out index bc7b9e162..e2a659a42 100644 --- a/cli/tests/testdata/npm/cjs_with_deps/main_info_json.out +++ b/cli/tests/testdata/npm/cjs_with_deps/main_info_json.out @@ -4,43 +4,43 @@ ], "modules": [ { + "kind": "esm", "dependencies": [ { - "specifier": "npm:chai@4.3", + "specifier": "npm:chalk@4", "code": { - "specifier": "npm:chai@4.3", + "specifier": "npm:chalk@4", "span": { "start": { - "line": 1, - "character": 23 + "line": 0, + "character": 18 }, "end": { - "line": 1, - "character": 37 + "line": 0, + "character": 31 } } }, - "npmPackage": "chai@4.3.6" + "npmPackage": "chalk@4.1.2" }, { - "specifier": "npm:chalk@4", + "specifier": "npm:chai@4.3", "code": { - "specifier": "npm:chalk@4", + "specifier": "npm:chai@4.3", "span": { "start": { - "line": 0, - "character": 18 + "line": 1, + "character": 23 }, "end": { - "line": 0, - "character": 31 + "line": 1, + "character": 37 } } }, - "npmPackage": "chalk@4.1.2" + "npmPackage": "chai@4.3.6" } ], - "kind": "esm", "local": "[WILDCARD]main.js", "emit": null, "map": null, @@ -49,7 +49,10 @@ "specifier": "[WILDCARD]/main.js" } ], - "redirects": {}, + "redirects": { + "npm:chai@4.3": "npm:chai@4.3.6", + "npm:chalk@4": "npm:chalk@4.1.2" + }, "npmPackages": { "ansi-styles@4.3.0": { "name": "ansi-styles", diff --git a/cli/tests/testdata/npm/compare_globals/main.out b/cli/tests/testdata/npm/compare_globals/main.out index e60a39ba6..286834168 100644 --- a/cli/tests/testdata/npm/compare_globals/main.out +++ b/cli/tests/testdata/npm/compare_globals/main.out @@ -1,5 +1,5 @@ -Download http://localhost:4545/npm/registry/@denotest/globals Download http://localhost:4545/npm/registry/@types/node +Download http://localhost:4545/npm/registry/@denotest/globals Download http://localhost:4545/npm/registry/@denotest/globals/1.0.0.tgz Download http://localhost:4545/npm/registry/@types/node/node-18.8.2.tgz Check file:///[WILDCARD]/npm/compare_globals/main.ts diff --git a/cli/tests/testdata/npm/import_map/main.out b/cli/tests/testdata/npm/import_map/main.out index b5b67651e..29f0f4283 100644 --- a/cli/tests/testdata/npm/import_map/main.out +++ b/cli/tests/testdata/npm/import_map/main.out @@ -1,5 +1,5 @@ -Download http://localhost:4545/npm/registry/@denotest/dual-cjs-esm Download http://localhost:4545/npm/registry/chalk +Download http://localhost:4545/npm/registry/@denotest/dual-cjs-esm Download http://localhost:4545/npm/registry/@denotest/dual-cjs-esm/1.0.0.tgz Download http://localhost:4545/npm/registry/chalk/chalk-5.0.1.tgz chalk import map loads diff --git a/cli/tests/testdata/npm/info/chalk.out b/cli/tests/testdata/npm/info/chalk.out index 89ea05e71..d7ac95120 100644 --- a/cli/tests/testdata/npm/info/chalk.out +++ b/cli/tests/testdata/npm/info/chalk.out @@ -1,8 +1,7 @@ -type: Unknown dependencies: 5 unique size: [WILDCARD] -npm:chalk@4 - 4.1.2 ([WILDCARD]) +npm:chalk@4.1.2 ([WILDCARD]) ├─┬ npm:ansi-styles@4.3.0 ([WILDCARD]) │ └─┬ npm:color-convert@2.0.1 ([WILDCARD]) │ └── npm:color-name@1.1.4 ([WILDCARD]) diff --git a/cli/tests/testdata/npm/info/chalk_json.out b/cli/tests/testdata/npm/info/chalk_json.out index f6673d032..0f86bc994 100644 --- a/cli/tests/testdata/npm/info/chalk_json.out +++ b/cli/tests/testdata/npm/info/chalk_json.out @@ -5,11 +5,13 @@ "modules": [ { "kind": "npm", - "specifier": "npm:chalk@4", + "specifier": "npm:chalk@4.1.2", "npmPackage": "chalk@4.1.2" } ], - "redirects": {}, + "redirects": { + "npm:chalk@4": "npm:chalk@4.1.2" + }, "npmPackages": { "ansi-styles@4.3.0": { "name": "ansi-styles", diff --git a/cli/tests/testdata/npm/peer_deps_with_copied_folders/main_info.out b/cli/tests/testdata/npm/peer_deps_with_copied_folders/main_info.out index c9c4a59c1..d85b00094 100644 --- a/cli/tests/testdata/npm/peer_deps_with_copied_folders/main_info.out +++ b/cli/tests/testdata/npm/peer_deps_with_copied_folders/main_info.out @@ -4,11 +4,11 @@ dependencies: 6 unique size: [WILDCARD] file:///[WILDCARD]/testdata/npm/peer_deps_with_copied_folders/main.ts (171B) -├─┬ npm:@denotest/peer-dep-test-child@1 - 1.0.0 ([WILDCARD]) +├─┬ npm:@denotest/peer-dep-test-child@1.0.0 ([WILDCARD]) │ ├─┬ npm:@denotest/peer-dep-test-grandchild@1.0.0_@denotest+peer-dep-test-peer@1.0.0 ([WILDCARD]) │ │ └── npm:@denotest/peer-dep-test-peer@1.0.0 ([WILDCARD]) │ └── npm:@denotest/peer-dep-test-peer@1.0.0 ([WILDCARD]) -└─┬ npm:@denotest/peer-dep-test-child@2 - 2.0.0 ([WILDCARD]) +└─┬ npm:@denotest/peer-dep-test-child@2.0.0 ([WILDCARD]) ├─┬ npm:@denotest/peer-dep-test-grandchild@1.0.0_@denotest+peer-dep-test-peer@2.0.0 ([WILDCARD]) │ └── npm:@denotest/peer-dep-test-peer@2.0.0 ([WILDCARD]) └── npm:@denotest/peer-dep-test-peer@2.0.0 ([WILDCARD]) diff --git a/cli/tests/testdata/npm/peer_deps_with_copied_folders/main_info_json.out b/cli/tests/testdata/npm/peer_deps_with_copied_folders/main_info_json.out index 634ec6251..6a455b001 100644 --- a/cli/tests/testdata/npm/peer_deps_with_copied_folders/main_info_json.out +++ b/cli/tests/testdata/npm/peer_deps_with_copied_folders/main_info_json.out @@ -4,6 +4,7 @@ ], "modules": [ { + "kind": "esm", "dependencies": [ { "specifier": "npm:@denotest/peer-dep-test-child@1", @@ -40,7 +41,6 @@ "npmPackage": "@denotest/peer-dep-test-child@2.0.0_@denotest+peer-dep-test-peer@2.0.0" } ], - "kind": "esm", "local": "[WILDCARD]main.ts", "emit": null, "map": null, @@ -49,7 +49,10 @@ "specifier": "file://[WILDCARD]/main.ts" } ], - "redirects": {}, + "redirects": { + "npm:@denotest/peer-dep-test-child@1": "npm:@denotest/peer-dep-test-child@1.0.0", + "npm:@denotest/peer-dep-test-child@2": "npm:@denotest/peer-dep-test-child@2.0.0" + }, "npmPackages": { "@denotest/peer-dep-test-child@1.0.0_@denotest+peer-dep-test-peer@1.0.0": { "name": "@denotest/peer-dep-test-child", diff --git a/cli/tests/testdata/npm/typescript_file_in_package/main.out b/cli/tests/testdata/npm/typescript_file_in_package/main.out index ba53f7725..0d6f53cd9 100644 --- a/cli/tests/testdata/npm/typescript_file_in_package/main.out +++ b/cli/tests/testdata/npm/typescript_file_in_package/main.out @@ -1,6 +1,6 @@ Download http://localhost:4545/npm/registry/@denotest/typescript-file Download http://localhost:4545/npm/registry/@denotest/typescript-file/1.0.0.tgz -error: Could not resolve 'npm:@denotest/typescript-file'. +error: Could not resolve 'npm:@denotest/typescript-file@1.0.0'. Caused by: TypeScript files are not supported in npm packages: file:///[WILDCARD]/@denotest/typescript-file/1.0.0/index.ts diff --git a/cli/tools/bench.rs b/cli/tools/bench.rs index 447cb1fcb..cd6e91c9f 100644 --- a/cli/tools/bench.rs +++ b/cli/tools/bench.rs @@ -642,7 +642,7 @@ pub async fn run_benchmarks_with_watch( output: &mut HashSet<&'a ModuleSpecifier>, no_check: bool, ) { - if let Some(module) = maybe_module { + if let Some(module) = maybe_module.and_then(|m| m.esm()) { for dep in module.dependencies.values() { if let Some(specifier) = &dep.get_code() { if !output.contains(specifier) { @@ -671,6 +671,7 @@ pub async fn run_benchmarks_with_watch( } } } + // This bench module and all it's dependencies let mut modules = HashSet::new(); modules.insert(&specifier); diff --git a/cli/tools/bundle.rs b/cli/tools/bundle.rs index 6a9019cd8..d75da5ec7 100644 --- a/cli/tools/bundle.rs +++ b/cli/tools/bundle.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use deno_core::error::AnyError; use deno_core::futures::FutureExt; use deno_core::resolve_url_or_path; +use deno_graph::Module; use deno_runtime::colors; use crate::args::BundleFlags; @@ -48,8 +49,12 @@ pub async fn bundle( let mut paths_to_watch: Vec<PathBuf> = graph .specifiers() .filter_map(|(_, r)| { - r.ok() - .and_then(|module| module.specifier.to_file_path().ok()) + r.ok().and_then(|module| match module { + Module::Esm(m) => m.specifier.to_file_path().ok(), + Module::Json(m) => m.specifier.to_file_path().ok(), + // nothing to watch + Module::Node(_) | Module::Npm(_) | Module::External(_) => None, + }) }) .collect(); diff --git a/cli/tools/check.rs b/cli/tools/check.rs index bf5e3033f..a29c4cea8 100644 --- a/cli/tools/check.rs +++ b/cli/tools/check.rs @@ -5,8 +5,8 @@ use std::sync::Arc; use deno_ast::MediaType; use deno_ast::ModuleSpecifier; use deno_core::error::AnyError; +use deno_graph::Module; use deno_graph::ModuleGraph; -use deno_graph::ModuleKind; use deno_runtime::colors; use once_cell::sync::Lazy; use regex::Regex; @@ -39,11 +39,6 @@ pub struct CheckOptions { /// If true, valid `.tsbuildinfo` files will be ignored and type checking /// will always occur. pub reload: bool, - /// If the graph has a node built-in specifier. - /// - /// Although this could be derived from the graph, this helps - /// speed things up. - pub has_node_builtin_specifier: bool, } /// The result of a check of a module graph. @@ -81,8 +76,7 @@ pub fn check( } } - let root_names = - get_tsc_roots(&graph, options.has_node_builtin_specifier, check_js); + let root_names = get_tsc_roots(&graph, check_js); // while there might be multiple roots, we can't "merge" the build info, so we // try to retrieve the build info for first root, which is the most common use // case. @@ -168,45 +162,53 @@ fn get_check_hash( let check_js = options.ts_config.get_check_js(); let mut sorted_modules = graph.modules().collect::<Vec<_>>(); - sorted_modules.sort_by_key(|m| m.specifier.as_str()); // make it deterministic + sorted_modules.sort_by_key(|m| m.specifier().as_str()); // make it deterministic let mut has_file = false; let mut has_file_to_type_check = false; for module in sorted_modules { - let ts_check = - has_ts_check(module.media_type, module.maybe_source.as_deref()); - if ts_check { - has_file_to_type_check = true; - } + match module { + Module::Esm(module) => { + let ts_check = has_ts_check(module.media_type, &module.source); + if ts_check { + has_file_to_type_check = true; + } - match module.media_type { - MediaType::TypeScript - | MediaType::Dts - | MediaType::Dmts - | MediaType::Dcts - | MediaType::Mts - | MediaType::Cts - | MediaType::Tsx => { - has_file = true; - has_file_to_type_check = true; - } - MediaType::JavaScript - | MediaType::Mjs - | MediaType::Cjs - | MediaType::Jsx => { - has_file = true; - if !check_js && !ts_check { - continue; + match module.media_type { + MediaType::TypeScript + | MediaType::Dts + | MediaType::Dmts + | MediaType::Dcts + | MediaType::Mts + | MediaType::Cts + | MediaType::Tsx => { + has_file = true; + has_file_to_type_check = true; + } + MediaType::JavaScript + | MediaType::Mjs + | MediaType::Cjs + | MediaType::Jsx => { + has_file = true; + if !check_js && !ts_check { + continue; + } + } + MediaType::Json + | MediaType::TsBuildInfo + | MediaType::SourceMap + | MediaType::Wasm + | MediaType::Unknown => continue, } + + hasher.write_str(module.specifier.as_str()); + hasher.write_str(&module.source); + } + Module::Json(_) + | Module::External(_) + | Module::Node(_) + | Module::Npm(_) => { + // ignore } - MediaType::Json - | MediaType::TsBuildInfo - | MediaType::SourceMap - | MediaType::Wasm - | MediaType::Unknown => continue, - } - hasher.write_str(module.specifier.as_str()); - if let Some(code) = &module.maybe_source { - hasher.write_str(code); } } @@ -226,38 +228,43 @@ fn get_check_hash( /// otherwise they would be ignored if only imported into JavaScript. fn get_tsc_roots( graph: &ModuleGraph, - has_node_builtin_specifier: bool, check_js: bool, ) -> Vec<(ModuleSpecifier, MediaType)> { let mut result = Vec::new(); - if has_node_builtin_specifier { + if graph.has_node_specifier { // inject a specifier that will resolve node types result.push(( ModuleSpecifier::parse("asset:///node_types.d.ts").unwrap(), MediaType::Dts, )); } - result.extend(graph.modules().filter_map(|module| { - if module.kind == ModuleKind::External || module.maybe_source.is_none() { - return None; - } - match module.media_type { + result.extend(graph.modules().filter_map(|module| match module { + Module::Esm(module) => match module.media_type { MediaType::TypeScript | MediaType::Tsx | MediaType::Mts | MediaType::Cts | MediaType::Jsx => Some((module.specifier.clone(), module.media_type)), - MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs - if check_js - || has_ts_check( - module.media_type, - module.maybe_source.as_deref(), - ) => - { - Some((module.specifier.clone(), module.media_type)) + MediaType::JavaScript | MediaType::Mjs | MediaType::Cjs => { + if check_js || has_ts_check(module.media_type, &module.source) { + Some((module.specifier.clone(), module.media_type)) + } else { + None + } } - _ => None, - } + MediaType::Json + | MediaType::Dts + | MediaType::Dmts + | MediaType::Dcts + | MediaType::Wasm + | MediaType::TsBuildInfo + | MediaType::SourceMap + | MediaType::Unknown => None, + }, + Module::External(_) + | Module::Node(_) + | Module::Npm(_) + | Module::Json(_) => None, })); result } @@ -266,11 +273,7 @@ fn get_tsc_roots( static TS_CHECK_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r#"(?i)^\s*@ts-check(?:\s+|$)"#).unwrap()); -fn has_ts_check(media_type: MediaType, maybe_file_text: Option<&str>) -> bool { - let file_text = match maybe_file_text { - Some(text) => text, - None => return false, - }; +fn has_ts_check(media_type: MediaType, file_text: &str) -> bool { match &media_type { MediaType::JavaScript | MediaType::Mjs @@ -278,7 +281,18 @@ fn has_ts_check(media_type: MediaType, maybe_file_text: Option<&str>) -> bool { | MediaType::Jsx => get_leading_comments(file_text) .iter() .any(|text| TS_CHECK_RE.is_match(text)), - _ => false, + MediaType::TypeScript + | MediaType::Mts + | MediaType::Cts + | MediaType::Dts + | MediaType::Dcts + | MediaType::Dmts + | MediaType::Tsx + | MediaType::Json + | MediaType::Wasm + | MediaType::TsBuildInfo + | MediaType::SourceMap + | MediaType::Unknown => false, } } @@ -374,20 +388,19 @@ mod test { fn has_ts_check_test() { assert!(has_ts_check( MediaType::JavaScript, - Some("// @ts-check\nconsole.log(5);") + "// @ts-check\nconsole.log(5);" )); assert!(has_ts_check( MediaType::JavaScript, - Some("// deno-lint-ignore\n// @ts-check\n") + "// deno-lint-ignore\n// @ts-check\n" )); assert!(!has_ts_check( MediaType::JavaScript, - Some("test;\n// @ts-check\n") + "test;\n// @ts-check\n" )); assert!(!has_ts_check( MediaType::JavaScript, - Some("// ts-check\nconsole.log(5);") + "// ts-check\nconsole.log(5);" )); - assert!(!has_ts_check(MediaType::TypeScript, None,)); } } diff --git a/cli/tools/info.rs b/cli/tools/info.rs index 2f9b2a183..8a7f4b6b9 100644 --- a/cli/tools/info.rs +++ b/cli/tools/info.rs @@ -10,7 +10,8 @@ use deno_core::error::AnyError; use deno_core::resolve_url_or_path; use deno_core::serde_json; use deno_core::serde_json::json; -use deno_graph::npm::NpmPackageReq; +use deno_graph::npm::NpmPackageNv; +use deno_graph::npm::NpmPackageNvReference; use deno_graph::npm::NpmPackageReqReference; use deno_graph::Dependency; use deno_graph::Module; @@ -141,7 +142,7 @@ fn add_npm_packages_to_json( let modules = json.get_mut("modules").and_then(|m| m.as_array_mut()); if let Some(modules) = modules { if modules.len() == 1 - && modules[0].get("kind").and_then(|k| k.as_str()) == Some("external") + && modules[0].get("kind").and_then(|k| k.as_str()) == Some("npm") { // If there is only one module and it's "external", then that means // someone provided an npm specifier as a cli argument. In this case, @@ -150,10 +151,10 @@ fn add_npm_packages_to_json( let maybe_package = module .get("specifier") .and_then(|k| k.as_str()) - .and_then(|specifier| NpmPackageReqReference::from_str(specifier).ok()) + .and_then(|specifier| NpmPackageNvReference::from_str(specifier).ok()) .and_then(|package_ref| { snapshot - .resolve_package_from_deno_module(&package_ref.req) + .resolve_package_from_deno_module(&package_ref.nv) .ok() }); if let Some(pkg) = maybe_package { @@ -162,8 +163,6 @@ fn add_npm_packages_to_json( "npmPackage".to_string(), pkg.pkg_id.as_serialized().into(), ); - // change the "kind" to be "npm" - module.insert("kind".to_string(), "npm".into()); } } } else { @@ -173,7 +172,10 @@ fn add_npm_packages_to_json( // references. So there could be listed multiple npm specifiers // that would resolve to a single npm package. for i in (0..modules.len()).rev() { - if modules[i].get("kind").and_then(|k| k.as_str()) == Some("external") { + if matches!( + modules[i].get("kind").and_then(|k| k.as_str()), + Some("npm") | Some("external") + ) { modules.remove(i); } } @@ -189,8 +191,7 @@ fn add_npm_packages_to_json( let specifier = dep.get("specifier").and_then(|s| s.as_str()); if let Some(specifier) = specifier { if let Ok(npm_ref) = NpmPackageReqReference::from_str(specifier) { - if let Ok(pkg) = - snapshot.resolve_package_from_deno_module(&npm_ref.req) + if let Ok(pkg) = snapshot.resolve_pkg_from_pkg_req(&npm_ref.req) { dep.insert( "npmPackage".to_string(), @@ -303,9 +304,8 @@ fn print_tree_node<TWrite: Write>( #[derive(Default)] struct NpmInfo { package_sizes: HashMap<NpmPackageId, u64>, - resolved_reqs: HashMap<NpmPackageReq, NpmPackageId>, + resolved_ids: HashMap<NpmPackageNv, NpmPackageId>, packages: HashMap<NpmPackageId, NpmResolutionPackage>, - specifiers: HashMap<ModuleSpecifier, NpmPackageReq>, } impl NpmInfo { @@ -315,21 +315,15 @@ impl NpmInfo { npm_snapshot: &'a NpmResolutionSnapshot, ) -> Self { let mut info = NpmInfo::default(); - if !npm_resolver.has_packages() { - return info; // skip going over the specifiers if there's no npm packages + if graph.npm_packages.is_empty() { + return info; // skip going over the modules if there's no npm packages } - for (specifier, _) in graph.specifiers() { - if let Ok(reference) = NpmPackageReqReference::from_specifier(specifier) { - info - .specifiers - .insert(specifier.clone(), reference.req.clone()); - if let Ok(package) = - npm_snapshot.resolve_package_from_deno_module(&reference.req) - { - info - .resolved_reqs - .insert(reference.req, package.pkg_id.clone()); + for module in graph.modules() { + if let Module::Npm(module) = module { + let nv = &module.nv_reference.nv; + if let Ok(package) = npm_snapshot.resolve_package_from_deno_module(nv) { + info.resolved_ids.insert(nv.clone(), package.pkg_id.clone()); if !info.packages.contains_key(&package.pkg_id) { info.fill_package_info(package, npm_resolver, npm_snapshot); } @@ -361,15 +355,12 @@ impl NpmInfo { } } - pub fn package_from_specifier( + pub fn resolve_package( &self, - specifier: &ModuleSpecifier, + nv: &NpmPackageNv, ) -> Option<&NpmResolutionPackage> { - self - .specifiers - .get(specifier) - .and_then(|package_req| self.resolved_reqs.get(package_req)) - .and_then(|id| self.packages.get(id)) + let id = self.resolved_ids.get(nv)?; + self.packages.get(id) } } @@ -407,7 +398,12 @@ impl<'a> GraphDisplayContext<'a> { let root_specifier = self.graph.resolve(&self.graph.roots[0]); match self.graph.try_get(&root_specifier) { Ok(Some(root)) => { - if let Some(cache_info) = root.maybe_cache_info.as_ref() { + let maybe_cache_info = match root { + Module::Esm(module) => module.maybe_cache_info.as_ref(), + Module::Json(module) => module.maybe_cache_info.as_ref(), + Module::Node(_) | Module::Npm(_) | Module::External(_) => None, + }; + if let Some(cache_info) = maybe_cache_info { if let Some(local) = &cache_info.local { writeln!( writer, @@ -433,9 +429,21 @@ impl<'a> GraphDisplayContext<'a> { )?; } } - writeln!(writer, "{} {}", colors::bold("type:"), root.media_type)?; - let total_modules_size = - self.graph.modules().map(|m| m.size() as f64).sum::<f64>(); + if let Some(module) = root.esm() { + writeln!(writer, "{} {}", colors::bold("type:"), module.media_type)?; + } + let total_modules_size = self + .graph + .modules() + .map(|m| { + let size = match m { + Module::Esm(module) => module.size(), + Module::Json(module) => module.size(), + Module::Node(_) | Module::Npm(_) | Module::External(_) => 0, + }; + size as f64 + }) + .sum::<f64>(); let total_npm_package_size = self .npm_info .package_sizes @@ -443,9 +451,9 @@ impl<'a> GraphDisplayContext<'a> { .map(|s| *s as f64) .sum::<f64>(); let total_size = total_modules_size + total_npm_package_size; - let dep_count = self.graph.modules().count() - 1 + let dep_count = self.graph.modules().count() - 1 // -1 for the root module + self.npm_info.packages.len() - - self.npm_info.resolved_reqs.len(); + - self.npm_info.resolved_ids.len(); writeln!( writer, "{} {} unique", @@ -507,42 +515,39 @@ impl<'a> GraphDisplayContext<'a> { use PackageOrSpecifier::*; - let package_or_specifier = - match self.npm_info.package_from_specifier(&module.specifier) { + let package_or_specifier = match module.npm() { + Some(npm) => match self.npm_info.resolve_package(&npm.nv_reference.nv) { Some(package) => Package(package.clone()), - None => Specifier(module.specifier.clone()), - }; + None => Specifier(module.specifier().clone()), // should never happen + }, + None => Specifier(module.specifier().clone()), + }; let was_seen = !self.seen.insert(match &package_or_specifier { Package(package) => package.pkg_id.as_serialized(), Specifier(specifier) => specifier.to_string(), }); let header_text = if was_seen { let specifier_str = if type_dep { - colors::italic_gray(&module.specifier).to_string() + colors::italic_gray(module.specifier()).to_string() } else { - colors::gray(&module.specifier).to_string() + colors::gray(module.specifier()).to_string() }; format!("{} {}", specifier_str, colors::gray("*")) } else { - let specifier_str = if type_dep { - colors::italic(&module.specifier).to_string() + let header_text = if type_dep { + colors::italic(module.specifier()).to_string() } else { - module.specifier.to_string() - }; - let header_text = match &package_or_specifier { - Package(package) => { - format!("{} - {}", specifier_str, package.pkg_id.nv.version) - } - Specifier(_) => specifier_str, + module.specifier().to_string() }; let maybe_size = match &package_or_specifier { Package(package) => { self.npm_info.package_sizes.get(&package.pkg_id).copied() } - Specifier(_) => module - .maybe_source - .as_ref() - .map(|s| s.as_bytes().len() as u64), + Specifier(_) => match module { + Module::Esm(module) => Some(module.size() as u64), + Module::Json(module) => Some(module.size() as u64), + Module::Node(_) | Module::Npm(_) | Module::External(_) => None, + }, }; format!("{} {}", header_text, maybe_size_to_text(maybe_size)) }; @@ -550,20 +555,22 @@ impl<'a> GraphDisplayContext<'a> { let mut tree_node = TreeNode::from_text(header_text); if !was_seen { - if let Some(types_dep) = &module.maybe_types_dependency { - if let Some(child) = - self.build_resolved_info(&types_dep.dependency, true) - { - tree_node.children.push(child); - } - } match &package_or_specifier { Package(package) => { tree_node.children.extend(self.build_npm_deps(package)); } Specifier(_) => { - for dep in module.dependencies.values() { - tree_node.children.extend(self.build_dep_info(dep)); + if let Some(module) = module.esm() { + if let Some(types_dep) = &module.maybe_types_dependency { + if let Some(child) = + self.build_resolved_info(&types_dep.dependency, true) + { + tree_node.children.push(child); + } + } + for dep in module.dependencies.values() { + tree_node.children.extend(self.build_dep_info(dep)); + } } } } diff --git a/cli/tools/test.rs b/cli/tools/test.rs index d308de8de..d0f3013b3 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -1389,7 +1389,7 @@ pub async fn run_tests_with_watch( output: &mut HashSet<&'a ModuleSpecifier>, no_check: bool, ) { - if let Some(module) = maybe_module { + if let Some(module) = maybe_module.and_then(|m| m.esm()) { for dep in module.dependencies.values() { if let Some(specifier) = &dep.get_code() { if !output.contains(specifier) { diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index 89130f391..3bee843fd 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -10,9 +10,9 @@ use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; +use deno_graph::EsmModule; use deno_graph::Module; use deno_graph::ModuleGraph; -use deno_graph::ModuleKind; use import_map::ImportMap; use import_map::SpecifierMap; @@ -93,7 +93,7 @@ pub fn build( let all_modules = graph.modules().collect::<Vec<_>>(); let remote_modules = all_modules .iter() - .filter(|m| is_remote_specifier(&m.specifier)) + .filter(|m| is_remote_specifier(m.specifier())) .copied() .collect::<Vec<_>>(); let mappings = @@ -101,21 +101,16 @@ pub fn build( // write out all the files for module in &remote_modules { - let source = match &module.maybe_source { - Some(source) => source, - None => continue, + let source = match module { + Module::Esm(module) => &module.source, + Module::Json(module) => &module.source, + Module::Node(_) | Module::Npm(_) | Module::External(_) => continue, }; + let specifier = module.specifier(); let local_path = mappings - .proxied_path(&module.specifier) - .unwrap_or_else(|| mappings.local_path(&module.specifier)); - if !matches!(module.kind, ModuleKind::Esm | ModuleKind::Asserted) { - log::warn!( - "Unsupported module kind {:?} for {}", - module.kind, - module.specifier - ); - continue; - } + .proxied_path(specifier) + .unwrap_or_else(|| mappings.local_path(specifier)); + environment.create_dir_all(local_path.parent().unwrap())?; environment.write_file(&local_path, source)?; } @@ -123,7 +118,7 @@ pub fn build( // write out the proxies for (specifier, proxied_module) in mappings.proxied_modules() { let proxy_path = mappings.local_path(specifier); - let module = graph.get(specifier).unwrap(); + let module = graph.get(specifier).unwrap().esm().unwrap(); let text = build_proxy_module_source(module, proxied_module, parsed_source_cache)?; @@ -185,7 +180,7 @@ fn validate_original_import_map( } fn build_proxy_module_source( - module: &Module, + module: &EsmModule, proxied_module: &ProxiedModule, parsed_source_cache: &ParsedSourceCache, ) -> Result<String, AnyError> { @@ -211,13 +206,11 @@ fn build_proxy_module_source( writeln!(text, "export * from \"{relative_specifier}\";").unwrap(); // add a default export if one exists in the module - if let Some(parsed_source) = - parsed_source_cache.get_parsed_source_from_module(module)? - { - if has_default_export(&parsed_source) { - writeln!(text, "export {{ default }} from \"{relative_specifier}\";") - .unwrap(); - } + let parsed_source = + parsed_source_cache.get_parsed_source_from_esm_module(module)?; + if has_default_export(&parsed_source) { + writeln!(text, "export {{ default }} from \"{relative_specifier}\";") + .unwrap(); } Ok(text) diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index 3d2c1efd9..916eb55c5 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -4,7 +4,6 @@ use deno_ast::LineAndColumnIndex; use deno_ast::ModuleSpecifier; use deno_ast::SourceTextInfo; use deno_core::error::AnyError; -use deno_graph::MediaType; use deno_graph::Module; use deno_graph::ModuleGraph; use deno_graph::Position; @@ -205,21 +204,20 @@ fn visit_modules( parsed_source_cache: &ParsedSourceCache, ) -> Result<(), AnyError> { for module in modules { - if module.media_type == MediaType::Json { + let module = match module { + Module::Esm(module) => module, // skip visiting Json modules as they are leaves - continue; - } - - let text_info = - match parsed_source_cache.get_parsed_source_from_module(module)? { - Some(source) => source.text_info().clone(), - None => continue, - }; - let source_text = match &module.maybe_source { - Some(source) => source, - None => continue, + Module::Json(_) + | Module::Npm(_) + | Module::Node(_) + | Module::External(_) => continue, }; + let parsed_source = + parsed_source_cache.get_parsed_source_from_esm_module(module)?; + let text_info = parsed_source.text_info().clone(); + let source_text = &module.source; + for dep in module.dependencies.values() { visit_resolution( &dep.maybe_code, @@ -291,7 +289,7 @@ fn handle_dep_specifier( mappings: &Mappings, ) { let specifier = match graph.get(unresolved_specifier) { - Some(module) => module.specifier.clone(), + Some(module) => module.specifier().clone(), // Ignore when None. The graph was previous validated so this is a // dynamic import that was missing and is ignored for vendoring None => return, diff --git a/cli/tools/vendor/mappings.rs b/cli/tools/vendor/mappings.rs index 399002ea3..1ecc14edf 100644 --- a/cli/tools/vendor/mappings.rs +++ b/cli/tools/vendor/mappings.rs @@ -39,8 +39,9 @@ impl Mappings { remote_modules: &[&Module], output_dir: &Path, ) -> Result<Self, AnyError> { - let partitioned_specifiers = - partition_by_root_specifiers(remote_modules.iter().map(|m| &m.specifier)); + let partitioned_specifiers = partition_by_root_specifiers( + remote_modules.iter().map(|m| m.specifier()), + ); let mut mapped_paths = HashSet::new(); let mut mappings = HashMap::new(); let mut proxies = HashMap::new(); @@ -52,7 +53,12 @@ impl Mappings { &mut mapped_paths, ); for specifier in specifiers { - let media_type = graph.get(&specifier).unwrap().media_type; + let module = graph.get(&specifier).unwrap(); + let media_type = match module { + Module::Esm(module) => module.media_type, + Module::Json(_) => MediaType::Json, + Module::Node(_) | Module::Npm(_) | Module::External(_) => continue, + }; let sub_path = sanitize_filepath(&make_url_relative(&root, &{ let mut specifier = specifier.clone(); specifier.set_query(None); @@ -75,28 +81,30 @@ impl Mappings { // resolve all the "proxy" paths to use for when an x-typescript-types header is specified for module in remote_modules { - if let Some(resolved) = &module - .maybe_types_dependency - .as_ref() - .and_then(|d| d.dependency.ok()) - { - let range = &resolved.range; - // hack to tell if it's an x-typescript-types header - let is_ts_types_header = - range.start == Position::zeroed() && range.end == Position::zeroed(); - if is_ts_types_header { - let module_path = mappings.get(&module.specifier).unwrap(); - let proxied_path = get_unique_path( - path_with_stem_suffix(module_path, ".proxied"), - &mut mapped_paths, - ); - proxies.insert( - module.specifier.clone(), - ProxiedModule { - output_path: proxied_path, - declaration_specifier: resolved.specifier.clone(), - }, - ); + if let Some(module) = module.esm() { + if let Some(resolved) = &module + .maybe_types_dependency + .as_ref() + .and_then(|d| d.dependency.ok()) + { + let range = &resolved.range; + // hack to tell if it's an x-typescript-types header + let is_ts_types_header = range.start == Position::zeroed() + && range.end == Position::zeroed(); + if is_ts_types_header { + let module_path = mappings.get(&module.specifier).unwrap(); + let proxied_path = get_unique_path( + path_with_stem_suffix(module_path, ".proxied"), + &mut mapped_paths, + ); + proxies.insert( + module.specifier.clone(), + ProxiedModule { + output_path: proxied_path, + declaration_specifier: resolved.specifier.clone(), + }, + ); + } } } } diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index 874b329da..aed2a852c 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -20,6 +20,8 @@ use deno_graph::ModuleGraph; use import_map::ImportMap; use crate::cache::ParsedSourceCache; +use crate::npm::NpmRegistryApi; +use crate::npm::NpmResolution; use crate::resolver::CliGraphResolver; use super::build::VendorEnvironment; @@ -260,8 +262,19 @@ async fn build_test_graph( mut loader: TestLoader, analyzer: &dyn deno_graph::ModuleAnalyzer, ) -> ModuleGraph { - let resolver = original_import_map - .map(|m| CliGraphResolver::new(None, Some(Arc::new(m)), None)); + let resolver = original_import_map.map(|m| { + let npm_registry_api = NpmRegistryApi::new_uninitialized(); + let npm_resolution = + NpmResolution::new(npm_registry_api.clone(), None, None); + CliGraphResolver::new( + None, + Some(Arc::new(m)), + false, + npm_registry_api, + npm_resolution, + None, + ) + }); let mut graph = ModuleGraph::default(); graph .build( diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 00186ca3b..cc6350642 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -28,9 +28,10 @@ use deno_core::ModuleSpecifier; use deno_core::OpState; use deno_core::RuntimeOptions; use deno_core::Snapshot; +use deno_graph::npm::NpmPackageNvReference; use deno_graph::npm::NpmPackageReqReference; +use deno_graph::Module; use deno_graph::ModuleGraph; -use deno_graph::ModuleKind; use deno_graph::ResolutionResolved; use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::permissions::PermissionsContainer; @@ -546,8 +547,17 @@ fn op_load(state: &mut OpState, args: Value) -> Result<Value, AnyError> { &specifier }; let maybe_source = if let Some(module) = graph.get(specifier) { - media_type = module.media_type; - module.maybe_source.as_ref().map(|s| Cow::Borrowed(&**s)) + match module { + Module::Esm(module) => { + media_type = module.media_type; + Some(Cow::Borrowed(&*module.source)) + } + Module::Json(module) => { + media_type = MediaType::Json; + Some(Cow::Borrowed(&*module.source)) + } + Module::External(_) | Module::Npm(_) | Module::Node(_) => None, + } } else if state .maybe_npm_resolver .as_ref() @@ -622,86 +632,17 @@ fn op_resolve( } let graph = &state.graph; - let resolved_dep = match graph.get(&referrer).map(|m| &m.dependencies) { - Some(dependencies) => dependencies.get(&specifier).and_then(|d| { - if let Some(type_resolution) = d.maybe_type.ok() { - Some(type_resolution) - } else if let Some(code_resolution) = d.maybe_code.ok() { - Some(code_resolution) - } else { - None - } - }), - None => None, - }; + let resolved_dep = graph + .get(&referrer) + .and_then(|m| m.esm()) + .and_then(|m| m.dependencies.get(&specifier)) + .and_then(|d| d.maybe_type.ok().or_else(|| d.maybe_code.ok())); let maybe_result = match resolved_dep { Some(ResolutionResolved { specifier, .. }) => { - let module = match graph.get(specifier) { - Some(module) => { - let maybe_types_dep = module - .maybe_types_dependency - .as_ref() - .map(|d| &d.dependency); - match maybe_types_dep.and_then(|d| d.maybe_specifier()) { - Some(specifier) => graph.get(specifier), - _ => Some(module), - } - } - _ => None, - }; - if let Some(module) = module { - if module.kind == ModuleKind::External { - // handle npm:<package> urls - if let Ok(npm_ref) = - NpmPackageReqReference::from_specifier(&module.specifier) - { - if let Some(npm_resolver) = &state.maybe_npm_resolver { - Some(resolve_npm_package_reference_types( - &npm_ref, - npm_resolver, - )?) - } else { - None - } - } else { - None - } - } else { - Some((module.specifier.clone(), module.media_type)) - } - } else { - None - } - } - _ => { - if let Some(npm_resolver) = state.maybe_npm_resolver.as_ref() { - if npm_resolver.in_npm_package(&referrer) { - // we're in an npm package, so use node resolution - Some(NodeResolution::into_specifier_and_media_type( - node::node_resolve( - &specifier, - &referrer, - NodeResolutionMode::Types, - npm_resolver, - &mut PermissionsContainer::allow_all(), - ) - .ok() - .flatten(), - )) - } else if let Ok(npm_ref) = - NpmPackageReqReference::from_str(&specifier) - { - // this could occur when resolving npm:@types/node when it is - // injected and not part of the graph - Some(resolve_npm_package_reference_types(&npm_ref, npm_resolver)?) - } else { - None - } - } else { - None - } + resolve_graph_specifier_types(specifier, state)? } + _ => resolve_non_graph_specifier_types(&specifier, &referrer, state)?, }; let result = match maybe_result { Some((specifier, media_type)) => { @@ -740,8 +681,89 @@ fn op_resolve( Ok(resolved) } +fn resolve_graph_specifier_types( + specifier: &ModuleSpecifier, + state: &State, +) -> Result<Option<(ModuleSpecifier, MediaType)>, AnyError> { + let graph = &state.graph; + let maybe_module = graph.get(specifier); + // follow the types reference directive, which may be pointing at an npm package + let maybe_module = match maybe_module { + Some(Module::Esm(module)) => { + let maybe_types_dep = module + .maybe_types_dependency + .as_ref() + .map(|d| &d.dependency); + match maybe_types_dep.and_then(|d| d.maybe_specifier()) { + Some(specifier) => graph.get(specifier), + _ => maybe_module, + } + } + maybe_module => maybe_module, + }; + + // now get the types from the resolved module + match maybe_module { + Some(Module::Esm(module)) => { + Ok(Some((module.specifier.clone(), module.media_type))) + } + Some(Module::Json(module)) => { + Ok(Some((module.specifier.clone(), module.media_type))) + } + Some(Module::Npm(module)) => { + if let Some(npm_resolver) = &state.maybe_npm_resolver { + resolve_npm_package_reference_types(&module.nv_reference, npm_resolver) + .map(Some) + } else { + Ok(None) + } + } + Some(Module::External(_) | Module::Node(_)) | None => Ok(None), + } +} + +fn resolve_non_graph_specifier_types( + specifier: &str, + referrer: &ModuleSpecifier, + state: &State, +) -> Result<Option<(ModuleSpecifier, MediaType)>, AnyError> { + let npm_resolver = match state.maybe_npm_resolver.as_ref() { + Some(npm_resolver) => npm_resolver, + None => return Ok(None), // we only support non-graph types for npm packages + }; + if npm_resolver.in_npm_package(referrer) { + // we're in an npm package, so use node resolution + Ok(Some(NodeResolution::into_specifier_and_media_type( + node::node_resolve( + specifier, + referrer, + NodeResolutionMode::Types, + npm_resolver, + &mut PermissionsContainer::allow_all(), + ) + .ok() + .flatten(), + ))) + } else if let Ok(npm_ref) = NpmPackageReqReference::from_str(specifier) { + // todo(dsherret): add support for injecting this in the graph so + // we don't need this special code here. + // This could occur when resolving npm:@types/node when it is + // injected and not part of the graph + let node_id = npm_resolver + .resolution() + .resolve_pkg_id_from_pkg_req(&npm_ref.req)?; + let npm_id_ref = NpmPackageNvReference { + nv: node_id.nv, + sub_path: npm_ref.sub_path, + }; + resolve_npm_package_reference_types(&npm_id_ref, npm_resolver).map(Some) + } else { + Ok(None) + } +} + pub fn resolve_npm_package_reference_types( - npm_ref: &NpmPackageReqReference, + npm_ref: &NpmPackageNvReference, npm_resolver: &NpmPackageResolver, ) -> Result<(ModuleSpecifier, MediaType), AnyError> { let maybe_resolution = node_resolve_npm_reference( diff --git a/cli/worker.rs b/cli/worker.rs index 9a89c8060..f8a9f54bb 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -448,8 +448,13 @@ async fn create_main_worker_internal( ps.npm_resolver .add_package_reqs(vec![package_ref.req.clone()]) .await?; + let pkg_nv = ps + .npm_resolver + .resolution() + .resolve_pkg_id_from_pkg_req(&package_ref.req)? + .nv; let node_resolution = node::node_resolve_binary_export( - &package_ref.req, + &pkg_nv, package_ref.sub_path.as_deref(), &ps.npm_resolver, &mut PermissionsContainer::allow_all(), diff --git a/ext/node/polyfill.rs b/ext/node/polyfill.rs index cedf1dd78..18faa6ea5 100644 --- a/ext/node/polyfill.rs +++ b/ext/node/polyfill.rs @@ -1,15 +1,15 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. pub fn find_builtin_node_module( - specifier: &str, + module_name: &str, ) -> Option<&NodeModulePolyfill> { SUPPORTED_BUILTIN_NODE_MODULES .iter() - .find(|m| m.name == specifier) + .find(|m| m.name == module_name) } -pub fn is_builtin_node_module(specifier: &str) -> bool { - find_builtin_node_module(specifier).is_some() +pub fn is_builtin_node_module(module_name: &str) -> bool { + find_builtin_node_module(module_name).is_some() } pub struct NodeModulePolyfill { |