More maintainable

denoland · dsherret · Dec 7, 2023 · Nov 28, 2023 · Dec 4, 2023 · Dec 4, 2023
commit 56ec8d4e71c7f17cf7cfcd06bbdebf6068374240
diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs
@@ -4,8 +4,8 @@ use crate::args::CacheSetting;
 use crate::errors::get_error_class_name;
 use crate::file_fetcher::FetchOptions;
 use crate::file_fetcher::FileFetcher;
-use crate::graph_util::js_to_ts_specifier;
 use crate::npm::CliNpmResolver;
+use crate::resolver::UnstableLooseImportsResolver;
 use crate::util::fs::atomic_write_file;
 
 use deno_ast::MediaType;
@@ -99,11 +99,6 @@ pub type LocalLspHttpCache =
 pub use deno_cache_dir::CachedUrlMetadata;
 pub use deno_cache_dir::HttpCache;
 
-pub struct FetchCacherOptions {
-  pub file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
-  pub unstable_loose_imports: bool,
-}
-
 /// A "wrapper" for the FileFetcher and DiskCache for the Deno CLI that provides
 /// a concise interface to the DENO_DIR when building module graphs.
 pub struct FetchCacher {
@@ -115,29 +110,30 @@ pub struct FetchCacher {
   module_info_cache: Arc<ModuleInfoCache>,
   permissions: PermissionsContainer,
   cache_info_enabled: bool,
-  unstable_loose_imports: bool,
+  loose_imports_resolver: Option<UnstableLooseImportsResolver>,
 }
 
 impl FetchCacher {
   pub fn new(
     emit_cache: EmitCache,
     file_fetcher: Arc<FileFetcher>,
+    file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
     global_http_cache: Arc<GlobalHttpCache>,
     npm_resolver: Arc<dyn CliNpmResolver>,
     module_info_cache: Arc<ModuleInfoCache>,
     permissions: PermissionsContainer,
-    options: FetchCacherOptions,
+    loose_imports_resolver: Option<UnstableLooseImportsResolver>,
   ) -> Self {
     Self {
       emit_cache,
       file_fetcher,
-      file_header_overrides: options.file_header_overrides,
+      file_header_overrides,
       global_http_cache,
       npm_resolver,
       module_info_cache,
       permissions,
       cache_info_enabled: false,
-      unstable_loose_imports: options.unstable_loose_imports,
+      loose_imports_resolver,
     }
   }
 
@@ -226,89 +222,6 @@ impl Loader for FetchCacher {
   ) -> LoadFuture {
     use deno_graph::source::CacheSetting as LoaderCacheSetting;
 
-    fn do_load(
-      file_fetcher: Arc<FileFetcher>,
-      file_header_overrides: HashMap<ModuleSpecifier, HashMap<String, String>>,
-      permissions: PermissionsContainer,
-      specifier: ModuleSpecifier,
-      cache_setting: deno_graph::source::CacheSetting,
-      unstable_loose_imports: bool,
-    ) -> LoadFuture {
-      async move {
-        let maybe_cache_setting = match cache_setting {
-          LoaderCacheSetting::Use => None,
-          LoaderCacheSetting::Reload => {
-            if matches!(file_fetcher.cache_setting(), CacheSetting::Only) {
-              return Err(deno_core::anyhow::anyhow!(
-                "Failed to resolve version constraint. Try running again without --cached-only"
-              ));
-            }
-            Some(CacheSetting::ReloadAll)
-          }
-          LoaderCacheSetting::Only => Some(CacheSetting::Only),
-        };
-        let result = file_fetcher
-          .fetch_with_options(FetchOptions {
-            specifier: &specifier,
-            permissions: permissions.clone(),
-            maybe_accept: None,
-            maybe_cache_setting: maybe_cache_setting.as_ref(),
-          })
-          .await
-          .map(|file| {
-            let maybe_headers =
-              match (file.maybe_headers, file_header_overrides.get(&specifier)) {
-                (Some(headers), Some(overrides)) => {
-                  Some(headers.into_iter().chain(overrides.clone()).collect())
-                }
-                (Some(headers), None) => Some(headers),
-                (None, Some(overrides)) => Some(overrides.clone()),
-                (None, None) => None,
-              };
-            Ok(Some(LoadResponse::Module {
-              specifier: file.specifier,
-              maybe_headers,
-              content: file.source,
-            }))
-          })
-          .unwrap_or_else(|err| {
-            if let Some(io_err) = err.downcast_ref::<std::io::Error>() {
-              if io_err.kind() == std::io::ErrorKind::NotFound {
-                return Ok(None);
-              } else {
-                return Err(err);
-              }
-            }
-            let error_class_name = get_error_class_name(&err);
-            match error_class_name {
-              "NotFound" => Ok(None),
-              "NotCached" if cache_setting == LoaderCacheSetting::Only => {
-                Ok(None)
-              }
-              _ => Err(err),
-            }
-          });
-
-        if unstable_loose_imports && matches!(result, Ok(None)) && specifier.scheme() == "file" {
-          if let Some(ts_specifier) = js_to_ts_specifier(&specifier) {
-            let result = do_load(
-              file_fetcher,
-              file_header_overrides,
-              permissions,
-              ts_specifier,
-              cache_setting,
-              unstable_loose_imports,
-            ).await;
-            if let Ok(Some(response)) = result {
-              return Ok(Some(response));
-            }
-          }
-        }
-
-        result
-      }.boxed_local()
-    }
-
     if specifier.path().contains("/node_modules/") {
       // The specifier might be in a completely different symlinked tree than
       // what the node_modules url is in (ex. `/my-project-1/node_modules`
@@ -327,16 +240,65 @@ impl Loader for FetchCacher {
     let file_fetcher = self.file_fetcher.clone();
     let file_header_overrides = self.file_header_overrides.clone();
     let permissions = self.permissions.clone();
-    let specifier = specifier.clone();
+    let specifier = match &self.loose_imports_resolver {
+      Some(resolver) => resolver.resolve(specifier).into_owned(),
+      None => specifier.clone(),
+    };
 
-    do_load(
-      file_fetcher,
-      file_header_overrides,
-      permissions,
-      specifier,
-      cache_setting,
-      self.unstable_loose_imports,
-    )
+    async move {
+      let maybe_cache_setting = match cache_setting {
+        LoaderCacheSetting::Use => None,
+        LoaderCacheSetting::Reload => {
+          if matches!(file_fetcher.cache_setting(), CacheSetting::Only) {
+            return Err(deno_core::anyhow::anyhow!(
+              "Failed to resolve version constraint. Try running again without --cached-only"
+            ));
+          }
+          Some(CacheSetting::ReloadAll)
+        }
+        LoaderCacheSetting::Only => Some(CacheSetting::Only),
+      };
+      file_fetcher
+        .fetch_with_options(FetchOptions {
+          specifier: &specifier,
+          permissions,
+          maybe_accept: None,
+          maybe_cache_setting: maybe_cache_setting.as_ref(),
+        })
+        .await
+        .map(|file| {
+          let maybe_headers =
+            match (file.maybe_headers, file_header_overrides.get(&specifier)) {
+              (Some(headers), Some(overrides)) => {
+                Some(headers.into_iter().chain(overrides.clone()).collect())
+              }
+              (Some(headers), None) => Some(headers),
+              (None, Some(overrides)) => Some(overrides.clone()),
+              (None, None) => None,
+            };
+          Ok(Some(LoadResponse::Module {
+            specifier: file.specifier,
+            maybe_headers,
+            content: file.source,
+          }))
+        })
+        .unwrap_or_else(|err| {
+          if let Some(io_err) = err.downcast_ref::<std::io::Error>() {
+            if io_err.kind() == std::io::ErrorKind::NotFound {
+              return Ok(None);
+            } else {
+              return Err(err);
+            }
+          }
+          let error_class_name = get_error_class_name(&err);
+          match error_class_name {
+            "NotFound" => Ok(None),
+            "NotCached" if cache_setting == LoaderCacheSetting::Only => Ok(None),
+            _ => Err(err),
+          }
+        })
+    }
+    .boxed()
   }
 
   fn cache_module_info(

diff --git a/cli/graph_util.rs b/cli/graph_util.rs
@@ -12,13 +12,13 @@ use crate::errors::get_error_class_name;
 use crate::file_fetcher::FileFetcher;
 use crate::npm::CliNpmResolver;
 use crate::resolver::CliGraphResolver;
+use crate::resolver::UnstableLooseImportsResolver;
 use crate::tools::check;
 use crate::tools::check::TypeChecker;
 use crate::util::file_watcher::WatcherCommunicator;
 use crate::util::sync::TaskQueue;
 use crate::util::sync::TaskQueuePermit;
 
-use deno_ast::MediaType;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::custom_error;
@@ -486,13 +486,15 @@ impl ModuleGraphBuilder {
     cache::FetchCacher::new(
       self.emit_cache.clone(),
       self.file_fetcher.clone(),
+      self.options.resolve_file_header_overrides(),
       self.global_http_cache.clone(),
       self.npm_resolver.clone(),
       self.module_info_cache.clone(),
       permissions,
-      cache::FetchCacherOptions {
-        file_header_overrides: self.options.resolve_file_header_overrides(),
-        unstable_loose_imports: self.options.unstable_loose_imports(),
+      if self.options.unstable_loose_imports() {
+        Some(UnstableLooseImportsResolver::new(self.fs.clone()))
+      } else {
+        None
       },
     )
   }
@@ -850,47 +852,6 @@ pub fn format_range_with_colors(range: &deno_graph::Range) -> String {
   )
 }
 
-/// Converts a `.js` specifier to the corresponding `.ts` specifier.
-///
-/// Example:
-///   file:///mod.js -> file:///mod.ts
-///   file:///mod.jsx -> file:///mod.tsx
-pub fn js_to_ts_specifier(
-  specifier: &ModuleSpecifier,
-) -> Option<ModuleSpecifier> {
-  // doesn't make sense otherwise
-  debug_assert_eq!(specifier.scheme(), "file");
-
-  let media_type = MediaType::from_specifier(&specifier);
-  let new_media_type = match media_type {
-    MediaType::JavaScript => MediaType::TypeScript,
-    MediaType::Jsx => MediaType::Tsx,
-    MediaType::Mjs => MediaType::Mts,
-    MediaType::Cjs => MediaType::Cts,
-    MediaType::TypeScript
-    | MediaType::Mts
-    | MediaType::Cts
-    | MediaType::Dts
-    | MediaType::Dmts
-    | MediaType::Dcts
-    | MediaType::Tsx
-    | MediaType::Json
-    | MediaType::Wasm
-    | MediaType::TsBuildInfo
-    | MediaType::SourceMap
-    | MediaType::Unknown => return None,
-  };
-  let old_specifier = specifier
-    .as_str()
-    .strip_suffix(media_type.as_ts_extension())?;
-  ModuleSpecifier::parse(&format!(
-    "{}{}",
-    old_specifier,
-    new_media_type.as_ts_extension()
-  ))
-  .ok()
-}
-
 #[cfg(test)]
 mod test {
   use std::sync::Arc;
@@ -942,28 +903,4 @@ mod test {
       assert_eq!(get_resolution_error_bare_node_specifier(&err), output,);
     }
   }
-
-  #[test]
-  fn test_js_to_ts_specifier() {
-    #[track_caller]
-    fn test(specifier: &str, result: Option<&str>) {
-      assert_eq!(
-        js_to_ts_specifier(&ModuleSpecifier::parse(specifier).unwrap())
-          .map(|s| s.to_string()),
-        result.map(|s| s.to_string())
-      );
-    }
-
-    test("file:///mod.ts", None);
-    test("file:///mod.tsx", None);
-    test("file:///mod.mts", None);
-    test("file:///mod.json", None);
-    test("file:///mod.d.ts", None);
-    test("file:///mod.d.mts", None);
-    test("file:///mod.d.cts", None);
-    test("file:///mod.js", Some("file:///mod.ts"));
-    test("file:///mod.jsx", Some("file:///mod.tsx"));
-    test("file:///mod.mjs", Some("file:///mod.mts"));
-    test("file:///mod.cjs", Some("file:///mod.cts"));
-  }
 }