refactor(bundle): remove usage of cli module loader (#30086)

Author: dsherret

Cargo.lock

@@ -251,7 +251,7 @@ simd-json = "0.14.0"
 slab = "0.4"
 smallvec = "1.8"
 socket2 = { version = "0.5.3", features = ["all"] }
-sys_traits = "=0.1.16"
+sys_traits = "=0.1.17"
 tar = "=0.4.43"
 tempfile = "3.4.0"
 termcolor = "1.1.3"

Cargo.toml

@@ -502,23 +502,6 @@ impl ModuleLoaderFactory for CliModuleLoaderFactory {
   }
 }
 
-impl CliModuleLoaderFactory {
-  pub fn create_cli_module_loader(
-    &self,
-    root_permissions: PermissionsContainer,
-  ) -> CliModuleLoader<MainModuleGraphContainer> {
-    CliModuleLoader(Rc::new(CliModuleLoaderInner {
-      lib: self.shared.lib_window,
-      is_worker: false,
-      parent_permissions: root_permissions.clone(),
-      permissions: root_permissions,
-      graph_container: (*self.shared.main_module_graph_container).clone(),
-      shared: self.shared.clone(),
-      loaded_files: Default::default(),
-    }))
-  }
-}
-
 struct ModuleCodeStringSource {
   pub code: ModuleSourceCode,
   pub found_url: ModuleSpecifier,
@@ -555,18 +538,13 @@ pub enum ResolveReferrerError {
 pub enum CliModuleLoaderError {
   #[class(inherit)]
   #[error(transparent)]
-  Fetch(deno_resolver::file_fetcher::FetchError),
+  Fetch(#[from] deno_resolver::file_fetcher::FetchError),
   #[class(inherit)]
   #[error(transparent)]
   LoadCodeSource(#[from] LoadCodeSourceError),
   #[class(inherit)]
   #[error(transparent)]
   LoadPreparedModule(#[from] LoadPreparedModuleError),
-  #[class(generic)]
-  #[error(
-    "Attempted to load JSON module without specifying \"type\": \"json\" attribute in the import statement."
-  )]
-  MissingJsonAttribute,
   #[class(inherit)]
   #[error(transparent)]
   PathToUrl(#[from] PathToUrlError),
@@ -578,50 +556,9 @@ pub enum CliModuleLoaderError {
   ResolveReferrer(#[from] ResolveReferrerError),
 }
 
-impl<TGraphContainer: ModuleGraphContainer> CliModuleLoader<TGraphContainer> {
-  pub async fn load_module_source(
-    &self,
-    specifier: &ModuleSpecifier,
-    maybe_referrer: Option<&ModuleSpecifier>,
-    requested_module_type: &RequestedModuleType,
-  ) -> Result<ModuleSource, CliModuleLoaderError> {
-    self
-      .0
-      .load_module_source(specifier, maybe_referrer, requested_module_type)
-      .await
-  }
-}
-
 impl<TGraphContainer: ModuleGraphContainer>
   CliModuleLoaderInner<TGraphContainer>
 {
-  async fn load_module_source(
-    &self,
-    specifier: &ModuleSpecifier,
-    maybe_referrer: Option<&ModuleSpecifier>,
-    requested_module_type: &RequestedModuleType,
-  ) -> Result<ModuleSource, CliModuleLoaderError> {
-    let code_source = self
-      .load_code_source(specifier, maybe_referrer, requested_module_type)
-      .await?;
-
-    // If we loaded a JSON file, but the "requested_module_type" (that is computed from
-    // import attributes) is not JSON we need to fail.
-    if code_source.module_type == ModuleType::Json
-      && *requested_module_type != RequestedModuleType::Json
-    {
-      return Err(CliModuleLoaderError::MissingJsonAttribute);
-    }
-
-    Ok(ModuleSource::new_with_redirect(
-      code_source.module_type,
-      code_source.code,
-      specifier,
-      &code_source.found_url,
-      None,
-    ))
-  }
-
   async fn load_inner(
     &self,
     specifier: &ModuleSpecifier,
@@ -633,13 +570,6 @@ impl<TGraphContainer: ModuleGraphContainer>
       .await
       .map_err(JsErrorBox::from_err)?;
 
-    // If we loaded a JSON file, but the "requested_module_type" (that is computed from
-    // import attributes) is not JSON we need to fail.
-    if code_source.module_type == ModuleType::Json
-      && *requested_module_type != RequestedModuleType::Json
-    {
-      return Err(JsErrorBox::generic("Attempted to load JSON module without specifying \"type\": \"json\" attribute in the import statement.").into());
-    }
     let code = if self.shared.is_inspecting
       || code_source.module_type == ModuleType::Wasm
     {
@@ -743,19 +673,20 @@ impl<TGraphContainer: ModuleGraphContainer>
         specifier,
         statically_analyzable,
       } => {
-        self
-          .load_asset(
-            &specifier,
-            if statically_analyzable {
-              CheckSpecifierKind::Static
-            } else {
-              // force permissions
-              CheckSpecifierKind::Dynamic
-            },
-            requested_module_type,
-          )
-          .await
-          .map_err(CliModuleLoaderError::Fetch)
+        Ok(
+          self
+            .load_asset(
+              &specifier,
+              if statically_analyzable {
+                CheckSpecifierKind::Static
+              } else {
+                // force permissions
+                CheckSpecifierKind::Dynamic
+              },
+              requested_module_type,
+            )
+            .await?,
+        )
       }
     }
   }

cli/module_loader.rs

@@ -17,7 +17,6 @@ use deno_ast::EmitOptions;
 use deno_ast::MediaType;
 use deno_ast::ModuleSpecifier;
 use deno_config::workspace::TsTypeLib;
-use deno_core::RequestedModuleType;
 use deno_core::error::AnyError;
 use deno_core::futures::FutureExt as _;
 use deno_core::resolve_url_or_path;
@@ -31,6 +30,9 @@ use deno_resolver::graph::ResolveWithGraphOptions;
 use deno_resolver::loader::LoadCodeSourceError;
 use deno_resolver::loader::LoadCodeSourceErrorKind;
 use deno_resolver::loader::LoadPreparedModuleErrorKind;
+use deno_resolver::loader::LoadedModuleOrAsset;
+use deno_resolver::loader::LoadedModuleSource;
+use deno_resolver::loader::RequestedModuleType;
 use deno_resolver::npm::managed::ResolvePkgFolderFromDenoModuleError;
 use deno_runtime::deno_permissions::PermissionsContainer;
 use deno_semver::npm::NpmPackageReqReference;
@@ -52,11 +54,11 @@ use crate::args::Flags;
 use crate::args::PackageHandling;
 use crate::args::SourceMapType;
 use crate::factory::CliFactory;
+use crate::file_fetcher::CliFileFetcher;
 use crate::graph_container::MainModuleGraphContainer;
 use crate::graph_container::ModuleGraphContainer;
 use crate::graph_container::ModuleGraphUpdatePermit;
-use crate::module_loader::CliModuleLoader;
-use crate::module_loader::CliModuleLoaderError;
+use crate::module_loader::CliDenoResolverModuleLoader;
 use crate::module_loader::ModuleLoadPreparer;
 use crate::module_loader::PrepareModuleLoadOptions;
 use crate::node::CliNodeResolver;
@@ -87,10 +89,7 @@ pub async fn bundle(
   let npm_resolver = factory.npm_resolver().await?;
   let node_resolver = factory.node_resolver().await?;
   let cli_options = factory.cli_options()?;
-  let module_loader = factory
-    .create_module_loader_factory()
-    .await?
-    .create_cli_module_loader(root_permissions.clone());
+  let module_loader = factory.resolver_factory()?.module_loader()?;
   let sys = factory.sys();
   let init_cwd = cli_options.initial_cwd().to_path_buf();
   let module_graph_container =
@@ -99,6 +98,7 @@ pub async fn bundle(
   let (on_end_tx, on_end_rx) = tokio::sync::mpsc::channel(10);
   #[allow(clippy::arc_with_non_send_sync)]
   let plugin_handler = Arc::new(DenoPluginHandler {
+    file_fetcher: factory.file_fetcher()?.clone(),
     resolver: resolver.clone(),
     module_load_preparer,
     module_graph_container,
@@ -470,23 +470,24 @@ enum BundleError {
 
 fn requested_type_from_map(
   map: &IndexMap<String, String>,
-) -> RequestedModuleType {
+) -> RequestedModuleType<'_> {
   let type_ = map.get("type").map(|s| s.as_str());
   match type_ {
     Some("json") => RequestedModuleType::Json,
     Some("bytes") => RequestedModuleType::Bytes,
     Some("text") => RequestedModuleType::Text,
-    Some(other) => RequestedModuleType::Other(other.to_string().into()),
+    Some(other) => RequestedModuleType::Other(other),
     None => RequestedModuleType::None,
   }
 }
 
 pub struct DenoPluginHandler {
+  file_fetcher: Arc<CliFileFetcher>,
   resolver: Arc<CliResolver>,
   module_load_preparer: Arc<ModuleLoadPreparer>,
   module_graph_container: Arc<MainModuleGraphContainer>,
   permissions: PermissionsContainer,
-  module_loader: CliModuleLoader<MainModuleGraphContainer>,
+  module_loader: Arc<CliDenoResolverModuleLoader>,
   externals_matcher: Option<ExternalsMatcher>,
   on_end_tx: tokio::sync::mpsc::Sender<esbuild_client::OnEndArgs>,
 }
@@ -642,7 +643,10 @@ fn import_kind_to_resolution_mode(
 pub enum BundleLoadError {
   #[class(inherit)]
   #[error(transparent)]
-  CliModuleLoader(#[from] CliModuleLoaderError),
+  Fetch(#[from] deno_resolver::file_fetcher::FetchError),
+  #[class(inherit)]
+  #[error(transparent)]
+  LoadCodeSource(#[from] LoadCodeSourceError),
   #[class(inherit)]
   #[error(transparent)]
   ResolveUrlOrPath(#[from] deno_path_util::ResolveUrlOrPathError),
@@ -666,9 +670,7 @@ pub enum BundleLoadError {
 impl BundleLoadError {
   pub fn is_unsupported_media_type(&self) -> bool {
     match self {
-      BundleLoadError::CliModuleLoader(
-        CliModuleLoaderError::LoadCodeSource(LoadCodeSourceError(e)),
-      ) => match &**e {
+      BundleLoadError::LoadCodeSource(e) => match e.as_kind() {
         LoadCodeSourceErrorKind::LoadPreparedModule(e) => match e.as_kind() {
           LoadPreparedModuleErrorKind::Graph(e) => matches!(
             e.error.as_kind(),
@@ -808,7 +810,7 @@ impl DenoPluginHandler {
   async fn bundle_load(
     &self,
     specifier: &str,
-    requested_type: &RequestedModuleType,
+    requested_type: &RequestedModuleType<'_>,
   ) -> Result<Option<(Vec<u8>, esbuild_client::BuiltinLoader)>, BundleLoadError>
   {
     log::debug!(
@@ -862,10 +864,24 @@ impl DenoPluginHandler {
         }
         (specifier, media_type, media_type_to_loader(media_type))
       };
-    let loaded = self
+    let graph = self.module_graph_container.graph();
+    let module_or_asset = self
       .module_loader
-      .load_module_source(&specifier, None, requested_type)
+      .load(&graph, &specifier, None, requested_type)
       .await?;
+    let loaded_code = match module_or_asset {
+      LoadedModuleOrAsset::Module(loaded_module) => loaded_module.source,
+      LoadedModuleOrAsset::ExternalAsset {
+        specifier,
+        statically_analyzable: _,
+      } => LoadedModuleSource::ArcBytes(
+        self
+          .file_fetcher
+          .fetch(&specifier, &self.permissions)
+          .await?
+          .source,
+      ),
+    };
 
     if matches!(
       media_type,
@@ -877,20 +893,16 @@ impl DenoPluginHandler {
         | MediaType::Cts
         | MediaType::Jsx
         | MediaType::Tsx
-    ) && !self
-      .module_graph_container
-      .graph()
-      .roots
-      .contains(&specifier)
+    ) && !graph.roots.contains(&specifier)
     {
       let code = self.apply_transform(
         &specifier,
         media_type,
-        std::str::from_utf8(loaded.code.as_bytes())?,
+        std::str::from_utf8(loaded_code.as_bytes())?,
       )?;
       Ok(Some((code.into_bytes(), loader)))
     } else {
-      Ok(Some((loaded.code.as_bytes().to_vec(), loader)))
+      Ok(Some((loaded_code.as_bytes().to_vec(), loader)))
     }
   }
 

cli/tools/bundle/mod.rs

@@ -13,6 +13,7 @@ pub use module_loader::*;
 pub use npm::*;
 use url::Url;
 
+#[derive(Debug)]
 pub enum RequestedModuleType<'a> {
   None,
   Json,