From 000ac5c40b71c38cc26a36e579fbb0936f0573d7 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Tue, 13 Oct 2020 21:54:28 +1100 Subject: [PATCH] reafactor(cli): use Emit enum and rename ts_build_info (#7952) --- cli/module_graph2.rs | 138 ++++++++++++++++------------------- cli/specifier_handler.rs | 152 ++++++++++++++------------------------- 2 files changed, 117 insertions(+), 173 deletions(-) diff --git a/cli/module_graph2.rs b/cli/module_graph2.rs index 6985864134..5e82632b0f 100644 --- a/cli/module_graph2.rs +++ b/cli/module_graph2.rs @@ -13,8 +13,7 @@ use crate::lockfile::Lockfile; use crate::media_type::MediaType; use crate::specifier_handler::CachedModule; use crate::specifier_handler::DependencyMap; -use crate::specifier_handler::EmitMap; -use crate::specifier_handler::EmitType; +use crate::specifier_handler::Emit; use crate::specifier_handler::FetchFuture; use crate::specifier_handler::SpecifierHandler; use crate::tsc_config::IgnoredCompilerOptions; @@ -41,8 +40,6 @@ use std::sync::Mutex; use std::time::Instant; use swc_ecmascript::dep_graph::DependencyKind; -pub type BuildInfoMap = HashMap; - lazy_static! { /// Matched the `@deno-types` pragma. static ref DENO_TYPES_RE: Regex = @@ -163,13 +160,12 @@ fn get_version(source: &str, version: &str, config: &[u8]) -> String { #[derive(Debug, Clone)] struct Module { dependencies: DependencyMap, - emits: EmitMap, is_dirty: bool, is_hydrated: bool, is_parsed: bool, - maybe_emit_path: Option, + maybe_emit: Option, + maybe_emit_path: Option<(PathBuf, Option)>, maybe_import_map: Option>>, - maybe_map_path: Option, maybe_parsed_module: Option, maybe_types: Option<(String, ModuleSpecifier)>, maybe_version: Option, @@ -183,13 +179,12 @@ impl Default for Module { fn default() -> Self { Module { dependencies: HashMap::new(), - emits: HashMap::new(), is_dirty: false, is_hydrated: false, is_parsed: false, + maybe_emit: None, maybe_emit_path: None, maybe_import_map: None, - maybe_map_path: None, maybe_parsed_module: None, maybe_types: None, maybe_version: None, @@ -227,8 +222,8 @@ impl Module { self.media_type = cached_module.media_type; self.source = cached_module.source; self.source_path = cached_module.source_path; + self.maybe_emit = cached_module.maybe_emit; self.maybe_emit_path = cached_module.maybe_emit_path; - self.maybe_map_path = cached_module.maybe_map_path; if self.maybe_import_map.is_none() { if let Some(dependencies) = cached_module.maybe_dependencies { self.dependencies = dependencies; @@ -245,9 +240,8 @@ impl Module { } else { None }; - self.is_dirty = false; - self.emits = cached_module.emits; self.maybe_version = cached_module.maybe_version; + self.is_dirty = false; self.is_hydrated = true; } @@ -418,8 +412,8 @@ pub struct TranspileOptions { /// be able to manipulate and handle the graph. #[derive(Debug)] pub struct Graph2 { - build_info: BuildInfoMap, handler: Rc>, + maybe_ts_build_info: Option, modules: HashMap, roots: Vec, } @@ -432,8 +426,8 @@ impl Graph2 { /// pub fn new(handler: Rc>) -> Self { Graph2 { - build_info: HashMap::new(), handler, + maybe_ts_build_info: None, modules: HashMap::new(), roots: Vec::new(), } @@ -534,15 +528,21 @@ impl Graph2 { let files = self.get_info_map(); let total_size = totals.get(&module).unwrap_or(&m.size()).to_owned(); + let (compiled, map) = + if let Some((emit_path, maybe_map_path)) = &m.maybe_emit_path { + (Some(emit_path.clone()), maybe_map_path.clone()) + } else { + (None, None) + }; Ok(ModuleGraphInfo { - compiled: m.maybe_emit_path.clone(), + compiled, dep_count: self.modules.len() - 1, file_type: m.media_type, files, info, local: m.source_path.clone(), - map: m.maybe_map_path.clone(), + map, module, total_size, }) @@ -550,30 +550,22 @@ impl Graph2 { /// Update the handler with any modules that are marked as _dirty_ and update /// any build info if present. - fn flush(&mut self, emit_type: &EmitType) -> Result<(), AnyError> { + fn flush(&mut self) -> Result<(), AnyError> { let mut handler = self.handler.borrow_mut(); for (_, module) in self.modules.iter_mut() { if module.is_dirty { - let (code, maybe_map) = module.emits.get(emit_type).unwrap(); - handler.set_cache( - &module.specifier, - &emit_type, - code.clone(), - maybe_map.clone(), - )?; - module.is_dirty = false; + if let Some(emit) = &module.maybe_emit { + handler.set_cache(&module.specifier, emit)?; + } if let Some(version) = &module.maybe_version { handler.set_version(&module.specifier, version.clone())?; } + module.is_dirty = false; } } for root_specifier in self.roots.iter() { - if let Some(build_info) = self.build_info.get(&emit_type) { - handler.set_build_info( - root_specifier, - &emit_type, - build_info.to_owned(), - )?; + if let Some(ts_build_info) = &self.maybe_ts_build_info { + handler.set_ts_build_info(root_specifier, ts_build_info.to_owned())?; } } @@ -618,7 +610,6 @@ impl Graph2 { options: TranspileOptions, ) -> Result<(Stats, Option), AnyError> { let start = Instant::now(); - let emit_type = EmitType::Cli; let mut ts_config = TsConfig::new(json!({ "checkJs": false, @@ -662,7 +653,7 @@ impl Graph2 { } let config = ts_config.as_bytes(); // skip modules that already have a valid emit - if module.emits.contains_key(&emit_type) && module.emit_valid(&config) { + if module.maybe_emit.is_some() && module.emit_valid(&config) { continue; } if module.maybe_parsed_module.is_none() { @@ -671,11 +662,11 @@ impl Graph2 { let parsed_module = module.maybe_parsed_module.clone().unwrap(); let emit = parsed_module.transpile(&emit_options)?; emit_count += 1; - module.emits.insert(emit_type.clone(), emit); + module.maybe_emit = Some(Emit::Cli(emit)); module.set_version(&config); module.is_dirty = true; } - self.flush(&emit_type)?; + self.flush()?; let stats = Stats(vec![ ("Files".to_string(), self.modules.len() as u128), @@ -822,9 +813,9 @@ mod tests { #[derive(Debug, Default)] pub struct MockSpecifierHandler { pub fixtures: PathBuf, - pub build_info: HashMap, - pub build_info_calls: Vec<(ModuleSpecifier, EmitType, String)>, - pub cache_calls: Vec<(ModuleSpecifier, EmitType, String, Option)>, + pub maybe_ts_build_info: Option, + pub ts_build_info_calls: Vec<(ModuleSpecifier, String)>, + pub cache_calls: Vec<(ModuleSpecifier, Emit)>, pub deps_calls: Vec<(ModuleSpecifier, DependencyMap)>, pub types_calls: Vec<(ModuleSpecifier, String)>, pub version_calls: Vec<(ModuleSpecifier, String)>, @@ -871,26 +862,18 @@ mod tests { fn fetch(&mut self, specifier: ModuleSpecifier) -> FetchFuture { Box::pin(future::ready(self.get_cache(specifier))) } - fn get_build_info( + fn get_ts_build_info( &self, - specifier: &ModuleSpecifier, - _cache_type: &EmitType, + _specifier: &ModuleSpecifier, ) -> Result, AnyError> { - Ok(self.build_info.get(specifier).cloned()) + Ok(self.maybe_ts_build_info.clone()) } fn set_cache( &mut self, specifier: &ModuleSpecifier, - cache_type: &EmitType, - code: String, - maybe_map: Option, + emit: &Emit, ) -> Result<(), AnyError> { - self.cache_calls.push(( - specifier.clone(), - cache_type.clone(), - code, - maybe_map, - )); + self.cache_calls.push((specifier.clone(), emit.clone())); Ok(()) } fn set_types( @@ -901,20 +884,15 @@ mod tests { self.types_calls.push((specifier.clone(), types)); Ok(()) } - fn set_build_info( + fn set_ts_build_info( &mut self, specifier: &ModuleSpecifier, - cache_type: &EmitType, - build_info: String, + ts_build_info: String, ) -> Result<(), AnyError> { + self.maybe_ts_build_info = Some(ts_build_info.clone()); self - .build_info - .insert(specifier.clone(), build_info.clone()); - self.build_info_calls.push(( - specifier.clone(), - cache_type.clone(), - build_info, - )); + .ts_build_info_calls + .push((specifier.clone(), ts_build_info)); Ok(()) } fn set_deps( @@ -1068,16 +1046,22 @@ mod tests { assert_eq!(maybe_ignored_options, None); let h = handler.borrow(); assert_eq!(h.cache_calls.len(), 2); - assert_eq!(h.cache_calls[0].1, EmitType::Cli); - assert!(h.cache_calls[0] - .2 - .contains("# sourceMappingURL=data:application/json;base64,")); - assert_eq!(h.cache_calls[0].3, None); - assert_eq!(h.cache_calls[1].1, EmitType::Cli); - assert!(h.cache_calls[1] - .2 - .contains("# sourceMappingURL=data:application/json;base64,")); - assert_eq!(h.cache_calls[0].3, None); + match &h.cache_calls[0].1 { + Emit::Cli((code, maybe_map)) => { + assert!( + code.contains("# sourceMappingURL=data:application/json;base64,") + ); + assert!(maybe_map.is_none()); + } + }; + match &h.cache_calls[1].1 { + Emit::Cli((code, maybe_map)) => { + assert!( + code.contains("# sourceMappingURL=data:application/json;base64,") + ); + assert!(maybe_map.is_none()); + } + }; assert_eq!(h.deps_calls.len(), 7); assert_eq!( h.deps_calls[0].0, @@ -1135,10 +1119,14 @@ mod tests { let h = handler.borrow(); assert_eq!(h.cache_calls.len(), 1, "only one file should be emitted"); // FIXME(bartlomieju): had to add space in `
`, probably a quirk in swc_ecma_codegen - assert!( - h.cache_calls[0].2.contains("
Hello world!
"), - "jsx should have been preserved" - ); + match &h.cache_calls[0].1 { + Emit::Cli((code, _)) => { + assert!( + code.contains("
Hello world!
"), + "jsx should have been preserved" + ); + } + } } #[tokio::test] diff --git a/cli/specifier_handler.rs b/cli/specifier_handler.rs index e392a3c3a8..2b81e1a2fb 100644 --- a/cli/specifier_handler.rs +++ b/cli/specifier_handler.rs @@ -16,23 +16,20 @@ use serde::Deserialize; use serde::Serialize; use std::collections::HashMap; use std::env; -use std::error::Error; use std::fmt; use std::path::PathBuf; use std::pin::Pin; use std::sync::Arc; pub type DependencyMap = HashMap; -pub type EmitMap = HashMap)>; pub type FetchFuture = Pin> + 'static)>>; #[derive(Debug, Clone)] pub struct CachedModule { - pub emits: EmitMap, pub maybe_dependencies: Option, - pub maybe_emit_path: Option, - pub maybe_map_path: Option, + pub maybe_emit: Option, + pub maybe_emit_path: Option<(PathBuf, Option)>, pub maybe_types: Option, pub maybe_version: Option, pub media_type: MediaType, @@ -45,10 +42,9 @@ pub struct CachedModule { impl Default for CachedModule { fn default() -> Self { CachedModule { - emits: HashMap::new(), maybe_dependencies: None, + maybe_emit: None, maybe_emit_path: None, - maybe_map_path: None, maybe_types: None, maybe_version: None, media_type: MediaType::Unknown, @@ -60,23 +56,21 @@ impl Default for CachedModule { } } -/// An enum that represents the different types of emitted code that can be -/// cached. Different types can utilise different configurations which can -/// change the validity of the emitted code. -#[allow(unused)] +/// An enum to own the a specific emit. +/// +/// Currently there is only one type of emit that is cacheable, but this has +/// been added to future proof the ability for the specifier handler +/// implementations to be able to handle other types of emits, like form a +/// runtime API which might have a different configuration. #[derive(Debug, Clone, Eq, PartialEq, Hash)] -pub enum EmitType { +pub enum Emit { /// Code that was emitted for use by the CLI - Cli, - /// Code that was emitted for bundling purposes - Bundle, - /// Code that was emitted based on a request to the runtime APIs - Runtime, + Cli((String, Option)), } -impl Default for EmitType { +impl Default for Emit { fn default() -> Self { - EmitType::Cli + Emit::Cli(("".to_string(), None)) } } @@ -100,20 +94,16 @@ pub trait SpecifierHandler { /// not expected to be cached for each module, but are "lazily" checked when /// a root module is identified. The `emit_type` also indicates what form /// of the module the build info is valid for. - fn get_build_info( + fn get_ts_build_info( &self, specifier: &ModuleSpecifier, - emit_type: &EmitType, ) -> Result, AnyError>; - /// Set the emitted code (and maybe map) for a given module specifier. The - /// cache type indicates what form the emit is related to. + /// Set the emit for the module specifier. fn set_cache( &mut self, specifier: &ModuleSpecifier, - emit_type: &EmitType, - code: String, - maybe_map: Option, + emit: &Emit, ) -> Result<(), AnyError>; /// When parsed out of a JavaScript module source, the triple slash reference @@ -125,11 +115,10 @@ pub trait SpecifierHandler { ) -> Result<(), AnyError>; /// Set the build info for a module specifier, also providing the cache type. - fn set_build_info( + fn set_ts_build_info( &mut self, specifier: &ModuleSpecifier, - emit_type: &EmitType, - build_info: String, + ts_build_info: String, ) -> Result<(), AnyError>; /// Set the graph dependencies for a given module specifier. @@ -154,29 +143,6 @@ impl fmt::Debug for dyn SpecifierHandler { } } -/// Errors that could be raised by a `SpecifierHandler` implementation. -#[derive(Debug, Clone, Eq, PartialEq)] -pub enum SpecifierHandlerError { - /// An error representing an error the `EmitType` that was supplied to a - /// method of an implementor of the `SpecifierHandler` trait. - UnsupportedEmitType(EmitType), -} -use SpecifierHandlerError::*; - -impl fmt::Display for SpecifierHandlerError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - UnsupportedEmitType(ref emit_type) => write!( - f, - "The emit type of \"{:?}\" is unsupported for this operation.", - emit_type - ), - } - } -} - -impl Error for SpecifierHandlerError {} - /// A representation of meta data for a compiled file. /// /// *Note* this is currently just a copy of what is located in `tsc.rs` but will @@ -249,29 +215,28 @@ impl SpecifierHandler for FetchHandler { None }; - let mut maybe_map_path: Option = None; + let mut maybe_map_path = None; let map_path = disk_cache.get_cache_filename_with_extension(&url, "js.map"); - let maybe_map: Option = if let Ok(map) = disk_cache.get(&map_path) - { + let maybe_map = if let Ok(map) = disk_cache.get(&map_path) { maybe_map_path = Some(disk_cache.location.join(map_path)); Some(String::from_utf8(map)?) } else { None }; - let mut emits = HashMap::new(); - let mut maybe_emit_path: Option = None; + let mut maybe_emit = None; + let mut maybe_emit_path = None; let emit_path = disk_cache.get_cache_filename_with_extension(&url, "js"); if let Ok(code) = disk_cache.get(&emit_path) { - maybe_emit_path = Some(disk_cache.location.join(emit_path)); - emits.insert(EmitType::Cli, (String::from_utf8(code)?, maybe_map)); + maybe_emit = Some(Emit::Cli((String::from_utf8(code)?, maybe_map))); + maybe_emit_path = + Some((disk_cache.location.join(emit_path), maybe_map_path)); }; Ok(CachedModule { - emits, maybe_dependencies: None, + maybe_emit, maybe_emit_path, - maybe_map_path, maybe_types: source_file.types_header, maybe_version, media_type: source_file.media_type, @@ -283,63 +248,54 @@ impl SpecifierHandler for FetchHandler { .boxed_local() } - fn get_build_info( + fn get_ts_build_info( &self, specifier: &ModuleSpecifier, - emit_type: &EmitType, ) -> Result, AnyError> { - if emit_type != &EmitType::Cli { - return Err(UnsupportedEmitType(emit_type.clone()).into()); - } let filename = self .disk_cache .get_cache_filename_with_extension(specifier.as_url(), "buildinfo"); - if let Ok(build_info) = self.disk_cache.get(&filename) { - return Ok(Some(String::from_utf8(build_info)?)); + if let Ok(ts_build_info) = self.disk_cache.get(&filename) { + return Ok(Some(String::from_utf8(ts_build_info)?)); } Ok(None) } - fn set_build_info( + fn set_ts_build_info( &mut self, specifier: &ModuleSpecifier, - emit_type: &EmitType, - build_info: String, + ts_build_info: String, ) -> Result<(), AnyError> { - if emit_type != &EmitType::Cli { - return Err(UnsupportedEmitType(emit_type.clone()).into()); - } let filename = self .disk_cache .get_cache_filename_with_extension(specifier.as_url(), "buildinfo"); self .disk_cache - .set(&filename, build_info.as_bytes()) + .set(&filename, ts_build_info.as_bytes()) .map_err(|e| e.into()) } fn set_cache( &mut self, specifier: &ModuleSpecifier, - emit_type: &EmitType, - code: String, - maybe_map: Option, + emit: &Emit, ) -> Result<(), AnyError> { - if emit_type != &EmitType::Cli { - return Err(UnsupportedEmitType(emit_type.clone()).into()); - } - let filename = self - .disk_cache - .get_cache_filename_with_extension(specifier.as_url(), "js"); - self.disk_cache.set(&filename, code.as_bytes())?; + match emit { + Emit::Cli((code, maybe_map)) => { + let url = specifier.as_url(); + let filename = + self.disk_cache.get_cache_filename_with_extension(url, "js"); + self.disk_cache.set(&filename, code.as_bytes())?; - if let Some(map) = maybe_map { - let filename = self - .disk_cache - .get_cache_filename_with_extension(specifier.as_url(), "js.map"); - self.disk_cache.set(&filename, map.as_bytes())?; - } + if let Some(map) = maybe_map { + let filename = self + .disk_cache + .get_cache_filename_with_extension(url, "js.map"); + self.disk_cache.set(&filename, map.as_bytes())?; + } + } + }; Ok(()) } @@ -423,7 +379,7 @@ pub mod tests { .unwrap(); let cached_module: CachedModule = file_fetcher.fetch(specifier.clone()).await.unwrap(); - assert_eq!(cached_module.emits.len(), 0); + assert!(cached_module.maybe_emit.is_none()); assert!(cached_module.maybe_dependencies.is_none()); assert_eq!(cached_module.media_type, MediaType::TypeScript); assert_eq!( @@ -443,16 +399,16 @@ pub mod tests { .unwrap(); let cached_module: CachedModule = file_fetcher.fetch(specifier.clone()).await.unwrap(); - assert_eq!(cached_module.emits.len(), 0); + assert!(cached_module.maybe_emit.is_none()); let code = String::from("some code"); file_fetcher - .set_cache(&specifier, &EmitType::Cli, code, None) + .set_cache(&specifier, &Emit::Cli((code, None))) .expect("could not set cache"); let cached_module: CachedModule = file_fetcher.fetch(specifier.clone()).await.unwrap(); - assert_eq!(cached_module.emits.len(), 1); - let actual_emit = cached_module.emits.get(&EmitType::Cli).unwrap(); - assert_eq!(actual_emit.0, "some code"); - assert_eq!(actual_emit.1, None); + assert_eq!( + cached_module.maybe_emit, + Some(Emit::Cli(("some code".to_string(), None))) + ); } }