refactor(cli): extract out NpmModuleLoader from CliModuleLoader (#18842)

Need to share this with the loader used in deno compile

cli/module_loader.rs

@@ -222,7 +222,7 @@ impl ModuleLoadPreparer {
   }
 }
 
-struct ModuleCodeSource {
+pub struct ModuleCodeSource {
   pub code: ModuleCode,
   pub found_url: ModuleSpecifier,
   pub media_type: MediaType,
@@ -238,14 +238,12 @@ pub struct CliModuleLoader {
   /// "root permissions" for Web Worker.
   dynamic_permissions: PermissionsContainer,
   cli_options: Arc<CliOptions>,
-  cjs_resolutions: Arc<CjsResolutionStore>,
   emitter: Arc<Emitter>,
   graph_container: Arc<ModuleGraphContainer>,
   module_load_preparer: Arc<ModuleLoadPreparer>,
-  node_code_translator: Arc<CliNodeCodeTranslator>,
-  node_resolver: Arc<NodeResolver>,
   parsed_source_cache: Arc<ParsedSourceCache>,
   resolver: Arc<CliGraphResolver>,
+  npm_module_loader: NpmModuleLoader,
 }
 
 impl CliModuleLoader {
@@ -259,14 +257,16 @@ impl CliModuleLoader {
       root_permissions,
       dynamic_permissions,
       cli_options: ps.options.clone(),
-      cjs_resolutions: ps.cjs_resolutions.clone(),
       emitter: ps.emitter.clone(),
       graph_container: ps.graph_container.clone(),
       module_load_preparer: ps.module_load_preparer.clone(),
-      node_code_translator: ps.node_code_translator.clone(),
-      node_resolver: ps.node_resolver.clone(),
       parsed_source_cache: ps.parsed_source_cache.clone(),
       resolver: ps.resolver.clone(),
+      npm_module_loader: NpmModuleLoader::new(
+        ps.cjs_resolutions.clone(),
+        ps.node_code_translator.clone(),
+        ps.node_resolver.clone(),
+      ),
     })
   }
 
@@ -280,14 +280,16 @@ impl CliModuleLoader {
       root_permissions,
       dynamic_permissions,
       cli_options: ps.options.clone(),
-      cjs_resolutions: ps.cjs_resolutions.clone(),
       emitter: ps.emitter.clone(),
       graph_container: ps.graph_container.clone(),
       module_load_preparer: ps.module_load_preparer.clone(),
-      node_code_translator: ps.node_code_translator.clone(),
-      node_resolver: ps.node_resolver.clone(),
       parsed_source_cache: ps.parsed_source_cache.clone(),
       resolver: ps.resolver.clone(),
+      npm_module_loader: NpmModuleLoader::new(
+        ps.cjs_resolutions.clone(),
+        ps.node_code_translator.clone(),
+        ps.node_resolver.clone(),
+      ),
     })
   }
 
@@ -367,41 +369,16 @@ impl CliModuleLoader {
     maybe_referrer: Option<&ModuleSpecifier>,
     is_dynamic: bool,
   ) -> Result<ModuleSource, AnyError> {
-    let code_source = if self.node_resolver.in_npm_package(specifier) {
+    let permissions = if is_dynamic {
-      let file_path = specifier.to_file_path().unwrap();
+      &self.dynamic_permissions
-      let code = std::fs::read_to_string(&file_path).with_context(|| {
+    } else {
-        let mut msg = "Unable to load ".to_string();
+      &self.root_permissions
-        msg.push_str(&file_path.to_string_lossy());
+    };
-        if let Some(referrer) = &maybe_referrer {
+    let code_source = if let Some(code_source) = self
-          msg.push_str(" imported from ");
+      .npm_module_loader
-          msg.push_str(referrer.as_str());
+      .load_sync(specifier, maybe_referrer, permissions)?
-        }
+    {
-        msg
+      code_source
-      })?;
-
-      let code = if self.cjs_resolutions.contains(specifier) {
-        let mut permissions = if is_dynamic {
-          self.dynamic_permissions.clone()
-        } else {
-          self.root_permissions.clone()
-        };
-        // translate cjs to esm if it's cjs and inject node globals
-        self.node_code_translator.translate_cjs_to_esm(
-          specifier,
-          &code,
-          &mut permissions,
-        )?
-      } else {
-        // only inject node globals for esm
-        self
-          .node_code_translator
-          .esm_code_with_node_globals(specifier, &code)?
-      };
-      ModuleCodeSource {
-        code: code.into(),
-        found_url: specifier.clone(),
-        media_type: MediaType::from_specifier(specifier),
-      }
     } else {
       self.load_prepared_module(specifier, maybe_referrer)?
     };
@@ -424,23 +401,6 @@ impl CliModuleLoader {
       &code_source.found_url,
     ))
   }
