Skip to content

Commit

Permalink
wip fix redirects in module graph
Browse files Browse the repository at this point in the history
  • Loading branch information
bartlomieju committed May 21, 2020
1 parent 65f4e59 commit 41ee861
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 13 deletions.
5 changes: 4 additions & 1 deletion cli/js/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -1032,6 +1034,7 @@ interface SourceFileMapEntry {
url: string;
sourceCode: string;
mediaType: MediaType;
redirect?: string;
imports: ImportDescriptor[];
referencedFiles: ReferenceDescriptor[];
libDirectives: ReferenceDescriptor[];
Expand Down
48 changes: 36 additions & 12 deletions cli/module_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -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,
Expand All @@ -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(());
}

Expand Down Expand Up @@ -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();
Expand All @@ -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();

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import { parse } from "https://cdn.pika.dev/html5parser@^1.1.0";

0 comments on commit 41ee861

Please sign in to comment.