refactor: use parsed source cache when unfurling import map (#22001)

cli/tools/registry/mod.rs

@@ -27,6 +27,7 @@ use crate::args::deno_registry_url;
 use crate::args::CliOptions;
 use crate::args::Flags;
 use crate::args::PublishFlags;
+use crate::cache::ParsedSourceCache;
 use crate::factory::CliFactory;
 use crate::graph_util::ModuleGraphBuilder;
 use crate::http_util::HttpClient;
@@ -84,6 +85,7 @@ fn get_deno_json_package_name(
 
 async fn prepare_publish(
   deno_json: &ConfigFile,
+  source_cache: Arc<ParsedSourceCache>,
   import_map: Arc<ImportMap>,
 ) -> Result<Rc<PreparedPublishPackage>, AnyError> {
   let config_path = deno_json.specifier.to_file_path().unwrap();
@@ -132,7 +134,12 @@ async fn prepare_publish(
 
   let tarball = deno_core::unsync::spawn_blocking(move || {
     let unfurler = ImportMapUnfurler::new(&import_map);
-    tar::create_gzipped_tarball(&dir_path, &unfurler, &exclude_patterns)
+    tar::create_gzipped_tarball(
+      &dir_path,
+      &*source_cache,
+      &unfurler,
+      &exclude_patterns,
+    )
     .context("Failed to create a tarball")
   })
   .await??;
@@ -683,6 +690,7 @@ async fn prepare_packages_for_publishing(
 > {
   let maybe_workspace_config = deno_json.to_workspace_config()?;
   let module_graph_builder = cli_factory.module_graph_builder().await?.as_ref();
+  let source_cache = cli_factory.parsed_source_cache();
   let type_checker = cli_factory.type_checker().await?;
   let cli_options = cli_factory.cli_options();
 
@@ -700,7 +708,8 @@ async fn prepare_packages_for_publishing(
     )
     .await?;
     let mut prepared_package_by_name = HashMap::with_capacity(1);
-    let package = prepare_publish(&deno_json, import_map).await?;
+    let package =
+      prepare_publish(&deno_json, source_cache.clone(), import_map).await?;
     let package_name = format!("@{}/{}", package.scope, package.package);
     let publish_order_graph =
       PublishOrderGraph::new_single(package_name.clone());
@@ -731,8 +740,9 @@ async fn prepare_packages_for_publishing(
       .cloned()
       .map(|member| {
         let import_map = import_map.clone();
+        let source_cache = source_cache.clone();
         deno_core::unsync::spawn(async move {
-          let package = prepare_publish(&member.config_file, import_map)
+          let package = prepare_publish(&member.config_file, source_cache, import_map)
             .await
             .with_context(|| {
               format!("Failed preparing '{}'.", member.package_name)

cli/tools/registry/tar.rs

@@ -31,8 +31,7 @@ pub struct PublishableTarball {
 
 pub fn create_gzipped_tarball(
   dir: &Path,
-  // TODO(bartlomieju): this is too specific, factor it out into a callback that
+  source_cache: &dyn deno_graph::ParsedSourceStore,
-  // returns data
   unfurler: &ImportMapUnfurler,
   exclude_patterns: &PathOrPatternSet,
 ) -> Result<PublishableTarball, AnyError> {
@@ -71,12 +70,15 @@ pub fn create_gzipped_tarball(
         path: relative_path.to_path_buf(),
         size: data.len(),
       });
+      let content = match source_cache.get_parsed_source(&url) {
+        Some(parsed_source) => {
           let (content, unfurl_diagnostics) =
-        unfurler.unfurl(&url, data).with_context(|| {
+            unfurler.unfurl(&url, &parsed_source);
-          format!("Unable to unfurl file '{}'", entry.path().display())
-        })?;
-
           diagnostics.extend_from_slice(&unfurl_diagnostics);
+          content.into_bytes()
+        }
+        None => data,
+      };
       tar
         .add_file(relative_path_str.to_string(), &content)
         .with_context(|| {

cli/util/import_map.rs

@@ -3,13 +3,11 @@
 use std::collections::HashSet;
 
 use deno_ast::ParsedSource;
-use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_core::ModuleSpecifier;
 use deno_graph::DefaultModuleAnalyzer;
 use deno_graph::DependencyDescriptor;
 use deno_graph::DynamicTemplatePart;
-use deno_graph::MediaType;
 use deno_graph::TypeScriptReference;
 use deno_semver::jsr::JsrDepPackageReq;
 use deno_semver::jsr::JsrPackageReqReference;
@@ -83,46 +81,11 @@ impl<'a> ImportMapUnfurler<'a> {
   pub fn unfurl(
     &self,
     url: &ModuleSpecifier,
-    data: Vec<u8>,
+    parsed_source: &ParsedSource,
-  ) -> Result<(Vec<u8>, Vec<String>), AnyError> {
+  ) -> (String, Vec<String>) {
-    let mut diagnostics = vec![];
+    let mut diagnostics = Vec::new();
-    let media_type = MediaType::from_specifier(url);
-
-    match media_type {
-      MediaType::JavaScript
-      | MediaType::Jsx
-      | MediaType::Mjs
-      | MediaType::Cjs
-      | MediaType::TypeScript
-      | MediaType::Mts
-      | MediaType::Cts
-      | MediaType::Dts
-      | MediaType::Dmts
-      | MediaType::Dcts
-      | MediaType::Tsx => {
-        // continue
-      }
-      MediaType::SourceMap
-      | MediaType::Unknown
-      | MediaType::Json
-      | MediaType::Wasm
-      | MediaType::TsBuildInfo => {
-        // not unfurlable data
-        return Ok((data, diagnostics));
-      }
-    }
-
-    let text = String::from_utf8(data)?;
-    let parsed_source = deno_ast::parse_module(deno_ast::ParseParams {
-      specifier: url.to_string(),
-      text_info: deno_ast::SourceTextInfo::from_string(text),
-      media_type,
-      capture_tokens: false,
-      maybe_syntax: None,
-      scope_analysis: false,
-    })?;
     let mut text_changes = Vec::new();
-    let module_info = DefaultModuleAnalyzer::module_info(&parsed_source);
+    let module_info = DefaultModuleAnalyzer::module_info(parsed_source);
     let analyze_specifier =
       |specifier: &str,
        range: &deno_graph::PositionRange,
@@ -130,7 +93,7 @@ impl<'a> ImportMapUnfurler<'a> {
         let resolved = self.import_map.resolve(specifier, url);
         if let Ok(resolved) = resolved {
           text_changes.push(deno_ast::TextChange {
-            range: to_range(&parsed_source, range),
+            range: to_range(parsed_source, range),
             new_text: make_relative_to(url, &resolved),
           });
         }
@@ -148,7 +111,7 @@ impl<'a> ImportMapUnfurler<'a> {
           let success = try_unfurl_dynamic_dep(
             self.import_map,
             url,
-            &parsed_source,
+            parsed_source,
             dep,
             &mut text_changes,
           );
@@ -192,25 +155,12 @@ impl<'a> ImportMapUnfurler<'a> {
         &mut text_changes,
       );
     }
-    Ok((
+
-      deno_ast::apply_text_changes(
+    let rewritten_text = deno_ast::apply_text_changes(
       parsed_source.text_info().text_str(),
       text_changes,
-      )
+    );
-      .into_bytes(),
+    (rewritten_text, diagnostics)
-      diagnostics,
-    ))
-  }
-
-  #[cfg(test)]
-  fn unfurl_to_string(
-    &self,
-    url: &ModuleSpecifier,
-    data: Vec<u8>,
-  ) -> Result<(String, Vec<String>), AnyError> {
-    let (data, diagnostics) = self.unfurl(url, data)?;
-    let content = String::from_utf8(data)?;
-    Ok((content, diagnostics))
   }
 }
 
@@ -309,11 +259,26 @@ fn to_range(
 #[cfg(test)]
 mod tests {
   use super::*;
+  use deno_ast::MediaType;
   use deno_ast::ModuleSpecifier;
   use deno_core::serde_json::json;
+  use deno_core::url::Url;
   use import_map::ImportMapWithDiagnostics;
   use pretty_assertions::assert_eq;
 
+  fn parse_ast(specifier: &Url, source_code: &str) -> ParsedSource {
+    let media_type = MediaType::from_specifier(specifier);
+    deno_ast::parse_module(deno_ast::ParseParams {
+      specifier: specifier.to_string(),
+      media_type,
+      capture_tokens: false,
+      maybe_syntax: None,
+      scope_analysis: false,
+      text_info: deno_ast::SourceTextInfo::new(source_code.into()),
+    })
+    .unwrap()
+  }
+
   #[test]
   fn test_unfurling() {
     let deno_json_url =
@@ -345,9 +310,8 @@ const test5 = await import(`lib${expr}`);
 const test6 = await import(`${expr}`);
 "#;
       let specifier = ModuleSpecifier::parse("file:///dev/mod.ts").unwrap();
-      let (unfurled_source, d) = unfurler
+      let source = parse_ast(&specifier, source_code);
-        .unfurl_to_string(&specifier, source_code.as_bytes().to_vec())
+      let (unfurled_source, d) = unfurler.unfurl(&specifier, &source);
-        .unwrap();
       assert_eq!(d.len(), 2);
       assert!(d[0].starts_with("Dynamic import was not analyzable and won't use the import map once published."));
       assert!(d[1].starts_with("Dynamic import was not analyzable and won't use the import map once published."));
@@ -366,20 +330,5 @@ const test6 = await import(`${expr}`);
 "#;
       assert_eq!(unfurled_source, expected_source);
     }
-
-    // Unfurling file with "unknown" media type should leave it as is
-    {
-      let source_code = r#"import express from "express";"
-import foo from "lib/foo.ts";
-import bar from "lib/bar.ts";
-import fizz from "fizz";
-"#;
-      let specifier = ModuleSpecifier::parse("file:///dev/mod").unwrap();
-      let (unfurled_source, d) = unfurler
-        .unfurl_to_string(&specifier, source_code.as_bytes().to_vec())
-        .unwrap();
-      assert!(d.is_empty());
-      assert_eq!(unfurled_source, source_code);
-    }
   }
 }