Reland "fix(cli): don't store blob and data urls in the module cache" (#18581)

Relands #18261 now that
https://github.com/lucacasonato/esbuild_deno_loader/pull/54 is landed
and used by fresh.
Fixes #18260.

1 files changed, 24 insertions, 104 deletions

diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs
index 17cc73bf2..0413e77a0 100644
--- a/cli/file_fetcher.rs
+++ b/cli/file_fetcher.rs
@@ -33,7 +33,6 @@ use deno_runtime::deno_fetch::reqwest::StatusCode;
 use deno_runtime::deno_web::BlobStore;
 use deno_runtime::permissions::PermissionsContainer;
 use log::debug;
-use std::borrow::Borrow;
 use std::collections::HashMap;
 use std::env;
 use std::fs;
@@ -50,10 +49,6 @@ pub const SUPPORTED_SCHEMES: [&str; 5] =
 /// A structure representing a source file.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct File {
-  /// The path to the local version of the source file.  For local files this
-  /// will be the direct path to that file.  For remote files, it will be the
-  /// path to the file in the HTTP cache.
-  pub local: PathBuf,
   /// For remote files, if there was an `X-TypeScript-Type` header, the parsed
   /// out value of that header.
   pub maybe_types: Option<String>,
@@ -90,13 +85,12 @@ fn fetch_local(specifier: &ModuleSpecifier) -> Result<File, AnyError> {
   let local = specifier.to_file_path().map_err(|_| {
     uri_error(format!("Invalid file path.\n  Specifier: {specifier}"))
   })?;
-  let bytes = fs::read(&local)?;
+  let bytes = fs::read(local)?;
   let charset = text_encoding::detect_charset(&bytes).to_string();
   let source = get_source_from_bytes(bytes, Some(charset))?;
   let media_type = MediaType::from_specifier(specifier);
 
   Ok(File {
-    local,
     maybe_types: None,
     media_type,
     source: source.into(),
@@ -179,7 +173,7 @@ pub struct FileFetcher {
   cache_setting: CacheSetting,
   pub http_cache: HttpCache,
   http_client: Arc<HttpClient>,
-  blob_store: BlobStore,
+  blob_store: Arc<BlobStore>,
   download_log_level: log::Level,
   progress_bar: Option<ProgressBar>,
 }
@@ -190,7 +184,7 @@ impl FileFetcher {
     cache_setting: CacheSetting,
     allow_remote: bool,
     http_client: Arc<HttpClient>,
-    blob_store: BlobStore,
+    blob_store: Arc<BlobStore>,
     progress_bar: Option<ProgressBar>,
   ) -> Self {
     Self {
@@ -218,13 +212,6 @@ impl FileFetcher {
     bytes: Vec<u8>,
     headers: &HashMap<String, String>,
   ) -> Result<File, AnyError> {
-    let local =
-      self
-        .http_cache
-        .get_cache_filename(specifier)
-        .ok_or_else(|| {
-          generic_error("Cannot convert specifier to cached filename.")
-        })?;
     let maybe_content_type = headers.get("content-type");
     let (media_type, maybe_charset) =
       map_content_type(specifier, maybe_content_type);
@@ -238,7 +225,6 @@ impl FileFetcher {
     };
 
     Ok(File {
-      local,
       maybe_types,
       media_type,
       source: source.into(),
@@ -290,39 +276,11 @@ impl FileFetcher {
     specifier: &ModuleSpecifier,
   ) -> Result<File, AnyError> {
     debug!("FileFetcher::fetch_data_url() - specifier: {}", specifier);
-    match self.fetch_cached(specifier, 0) {
-      Ok(Some(file)) => return Ok(file),
-      Ok(None) => {}
-      Err(err) => return Err(err),
-    }
-
-    if self.cache_setting == CacheSetting::Only {
-      return Err(custom_error(
-        "NotCached",
-        format!(
-          "Specifier not found in cache: \"{specifier}\", --cached-only is specified."
-        ),
-      ));
-    }
-
     let (source, content_type) = get_source_from_data_url(specifier)?;
     let (media_type, _) = map_content_type(specifier, Some(&content_type));
-
-    let local =
-      self
-        .http_cache
-        .get_cache_filename(specifier)
-        .ok_or_else(|| {
-          generic_error("Cannot convert specifier to cached filename.")
-        })?;
     let mut headers = HashMap::new();
     headers.insert("content-type".to_string(), content_type);
-    self
-      .http_cache
-      .set(specifier, headers.clone(), source.as_bytes())?;
-
     Ok(File {
-      local,
       maybe_types: None,
       media_type,
       source: source.into(),
@@ -337,32 +295,15 @@ impl FileFetcher {
     specifier: &ModuleSpecifier,
   ) -> Result<File, AnyError> {
     debug!("FileFetcher::fetch_blob_url() - specifier: {}", specifier);
-    match self.fetch_cached(specifier, 0) {
-      Ok(Some(file)) => return Ok(file),
-      Ok(None) => {}
-      Err(err) => return Err(err),
-    }
-
-    if self.cache_setting == CacheSetting::Only {
-      return Err(custom_error(
-        "NotCached",
-        format!(
-          "Specifier not found in cache: \"{specifier}\", --cached-only is specified."
-        ),
-      ));
-    }
-
-    let blob = {
-      let blob_store = self.blob_store.borrow();
-      blob_store
-        .get_object_url(specifier.clone())
-        .ok_or_else(|| {
-          custom_error(
-            "NotFound",
-            format!("Blob URL not found: \"{specifier}\"."),
-          )
-        })?
-    };
+    let blob = self
+      .blob_store
+      .get_object_url(specifier.clone())
+      .ok_or_else(|| {
+        custom_error(
+          "NotFound",
+          format!("Blob URL not found: \"{specifier}\"."),
+        )
+      })?;
 
     let content_type = blob.media_type.clone();
     let bytes = blob.read_all().await?;
@@ -370,22 +311,10 @@ impl FileFetcher {
     let (media_type, maybe_charset) =
       map_content_type(specifier, Some(&content_type));
     let source = get_source_from_bytes(bytes, maybe_charset)?;
-
-    let local =
-      self
-        .http_cache
-        .get_cache_filename(specifier)
-        .ok_or_else(|| {
-          generic_error("Cannot convert specifier to cached filename.")
-        })?;
     let mut headers = HashMap::new();
     headers.insert("content-type".to_string(), content_type);
-    self
-      .http_cache
-      .set(specifier, headers.clone(), source.as_bytes())?;
 
     Ok(File {
-      local,
       maybe_types: None,
       media_type,
       source: source.into(),
@@ -562,17 +491,9 @@ impl FileFetcher {
       // disk changing effecting things like workers and dynamic imports.
       fetch_local(specifier)
     } else if scheme == "data" {
-      let result = self.fetch_data_url(specifier);
-      if let Ok(file) = &result {
-        self.cache.insert(specifier.clone(), file.clone());
-      }
-      result
+      self.fetch_data_url(specifier)
     } else if scheme == "blob" {
-      let result = self.fetch_blob_url(specifier).await;
-      if let Ok(file) = &result {
-        self.cache.insert(specifier.clone(), file.clone());
-      }
-      result
+      self.fetch_blob_url(specifier).await
     } else if !self.allow_remote {
       Err(custom_error(
         "NoRemote",
@@ -762,10 +683,10 @@ mod tests {
   fn setup_with_blob_store(
     cache_setting: CacheSetting,
     maybe_temp_dir: Option<TempDir>,
-  ) -> (FileFetcher, TempDir, BlobStore) {
+  ) -> (FileFetcher, TempDir, Arc<BlobStore>) {
     let temp_dir = maybe_temp_dir.unwrap_or_default();
     let location = temp_dir.path().join("deps").to_path_buf();
-    let blob_store = BlobStore::default();
+    let blob_store: Arc<BlobStore> = Default::default();
     let file_fetcher = FileFetcher::new(
       HttpCache::new(location),
       cache_setting,
@@ -1035,7 +956,6 @@ mod tests {
     let local = temp_dir.path().join("a.ts");
     let specifier = ModuleSpecifier::from_file_path(&local).unwrap();
     let file = File {
-      local: local.to_path_buf(),
       maybe_types: None,
       media_type: MediaType::TypeScript,
       source: "some source code".into(),
@@ -1206,7 +1126,7 @@ mod tests {
       CacheSetting::ReloadAll,
       true,
       Arc::new(HttpClient::new(None, None)),
-      BlobStore::default(),
+      Default::default(),
       None,
     );
     let result = file_fetcher
@@ -1231,7 +1151,7 @@ mod tests {
       CacheSetting::Use,
       true,
       Arc::new(HttpClient::new(None, None)),
-      BlobStore::default(),
+      Default::default(),
       None,
     );
     let specifier =
@@ -1256,7 +1176,7 @@ mod tests {
       CacheSetting::Use,
       true,
       Arc::new(HttpClient::new(None, None)),
-      BlobStore::default(),
+      Default::default(),
       None,
     );
     let result = file_fetcher_02
@@ -1397,7 +1317,7 @@ mod tests {
       CacheSetting::Use,
       true,
       Arc::new(HttpClient::new(None, None)),
-      BlobStore::default(),
+      Default::default(),
       None,
     );
     let specifier =
@@ -1425,7 +1345,7 @@ mod tests {
       CacheSetting::Use,
       true,
       Arc::new(HttpClient::new(None, None)),
-      BlobStore::default(),
+      Default::default(),
       None,
     );
     let result = file_fetcher_02
@@ -1524,7 +1444,7 @@ mod tests {
       CacheSetting::Use,
       false,
       Arc::new(HttpClient::new(None, None)),
-      BlobStore::default(),
+      Default::default(),
       None,
     );
     let specifier =
@@ -1549,7 +1469,7 @@ mod tests {
       CacheSetting::Only,
       true,
       Arc::new(HttpClient::new(None, None)),
-      BlobStore::default(),
+      Default::default(),
       None,
     );
     let file_fetcher_02 = FileFetcher::new(
@@ -1557,7 +1477,7 @@ mod tests {
       CacheSetting::Use,
       true,
       Arc::new(HttpClient::new(None, None)),
-      BlobStore::default(),
+      Default::default(),
       None,
     );
     let specifier =