From 8c1677ecbcbb474fc6a5ac9b5f73b562677bb829 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 3 Oct 2023 19:05:06 -0400 Subject: refactor(npm): break up `NpmModuleLoader` and move more methods into the managed `CliNpmResolver` (#20777) Part of https://github.com/denoland/deno/issues/18967 --- cli/module_loader.rs | 158 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 99 insertions(+), 59 deletions(-) (limited to 'cli/module_loader.rs') diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 4a694e615..51b9ea598 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -50,10 +50,10 @@ use deno_runtime::deno_node::NodeResolution; use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::deno_node::NodeResolver; use deno_runtime::permissions::PermissionsContainer; -use deno_semver::npm::NpmPackageNvReference; use deno_semver::npm::NpmPackageReqReference; use std::borrow::Cow; use std::collections::HashSet; +use std::path::Path; use std::pin::Pin; use std::rc::Rc; use std::str; @@ -309,6 +309,7 @@ struct SharedCliModuleLoaderState { module_load_preparer: Arc, prepared_module_loader: PreparedModuleLoader, resolver: Arc, + node_resolver: Arc, npm_module_loader: NpmModuleLoader, } @@ -317,6 +318,7 @@ pub struct CliModuleLoaderFactory { } impl CliModuleLoaderFactory { + #[allow(clippy::too_many_arguments)] pub fn new( options: &CliOptions, emitter: Arc, @@ -324,6 +326,7 @@ impl CliModuleLoaderFactory { module_load_preparer: Arc, parsed_source_cache: Arc, resolver: Arc, + node_resolver: Arc, npm_module_loader: NpmModuleLoader, ) -> Self { Self { @@ -343,6 +346,7 @@ impl CliModuleLoaderFactory { graph_container, module_load_preparer, resolver, + node_resolver, npm_module_loader, }), } @@ -471,11 +475,11 @@ impl ModuleLoader for CliModuleLoader { let referrer_result = deno_core::resolve_url_or_path(referrer, &cwd); if let Ok(referrer) = referrer_result.as_ref() { - if let Some(result) = self - .shared - .npm_module_loader - .resolve_if_in_npm_package(specifier, referrer, permissions) - { + if let Some(result) = self.shared.node_resolver.resolve_if_in_npm_package( + specifier, + referrer, + permissions, + ) { return result; } @@ -492,10 +496,28 @@ impl ModuleLoader for CliModuleLoader { let specifier = &resolved.specifier; return match graph.get(specifier) { - Some(Module::Npm(module)) => self - .shared - .npm_module_loader - .resolve_nv_ref(&module.nv_reference, permissions), + Some(Module::Npm(module)) => { + let package_folder = self + .shared + .node_resolver + .npm_resolver + .as_managed() + .unwrap() // byonm won't create a Module::Npm + .resolve_pkg_folder_from_deno_module( + module.nv_reference.nv(), + )?; + self + .shared + .node_resolver + .resolve_package_sub_path( + &package_folder, + module.nv_reference.sub_path(), + permissions, + ) + .with_context(|| { + format!("Could not resolve '{}'.", module.nv_reference) + }) + } Some(Module::Node(module)) => Ok(module.specifier.clone()), Some(Module::Esm(module)) => Ok(module.specifier.clone()), Some(Module::Json(module)) => Ok(module.specifier.clone()), @@ -538,10 +560,11 @@ impl ModuleLoader for CliModuleLoader { if let Ok(reference) = NpmPackageReqReference::from_specifier(&specifier) { - return self - .shared - .npm_module_loader - .resolve_req_reference(&reference, permissions); + return self.shared.node_resolver.resolve_req_reference( + &reference, + permissions, + &referrer, + ); } } } @@ -642,38 +665,36 @@ impl SourceMapGetter for CliSourceMapGetter { } } -pub struct NpmModuleLoader { +pub struct CliNodeResolver { cjs_resolutions: Arc, - node_code_translator: Arc, - fs: Arc, node_resolver: Arc, npm_resolver: Arc, } -impl NpmModuleLoader { +impl CliNodeResolver { pub fn new( cjs_resolutions: Arc, - node_code_translator: Arc, - fs: Arc, node_resolver: Arc, npm_resolver: Arc, ) -> Self { Self { cjs_resolutions, - node_code_translator, - fs, node_resolver, npm_resolver, } } + pub fn in_npm_package(&self, referrer: &ModuleSpecifier) -> bool { + self.npm_resolver.in_npm_package(referrer) + } + pub fn resolve_if_in_npm_package( &self, specifier: &str, referrer: &ModuleSpecifier, permissions: &PermissionsContainer, ) -> Option> { - if self.node_resolver.in_npm_package(referrer) { + if self.in_npm_package(referrer) { // we're in an npm package, so use node resolution Some( self @@ -692,40 +713,74 @@ impl NpmModuleLoader { } } - pub fn resolve_nv_ref( + pub fn resolve_req_reference( &self, - nv_ref: &NpmPackageNvReference, + req_ref: &NpmPackageReqReference, permissions: &PermissionsContainer, + referrer: &ModuleSpecifier, ) -> Result { let package_folder = self .npm_resolver - .resolve_pkg_folder_from_deno_module(nv_ref.nv())?; + .resolve_pkg_folder_from_deno_module_req(req_ref.req(), referrer)?; self - .handle_node_resolve_result(self.node_resolver.resolve_npm_reference( + .resolve_package_sub_path( &package_folder, - nv_ref.sub_path(), - NodeResolutionMode::Execution, + req_ref.sub_path(), permissions, - )) - .with_context(|| format!("Could not resolve '{}'.", nv_ref)) + ) + .with_context(|| format!("Could not resolve '{}'.", req_ref)) } - pub fn resolve_req_reference( + fn resolve_package_sub_path( &self, - req_ref: &NpmPackageReqReference, + package_folder: &Path, + sub_path: Option<&str>, permissions: &PermissionsContainer, ) -> Result { - let package_folder = self - .npm_resolver - .resolve_pkg_folder_from_deno_module_req(req_ref.req())?; - self - .handle_node_resolve_result(self.node_resolver.resolve_npm_reference( - &package_folder, - req_ref.sub_path(), - NodeResolutionMode::Execution, - permissions, - )) - .with_context(|| format!("Could not resolve '{}'.", req_ref)) + self.handle_node_resolve_result(self.node_resolver.resolve_npm_reference( + package_folder, + sub_path, + NodeResolutionMode::Execution, + permissions, + )) + } + + fn handle_node_resolve_result( + &self, + result: Result, AnyError>, + ) -> Result { + let response = match result? { + Some(response) => response, + None => return Err(generic_error("not found")), + }; + if let NodeResolution::CommonJs(specifier) = &response { + // remember that this was a common js resolution + self.cjs_resolutions.insert(specifier.clone()); + } + Ok(response.into_url()) + } +} + +pub struct NpmModuleLoader { + cjs_resolutions: Arc, + node_code_translator: Arc, + fs: Arc, + node_resolver: Arc, +} + +impl NpmModuleLoader { + pub fn new( + cjs_resolutions: Arc, + node_code_translator: Arc, + fs: Arc, + node_resolver: Arc, + ) -> Self { + Self { + cjs_resolutions, + node_code_translator, + fs, + node_resolver, + } } pub fn maybe_prepare_load( @@ -812,21 +867,6 @@ impl NpmModuleLoader { media_type: MediaType::from_specifier(specifier), }) } - - fn handle_node_resolve_result( - &self, - result: Result, AnyError>, - ) -> Result { - let response = match result? { - Some(response) => response, - None => return Err(generic_error("not found")), - }; - if let NodeResolution::CommonJs(specifier) = &response { - // remember that this was a common js resolution - self.cjs_resolutions.insert(specifier.clone()); - } - Ok(response.into_url()) - } } /// Keeps track of what module specifiers were resolved as CJS. -- cgit v1.2.3