From c824eb5817db675be4d9966a0d1a43d90dfa61fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 25 Jan 2020 18:53:16 +0100 Subject: refactor: Modules and Loader trait (#3791) * move is_dyn_import argument from Loader::resolve to Loader::load - it was always kind of strange that resolve() checks permissions. * change argument type from &str to &ModuleSpecifier where applicable --- core/modules.rs | 134 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 73 insertions(+), 61 deletions(-) (limited to 'core/modules.rs') diff --git a/core/modules.rs b/core/modules.rs index 60158d6f1..21c2481dd 100644 --- a/core/modules.rs +++ b/core/modules.rs @@ -28,19 +28,25 @@ pub trait Loader: Send + Sync { /// When implementing an spec-complaint VM, this should be exactly the /// algorithm described here: /// https://html.spec.whatwg.org/multipage/webappapis.html#resolve-a-module-specifier + /// + /// `is_main` can be used to resolve from current working directory or + /// apply import map for child imports. fn resolve( &self, specifier: &str, referrer: &str, is_main: bool, - is_dyn_import: bool, ) -> Result; /// Given ModuleSpecifier, load its source code. + /// + /// `is_dyn_import` can be used to check permissions or deny + /// dynamic imports altogether. fn load( &self, module_specifier: &ModuleSpecifier, maybe_referrer: Option, + is_dyn_import: bool, ) -> Pin>; } @@ -121,10 +127,10 @@ impl RecursiveModuleLoad { fn add_root(&mut self) -> Result<(), ErrBox> { let module_specifier = match self.state { LoadState::ResolveMain(ref specifier, _) => { - self.loader.resolve(specifier, ".", true, false)? + self.loader.resolve(specifier, ".", true)? } LoadState::ResolveImport(ref specifier, ref referrer) => { - self.loader.resolve(specifier, referrer, false, true)? + self.loader.resolve(specifier, referrer, false)? } _ => unreachable!(), @@ -139,7 +145,10 @@ impl RecursiveModuleLoad { }) .boxed() } - _ => self.loader.load(&module_specifier, None).boxed(), + _ => self + .loader + .load(&module_specifier, None, self.is_dynamic_import()) + .boxed(), }; self.pending.push(load_fut); @@ -154,7 +163,10 @@ impl RecursiveModuleLoad { referrer: ModuleSpecifier, ) { if !self.is_pending.contains(&specifier) { - let fut = self.loader.load(&specifier, Some(referrer)); + let fut = + self + .loader + .load(&specifier, Some(referrer), self.is_dynamic_import()); self.pending.push(fut.boxed()); self.is_pending.insert(specifier); } @@ -192,7 +204,7 @@ pub struct ModuleInfo { pub main: bool, pub name: String, pub handle: v8::Global, - pub import_specifiers: Vec, + pub import_specifiers: Vec, } /// A symbolic module entity. @@ -264,7 +276,6 @@ impl ModuleNameMap { pub struct Modules { pub(crate) info: HashMap, by_name: ModuleNameMap, - pub(crate) specifier_cache: HashMap<(String, String), ModuleSpecifier>, } impl Modules { @@ -272,7 +283,6 @@ impl Modules { Self { info: HashMap::new(), by_name: ModuleNameMap::new(), - specifier_cache: HashMap::new(), } } @@ -280,20 +290,16 @@ impl Modules { self.by_name.get(name) } - pub fn get_children(&self, id: ModuleId) -> Option<&Vec> { + pub fn get_children(&self, id: ModuleId) -> Option<&Vec> { self.info.get(&id).map(|i| &i.import_specifiers) } - pub fn get_children2(&self, name: &str) -> Option<&Vec> { - self.get_id(name).and_then(|id| self.get_children(id)) - } - pub fn get_name(&self, id: ModuleId) -> Option<&String> { self.info.get(&id).map(|i| &i.name) } - pub fn is_registered(&self, name: &str) -> bool { - self.by_name.get(name).is_some() + pub fn is_registered(&self, specifier: &ModuleSpecifier) -> bool { + self.by_name.get(&specifier.to_string()).is_some() } pub fn register( @@ -302,7 +308,7 @@ impl Modules { name: &str, main: bool, handle: v8::Global, - import_specifiers: Vec, + import_specifiers: Vec, ) { let name = String::from(name); debug!("register_complete {}", name); @@ -334,30 +340,8 @@ impl Modules { self.info.get(&id) } - pub fn cache_specifier( - &mut self, - specifier: &str, - referrer: &str, - resolved_specifier: &ModuleSpecifier, - ) { - self.specifier_cache.insert( - (specifier.to_string(), referrer.to_string()), - resolved_specifier.to_owned(), - ); - } - - pub fn get_cached_specifier( - &self, - specifier: &str, - referrer: &str, - ) -> Option<&ModuleSpecifier> { - self - .specifier_cache - .get(&(specifier.to_string(), referrer.to_string())) - } - - pub fn deps(&self, url: &str) -> Option { - Deps::new(self, url) + pub fn deps(&self, module_specifier: &ModuleSpecifier) -> Option { + Deps::new(self, module_specifier) } } @@ -373,9 +357,12 @@ pub struct Deps { } impl Deps { - fn new(modules: &Modules, module_name: &str) -> Option { + fn new( + modules: &Modules, + module_specifier: &ModuleSpecifier, + ) -> Option { let mut seen = HashSet::new(); - Self::helper(&mut seen, "".to_string(), true, modules, module_name) + Self::helper(&mut seen, "".to_string(), true, modules, module_specifier) } fn helper( @@ -383,35 +370,37 @@ impl Deps { prefix: String, is_last: bool, modules: &Modules, - name: &str, // TODO(ry) rename url + module_specifier: &ModuleSpecifier, ) -> Option { - if seen.contains(name) { + let name = module_specifier.to_string(); + if seen.contains(&name) { Some(Deps { - name: name.to_string(), + name, prefix, deps: None, is_last, }) } else { - let children = modules.get_children2(name)?; + let mod_id = modules.get_id(&name)?; + let children = modules.get_children(mod_id).unwrap(); seen.insert(name.to_string()); let child_count = children.len(); let deps: Vec = children .iter() .enumerate() - .map(|(index, dep_name)| { + .map(|(index, dep_specifier)| { let new_is_last = index == child_count - 1; let mut new_prefix = prefix.clone(); new_prefix.push(if is_last { ' ' } else { '│' }); new_prefix.push(' '); - Self::helper(seen, new_prefix, new_is_last, modules, dep_name) + Self::helper(seen, new_prefix, new_is_last, modules, dep_specifier) }) // If any of the children are missing, return None. .collect::>()?; Some(Deps { - name: name.to_string(), + name, prefix, deps: Some(deps), is_last, @@ -565,7 +554,6 @@ mod tests { specifier: &str, referrer: &str, _is_root: bool, - _is_dyn_import: bool, ) -> Result { let referrer = if referrer == "." { "file:///" @@ -592,6 +580,7 @@ mod tests { &self, module_specifier: &ModuleSpecifier, _maybe_referrer: Option, + _is_dyn_import: bool, ) -> Pin> { let mut loads = self.loads.lock().unwrap(); loads.push(module_specifier.to_string()); @@ -666,10 +655,19 @@ mod tests { let d_id = modules.get_id("file:///d.js").unwrap(); assert_eq!( modules.get_children(a_id), - Some(&vec!["/b.js".to_string(), "/c.js".to_string()]) + Some(&vec![ + ModuleSpecifier::resolve_url("file:///b.js").unwrap(), + ModuleSpecifier::resolve_url("file:///c.js").unwrap() + ]) + ); + assert_eq!( + modules.get_children(b_id), + Some(&vec![ModuleSpecifier::resolve_url("file:///c.js").unwrap()]) + ); + assert_eq!( + modules.get_children(c_id), + Some(&vec![ModuleSpecifier::resolve_url("file:///d.js").unwrap()]) ); - assert_eq!(modules.get_children(b_id), Some(&vec!["/c.js".to_string()])); - assert_eq!(modules.get_children(c_id), Some(&vec!["/d.js".to_string()])); assert_eq!(modules.get_children(d_id), Some(&vec![])); } @@ -719,12 +717,16 @@ mod tests { assert_eq!( modules.get_children(circular1_id), - Some(&vec!["/circular2.js".to_string()]) + Some(&vec![ + ModuleSpecifier::resolve_url("file:///circular2.js").unwrap() + ]) ); assert_eq!( modules.get_children(circular2_id), - Some(&vec!["/circular3.js".to_string()]) + Some(&vec![ + ModuleSpecifier::resolve_url("file:///circular3.js").unwrap() + ]) ); assert!(modules.get_id("file:///circular3.js").is_some()); @@ -732,8 +734,8 @@ mod tests { assert_eq!( modules.get_children(circular3_id), Some(&vec![ - "/circular1.js".to_string(), - "/circular2.js".to_string() + ModuleSpecifier::resolve_url("file:///circular1.js").unwrap(), + ModuleSpecifier::resolve_url("file:///circular2.js").unwrap() ]) ); } @@ -923,17 +925,27 @@ mod tests { assert_eq!( modules.get_children(main_id), - Some(&vec!["/b.js".to_string(), "/c.js".to_string()]) + Some(&vec![ + ModuleSpecifier::resolve_url("file:///b.js").unwrap(), + ModuleSpecifier::resolve_url("file:///c.js").unwrap() + ]) + ); + assert_eq!( + modules.get_children(b_id), + Some(&vec![ModuleSpecifier::resolve_url("file:///c.js").unwrap()]) + ); + assert_eq!( + modules.get_children(c_id), + Some(&vec![ModuleSpecifier::resolve_url("file:///d.js").unwrap()]) ); - assert_eq!(modules.get_children(b_id), Some(&vec!["/c.js".to_string()])); - assert_eq!(modules.get_children(c_id), Some(&vec!["/d.js".to_string()])); assert_eq!(modules.get_children(d_id), Some(&vec![])); } #[test] fn empty_deps() { let modules = Modules::new(); - assert!(modules.deps("foo").is_none()); + let specifier = ModuleSpecifier::resolve_url("file:///foo").unwrap(); + assert!(modules.deps(&specifier).is_none()); } /* TODO(bartlomieju): reenable -- cgit v1.2.3