-
-  fn handle_node_resolve_result(
-    &self,
-    result: Result<Option<NodeResolution>, AnyError>,
-  ) -> Result<ModuleSpecifier, AnyError> {
-    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());
-    } else if let NodeResolution::BuiltIn(specifier) = &response {
-      return deno_node::resolve_builtin_node_module(specifier);
-    }
-    Ok(response.into_url())
-  }
 }
 
 impl ModuleLoader for CliModuleLoader {
@@ -462,18 +422,12 @@ impl ModuleLoader for CliModuleLoader {
     let referrer_result = deno_core::resolve_url_or_path(referrer, &cwd);
 
     if let Ok(referrer) = referrer_result.as_ref() {
-      if self.node_resolver.in_npm_package(referrer) {
+      if let Some(result) = self.npm_module_loader.resolve_if_in_npm_package(
-        // we're in an npm package, so use node resolution
+        specifier,
-        return self
+        referrer,
-          .handle_node_resolve_result(self.node_resolver.resolve(
+        permissions,
-            specifier,
+      ) {
-            referrer,
+        return result;
-            NodeResolutionMode::Execution,
-            permissions,
-          ))
-          .with_context(|| {
-            format!("Could not resolve '{specifier}' from '{referrer}'.")
-          });
       }
 
       let graph = self.graph_container.graph();
@@ -490,16 +444,8 @@ impl ModuleLoader for CliModuleLoader {
 
           return match graph.get(specifier) {
             Some(Module::Npm(module)) => self
-              .handle_node_resolve_result(
+              .npm_module_loader
-                self.node_resolver.resolve_npm_reference(
+              .resolve_npm_module(module, permissions),
-                  &module.nv_reference,
-                  NodeResolutionMode::Execution,
-                  permissions,
-                ),
-              )
-              .with_context(|| {
-                format!("Could not resolve '{}'.", module.nv_reference)
-              }),
             Some(Module::Node(module)) => {
               deno_node::resolve_builtin_node_module(&module.module_name)
             }
@@ -552,14 +498,8 @@ impl ModuleLoader for CliModuleLoader {
           NpmPackageReqReference::from_specifier(&specifier)
         {
           return self
-            .handle_node_resolve_result(
+            .npm_module_loader
-              self.node_resolver.resolve_npm_req_reference(
+            .resolve_for_repl(&reference, permissions);
-                &reference,
-                NodeResolutionMode::Execution,
-                permissions,
-              ),
-            )
-            .with_context(|| format!("Could not resolve '{reference}'."));
         }
       }
     }
@@ -590,9 +530,8 @@ impl ModuleLoader for CliModuleLoader {
     _maybe_referrer: Option<String>,
     is_dynamic: bool,
   ) -> Pin<Box<dyn Future<Output = Result<(), AnyError>>>> {
-    if self.node_resolver.in_npm_package(specifier) {
+    if let Some(result) = self.npm_module_loader.maybe_prepare_load(specifier) {
-      // nothing to prepare
+      return Box::pin(deno_core::futures::future::ready(result));
-      return Box::pin(deno_core::futures::future::ready(Ok(())));
     }
 
     let specifier = specifier.clone();
@@ -658,3 +597,145 @@ impl SourceMapGetter for CliModuleLoader {
     }
   }
 }
+
+pub struct NpmModuleLoader {
+  cjs_resolutions: Arc<CjsResolutionStore>,
+  node_code_translator: Arc<CliNodeCodeTranslator>,
+  node_resolver: Arc<NodeResolver>,
+}
+
+impl NpmModuleLoader {
+  pub fn new(
+    cjs_resolutions: Arc<CjsResolutionStore>,
+    node_code_translator: Arc<CliNodeCodeTranslator>,
+    node_resolver: Arc<NodeResolver>,
+  ) -> Self {
+    Self {
+      cjs_resolutions,
+      node_code_translator,
+      node_resolver,
+    }
+  }
+
+  pub fn resolve_if_in_npm_package(
+    &self,
+    specifier: &str,
+    referrer: &ModuleSpecifier,
+    permissions: &PermissionsContainer,
+  ) -> Option<Result<ModuleSpecifier, AnyError>> {
+    if self.node_resolver.in_npm_package(referrer) {
+      // we're in an npm package, so use node resolution
+      Some(
+        self
+          .handle_node_resolve_result(self.node_resolver.resolve(
+            specifier,
+            referrer,
+            NodeResolutionMode::Execution,
+            permissions,
+          ))
+          .with_context(|| {
+            format!("Could not resolve '{specifier}' from '{referrer}'.")
+          }),
+      )
+    } else {
+      None
+    }
+  }
+
+  pub fn resolve_npm_module(
+    &self,
+    module: &deno_graph::NpmModule,
+    permissions: &PermissionsContainer,
+  ) -> Result<ModuleSpecifier, AnyError> {
+    self
+      .handle_node_resolve_result(self.node_resolver.resolve_npm_reference(
+        &module.nv_reference,
+        NodeResolutionMode::Execution,
+        permissions,
+      ))
+      .with_context(|| format!("Could not resolve '{}'.", module.nv_reference))
+  }
+
+  pub fn resolve_for_repl(
+    &self,
+    reference: &NpmPackageReqReference,
+    permissions: &PermissionsContainer,
+  ) -> Result<ModuleSpecifier, AnyError> {
+    self
+      .handle_node_resolve_result(self.node_resolver.resolve_npm_req_reference(
+        reference,
+        NodeResolutionMode::Execution,
+        permissions,
+      ))
+      .with_context(|| format!("Could not resolve '{reference}'."))
+  }
+
+  pub fn maybe_prepare_load(
+    &self,
+    specifier: &ModuleSpecifier,
+  ) -> Option<Result<(), AnyError>> {
+    if self.node_resolver.in_npm_package(specifier) {
+      // nothing to prepare
+      Some(Ok(()))
+    } else {
+      None
+    }
+  }
+
+  pub fn load_sync(
+    &self,
+    specifier: &ModuleSpecifier,
+    maybe_referrer: Option<&ModuleSpecifier>,
+    permissions: &PermissionsContainer,
+  ) -> Result<Option<ModuleCodeSource>, AnyError> {
+    if !self.node_resolver.in_npm_package(specifier) {
+      return Ok(None);
+    }
+    let file_path = specifier.to_file_path().unwrap();
+    let code = std::fs::read_to_string(&file_path).with_context(|| {
+      let mut msg = "Unable to load ".to_string();
+      msg.push_str(&file_path.to_string_lossy());
+      if let Some(referrer) = &maybe_referrer {
+        msg.push_str(" imported from ");
+        msg.push_str(referrer.as_str());
+      }
+      msg
+    })?;
+
+    let code = if self.cjs_resolutions.contains(specifier) {
+      // translate cjs to esm if it's cjs and inject node globals
+      self.node_code_translator.translate_cjs_to_esm(
+        specifier,
+        &code,
+        permissions,
+      )?
+    } else {
+      // only inject node globals for esm
+      self
+        .node_code_translator
+        .esm_code_with_node_globals(specifier, &code)?
+    };
+    Ok(Some(ModuleCodeSource {
+      code: code.into(),
+      found_url: specifier.clone(),
+      media_type: MediaType::from_specifier(specifier),
+    }))
+  }
+
+  fn handle_node_resolve_result(
+    &self,
+    result: Result<Option<NodeResolution>, AnyError>,
+  ) -> Result<ModuleSpecifier, AnyError> {
+    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());
+    } else if let NodeResolution::BuiltIn(specifier) = &response {
+      return deno_node::resolve_builtin_node_module(specifier);
+    }
+    Ok(response.into_url())
+  }
+}

ext/node/analyze.rs

@@ -113,7 +113,7 @@ impl<TCjsEsmCodeAnalyzer: CjsEsmCodeAnalyzer>
     &self,
     specifier: &ModuleSpecifier,
     source: &str,
-    permissions: &mut dyn NodePermissions,
+    permissions: &dyn NodePermissions,
   ) -> Result<String, AnyError> {
     let mut temp_var_count = 0;
     let mut handled_reexports: HashSet<String> = HashSet::default();
@@ -220,7 +220,7 @@ impl<TCjsEsmCodeAnalyzer: CjsEsmCodeAnalyzer>
     referrer: &ModuleSpecifier,
     conditions: &[&str],
     mode: NodeResolutionMode,
-    permissions: &mut dyn NodePermissions,
+    permissions: &dyn NodePermissions,
   ) -> Result<PathBuf, AnyError> {
     if specifier.starts_with('/') {
       todo!();