fix: redirects handling in module analysis

cli/js/compiler.ts

@@ -585,7 +585,6 @@ function buildSourceFileCache(
   sourceFileMap: Record<string, SourceFileMapEntry>
 ): void {
   for (const entry of Object.values(sourceFileMap)) {
-    assert(entry.sourceCode.length > 0);
     SourceFile.addToCache({
       url: entry.url,
       filename: entry.url,
@@ -597,6 +596,9 @@ function buildSourceFileCache(
       let mappedUrl = importDesc.resolvedSpecifier;
       const importedFile = sourceFileMap[importDesc.resolvedSpecifier];
       assert(importedFile);
+      if (importedFile.redirect) {
+        mappedUrl = importedFile.redirect;
+      }
       const isJsOrJsx =
         importedFile.mediaType === MediaType.JavaScript ||
         importedFile.mediaType === MediaType.JSX;
@@ -1032,6 +1034,7 @@ interface SourceFileMapEntry {
   url: string;
   sourceCode: string;
   mediaType: MediaType;
+  redirect?: string;
   imports: ImportDescriptor[];
   referencedFiles: ReferenceDescriptor[];
   libDirectives: ReferenceDescriptor[];

cli/module_graph.rs

@@ -19,6 +19,7 @@ use futures::FutureExt;
 use serde::Serialize;
 use serde::Serializer;
 use std::collections::HashMap;
+use std::collections::HashSet;
 use std::hash::BuildHasher;
 use std::path::PathBuf;
 use std::pin::Pin;
@@ -76,6 +77,7 @@ pub struct ReferenceDescriptor {
 pub struct ModuleGraphFile {
   pub specifier: String,
   pub url: String,
+  pub redirect: Option<String>,
   pub filename: String,
   pub imports: Vec<ImportDescriptor>,
   pub referenced_files: Vec<ReferenceDescriptor>,
@@ -87,13 +89,14 @@ pub struct ModuleGraphFile {
 }
 
 type SourceFileFuture =
-  Pin<Box<dyn Future<Output = Result<SourceFile, ErrBox>>>>;
+  Pin<Box<dyn Future<Output = Result<(ModuleSpecifier, SourceFile), ErrBox>>>>;
 
 pub struct ModuleGraphLoader {
   permissions: Permissions,
   file_fetcher: SourceFileFetcher,
   maybe_import_map: Option<ImportMap>,
   pending_downloads: FuturesUnordered<SourceFileFuture>,
+  has_downloaded: HashSet<ModuleSpecifier>,
   pub graph: ModuleGraph,
   is_dyn_import: bool,
   analyze_dynamic_imports: bool,
@@ -112,6 +115,7 @@ impl ModuleGraphLoader {
       permissions,
       maybe_import_map,
       pending_downloads: FuturesUnordered::new(),
+      has_downloaded: HashSet::new(),
       graph: ModuleGraph(HashMap::new()),
       is_dyn_import,
       analyze_dynamic_imports,
@@ -131,8 +135,8 @@ impl ModuleGraphLoader {
     self.download_module(specifier.clone(), None)?;
 
     loop {
-      let source_file = self.pending_downloads.next().await.unwrap()?;
-      self.visit_module(&source_file.url.clone().into(), source_file)?;
+      let (specifier, source_file) = self.pending_downloads.next().await.unwrap()?;
+      self.visit_module(&specifier, source_file)?;
       if self.pending_downloads.is_empty() {
         break;
       }
@@ -260,6 +264,7 @@ impl ModuleGraphLoader {
       ModuleGraphFile {
         specifier: specifier.to_string(),
         url: specifier.to_string(),
+        redirect: None,
         media_type: map_file_extension(&PathBuf::from(specifier.clone()))
           as i32,
         filename: specifier,
@@ -281,7 +286,7 @@ impl ModuleGraphLoader {
     module_specifier: ModuleSpecifier,
     maybe_referrer: Option<ModuleSpecifier>,
   ) -> Result<(), ErrBox> {
-    if self.graph.0.contains_key(&module_specifier.to_string()) {
+    if self.has_downloaded.contains(&module_specifier) {
       return Ok(());
     }
 
@@ -319,6 +324,7 @@ impl ModuleGraphLoader {
       }
     }
 
+    self.has_downloaded.insert(module_specifier.clone());
     let spec = module_specifier;
     let file_fetcher = self.file_fetcher.clone();
     let perms = self.permissions.clone();
@@ -328,13 +334,7 @@ impl ModuleGraphLoader {
       let source_file = file_fetcher
         .fetch_source_file(&spec_, maybe_referrer, perms)
         .await?;
-      // FIXME(bartlomieju):
-      // because of redirects we may end up with wrong URL,
-      // substitute with original one
-      Ok(SourceFile {
-        url: spec_.as_url().to_owned(),
-        ..source_file
-      })
+      Ok((spec_.clone(), source_file))
     }
     .boxed_local();
 
@@ -353,6 +353,29 @@ impl ModuleGraphLoader {
     let mut types_directives = vec![];
     let mut type_headers = vec![];
 
+    // IMPORTANT: source_file.url might be different than requested
+    // module_specifier - this situation needs to be handled manually
+    if module_specifier.to_string() != source_file.url.to_string() {
+      // TODO(bartlomieju): refactor, this is a band-aid
+      self.graph.0.insert(
+        module_specifier.to_string(),
+        ModuleGraphFile {
+          specifier: module_specifier.to_string(),
+          url: module_specifier.to_string(),
+          redirect: Some(source_file.url.to_string()),
+          filename: source_file.filename.to_str().unwrap().to_string(),
+          media_type: source_file.media_type as i32,
+          source_code: "".to_string(),
+          imports: vec![],
+          referenced_files: vec![],
+          lib_directives: vec![],
+          types_directives: vec![],
+          type_headers: vec![],
+        },
+      );
+    } 
+
+    let module_specifier = ModuleSpecifier::from(source_file.url.clone());
     let source_code = String::from_utf8(source_file.source_code)?;
 
     if source_file.media_type == MediaType::JavaScript
@@ -470,7 +493,8 @@ impl ModuleGraphLoader {
       module_specifier.to_string(),
       ModuleGraphFile {
         specifier: module_specifier.to_string(),
-        url: source_file.url.to_string(),
+        url: module_specifier.to_string(),
+        redirect: None,
         filename: source_file.filename.to_str().unwrap().to_string(),
         media_type: source_file.media_type as i32,
         source_code,

test.ts

@@ -0,0 +1 @@
+import { parse } from "https://cdn.pika.dev/html5parser@^1.1.0";