Implement async module loading in CLI (#2084)

cli/errors.rs

@@ -217,3 +217,13 @@ impl fmt::Display for RustOrJsError {
     }
   }
 }
+
+// TODO(ry) This is ugly. They are essentially the same type.
+impl From<deno::JSErrorOr<DenoError>> for RustOrJsError {
+  fn from(e: deno::JSErrorOr<DenoError>) -> Self {
+    match e {
+      deno::JSErrorOr::JSError(err) => RustOrJsError::Js(err),
+      deno::JSErrorOr::Other(err) => RustOrJsError::Rust(err),
+    }
+  }
+}

cli/main.rs

@@ -20,7 +20,6 @@ mod global_timer;
 mod http_body;
 mod http_util;
 pub mod js_errors;
-pub mod modules;
 pub mod msg;
 pub mod msg_util;
 pub mod ops;
@@ -37,6 +36,7 @@ pub mod worker;
 
 use crate::errors::RustOrJsError;
 use crate::state::ThreadSafeState;
+use crate::worker::root_specifier_to_url;
 use crate::worker::Worker;
 use futures::lazy;
 use futures::Future;
@@ -74,6 +74,49 @@ where
   }
 }
 
+// TODO(ry) Move this to main.rs
+pub fn print_file_info(worker: &Worker, url: &str) {
+  let maybe_out =
+    worker::fetch_module_meta_data_and_maybe_compile(&worker.state, url, ".");
+  if let Err(err) = maybe_out {
+    println!("{}", err);
+    return;
+  }
+  let out = maybe_out.unwrap();
+
+  println!("{} {}", ansi::bold("local:".to_string()), &(out.filename));
+
+  println!(
+    "{} {}",
+    ansi::bold("type:".to_string()),
+    msg::enum_name_media_type(out.media_type)
+  );
+
+  if out.maybe_output_code_filename.is_some() {
+    println!(
+      "{} {}",
+      ansi::bold("compiled:".to_string()),
+      out.maybe_output_code_filename.as_ref().unwrap(),
+    );
+  }
+
+  if out.maybe_source_map_filename.is_some() {
+    println!(
+      "{} {}",
+      ansi::bold("map:".to_string()),
+      out.maybe_source_map_filename.as_ref().unwrap()
+    );
+  }
+
+  let deps = worker.modules.deps(&out.module_name);
+  println!("{}{}", ansi::bold("deps:\n".to_string()), deps.name);
+  if let Some(ref depsdeps) = deps.deps {
+    for d in depsdeps {
+      println!("{}", d);
+    }
+  }
+}
+
 fn main() {
   #[cfg(windows)]
   ansi_term::enable_ansi_support().ok(); // For Windows 10
@@ -102,17 +145,18 @@ fn main() {
   let should_display_info = flags.info;
 
   let state = ThreadSafeState::new(flags, rest_argv, ops::op_selector_std);
-  let mut main_worker = Worker::new(
+  let mut worker = Worker::new(
     "main".to_string(),
     startup_data::deno_isolate_init(),
     state.clone(),
   );
 
-  let main_future = lazy(move || {
+  // TODO(ry) somehow combine the two branches below. They're very similar but
-    // Setup runtime.
+  // it's difficult to get the types to workout.
-    js_check(main_worker.execute("denoMain()"));
 
-    if state.flags.eval {
+  if state.flags.eval {
+    let main_future = lazy(move || {
+      js_check(worker.execute("denoMain()"));
       // Wrap provided script in async function so asynchronous methods
       // work. This is required until top-level await is not supported.
       let js_source = format!(
@@ -125,25 +169,51 @@ fn main() {
       );
       // ATM imports in `deno eval` are not allowed
       // TODO Support ES modules once Worker supports evaluating anonymous modules.
-      js_check(main_worker.execute(&js_source));
+      js_check(worker.execute(&js_source));
-    } else {
+      worker.then(|result| {
-      // Execute main module.
+        js_check(result);
-      if let Some(main_module) = state.main_module() {
+        Ok(())
-        debug!("main_module {}", main_module);
+      })
-        js_check(main_worker.execute_mod(&main_module, should_prefetch));
+    });
-        if should_display_info {
+    tokio_util::run(main_future);
-          // Display file info and exit. Do not run file
+  } else if let Some(main_module) = state.main_module() {
-          main_worker.print_file_info(&main_module);
+    // Normal situation of executing a module.
-          std::process::exit(0);
-        }
-      }
-    }
 
-    main_worker.then(|result| {
+    let main_future = lazy(move || {
-      js_check(result);
+      // Setup runtime.
-      Ok(())
+      js_check(worker.execute("denoMain()"));
-    })
+      debug!("main_module {}", main_module);
-  });
 
-  tokio_util::run(main_future);
+      let main_url = root_specifier_to_url(&main_module).unwrap();
+
+      worker
+        .execute_mod_async(&main_url, should_prefetch)
+        .and_then(move |worker| {
+          if should_display_info {
+            // Display file info and exit. Do not run file
+            print_file_info(&worker, &main_module);
+            std::process::exit(0);
+          }
+          worker.then(|result| {
+            js_check(result);
+            Ok(())
+          })
+        }).map_err(|(err, _worker)| print_err_and_exit(err))
+    });
+    tokio_util::run(main_future);
+  } else {
+    // REPL situation.
+    let main_future = lazy(move || {
+      // Setup runtime.
+      js_check(worker.execute("denoMain()"));
+      worker
+        .then(|result| {
+          js_check(result);
+          Ok(())
+        }).map_err(|(err, _worker): (RustOrJsError, Worker)| {
+          print_err_and_exit(err)
+        })
+    });
+    tokio_util::run(main_future);
+  }
 }

cli/modules.rs

@@ -1,263 +0,0 @@
-// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
-use crate::ansi;
-use crate::deno_dir::DenoDir;
-use crate::msg;
-use deno::deno_mod;
-use std::collections::HashMap;
-use std::collections::HashSet;
-use std::fmt;
-
-pub struct ModuleInfo {
-  name: String,
-  children: Vec<deno_mod>,
-}
-
-/// A symbolic module entity.
-pub enum SymbolicModule {
-  /// This module is an alias to another module.
-  /// This is useful such that multiple names could point to
-  /// the same underlying module (particularly due to redirects).
-  Alias(String),
-  /// This module associates with a V8 module by id.
-  Mod(deno_mod),
-}
-
-#[derive(Default)]
-/// Alias-able module name map
-pub struct ModuleNameMap {
-  inner: HashMap<String, SymbolicModule>,
-}
-
-impl ModuleNameMap {
-  pub fn new() -> Self {
-    ModuleNameMap {
-      inner: HashMap::new(),
-    }
-  }
-
-  /// Get the id of a module.
-  /// If this module is internally represented as an alias,
-  /// follow the alias chain to get the final module id.
-  pub fn get(&self, name: &str) -> Option<deno_mod> {
-    let mut mod_name = name;
-    loop {
-      let cond = self.inner.get(mod_name);
-      match cond {
-        Some(SymbolicModule::Alias(target)) => {
-          mod_name = target;
-        }
-        Some(SymbolicModule::Mod(mod_id)) => {
-          return Some(*mod_id);
-        }
-        _ => {
-          return None;
-        }
-      }
-    }
-  }
-
-  /// Insert a name assocated module id.
-  pub fn insert(&mut self, name: String, id: deno_mod) {
-    self.inner.insert(name, SymbolicModule::Mod(id));
-  }
-
-  /// Create an alias to another module.
-  pub fn alias(&mut self, name: String, target: String) {
-    self.inner.insert(name, SymbolicModule::Alias(target));
-  }
-}
-
-/// A collection of JS modules.
-#[derive(Default)]
-pub struct Modules {
-  pub info: HashMap<deno_mod, ModuleInfo>,
-  pub by_name: ModuleNameMap,
-}
-
-impl Modules {
-  pub fn new() -> Modules {
-    Self {
-      info: HashMap::new(),
-      by_name: ModuleNameMap::new(),
-    }
-  }
-
-  pub fn get_id(&self, name: &str) -> Option<deno_mod> {
-    self.by_name.get(name)
-  }
-
-  pub fn get_children(&self, id: deno_mod) -> Option<&Vec<deno_mod>> {
-    self.info.get(&id).map(|i| &i.children)
-  }
-
-  pub fn get_name(&self, id: deno_mod) -> 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 register(&mut self, id: deno_mod, name: &str) {
-    let name = String::from(name);
-    debug!("register {}", name);
-    self.by_name.insert(name.clone(), id);
-    self.info.insert(
-      id,
-      ModuleInfo {
-        name,
-        children: Vec::new(),
-      },
-    );
-  }
-
-  pub fn alias(&mut self, name: &str, target: &str) {
-    self.by_name.alias(name.to_owned(), target.to_owned());
-  }
-
-  pub fn resolve_cb(
-    &mut self,
-    deno_dir: &DenoDir,
-    specifier: &str,
-    referrer: deno_mod,
-  ) -> deno_mod {
-    debug!("resolve_cb {}", specifier);
-
-    let maybe_info = self.info.get_mut(&referrer);
-    if maybe_info.is_none() {
-      debug!("cant find referrer {}", referrer);
-      return 0;
-    }
-    let info = maybe_info.unwrap();
-    let referrer_name = &info.name;
-    let r = deno_dir.resolve_module(specifier, referrer_name);
-    if let Err(err) = r {
-      debug!("potentially swallowed err: {}", err);
-      return 0;
-    }
-    let (name, _local_filename) = r.unwrap();
-
-    if let Some(child_id) = self.by_name.get(&name) {
-      info.children.push(child_id);
-      return child_id;
-    } else {
-      return 0;
-    }
-  }
-
-  pub fn print_file_info(&self, deno_dir: &DenoDir, filename: String) {
-    // TODO Note the --reload flag is ignored here.
-    let maybe_out = deno_dir.fetch_module_meta_data(&filename, ".", true);
-    if maybe_out.is_err() {
-      println!("{}", maybe_out.unwrap_err());
-      return;
-    }
-    let out = maybe_out.unwrap();
-
-    println!("{} {}", ansi::bold("local:".to_string()), &(out.filename));
-    println!(
-      "{} {}",
-      ansi::bold("type:".to_string()),
-      msg::enum_name_media_type(out.media_type)
-    );
-    if out.maybe_output_code_filename.is_some() {
-      println!(
-        "{} {}",
-        ansi::bold("compiled:".to_string()),
-        out.maybe_output_code_filename.as_ref().unwrap(),
-      );
-    }
-    if out.maybe_source_map_filename.is_some() {
-      println!(
-        "{} {}",
-        ansi::bold("map:".to_string()),
-        out.maybe_source_map_filename.as_ref().unwrap()
-      );
-    }
-
-    let deps = Deps::new(self, &out.module_name);
-    println!("{}{}", ansi::bold("deps:\n".to_string()), deps.name);
-    if let Some(ref depsdeps) = deps.deps {
-      for d in depsdeps {
-        println!("{}", d);
-      }
-    }
-  }
-}
-
-pub struct Deps {
-  pub name: String,
-  pub deps: Option<Vec<Deps>>,
-  prefix: String,
-  is_last: bool,
-}
-
-impl Deps {
-  pub fn new(modules: &Modules, module_name: &str) -> Deps {
-    let mut seen = HashSet::new();
-    let id = modules.get_id(module_name).unwrap();
-    Self::helper(&mut seen, "".to_string(), true, modules, id)
-  }
-
-  fn helper(
-    seen: &mut HashSet<deno_mod>,
-    prefix: String,
-    is_last: bool,
-    modules: &Modules,
-    id: deno_mod,
-  ) -> Deps {
-    let name = modules.get_name(id).unwrap().to_string();
-    if seen.contains(&id) {
-      Deps {
-        name,
-        prefix,
-        deps: None,
-        is_last,
-      }
-    } else {
-      seen.insert(id);
-      let child_ids = modules.get_children(id).unwrap();
-      let child_count = child_ids.iter().count();
-      let deps = child_ids
-        .iter()
-        .enumerate()
-        .map(|(index, dep_id)| {
-          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_id)
-        }).collect();
-      Deps {
-        name,
-        prefix,
-        deps: Some(deps),
-        is_last,
-      }
-    }
-  }
-}
-
-impl fmt::Display for Deps {
-  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-    let mut has_children = false;
-    if let Some(ref deps) = self.deps {
-      has_children = !deps.is_empty();
-    }
-    write!(
-      f,
-      "{}{}─{} {}",
-      self.prefix,
-      if self.is_last { "└" } else { "├" },
-      if has_children { "┬" } else { "─" },
-      self.name
-    )?;
-
-    if let Some(ref deps) = self.deps {
-      for d in deps {
-        write!(f, "\n{}", d)?;
-      }
-    }
-    Ok(())
-  }
-}

cli/ops.rs

@@ -19,6 +19,7 @@ use crate::state::ThreadSafeState;
 use crate::tokio_util;
 use crate::tokio_write;
 use crate::version;
+use crate::worker::root_specifier_to_url;
 use crate::worker::Worker;
 use deno::deno_buf;
 use deno::js_check;
@@ -1878,9 +1879,14 @@ fn op_create_worker(
       Worker::new(name, startup_data::deno_isolate_init(), child_state);
     js_check(worker.execute("denoMain()"));
     js_check(worker.execute("workerMain()"));
-    let result = worker.execute_mod(specifier, false);
+
+    let specifier_url =
+      root_specifier_to_url(specifier).map_err(DenoError::from)?;
+
+    // TODO(ry) Use execute_mod_async here.
+    let result = worker.execute_mod(&specifier_url, false);
     match result {
-      Ok(_) => {
+      Ok(worker) => {
         let mut workers_tl = parent_state.workers.lock().unwrap();
         workers_tl.insert(rid, worker.shared());
         let builder = &mut FlatBufferBuilder::new();
@@ -1898,8 +1904,10 @@ fn op_create_worker(
           },
         ))
       }
-      Err(errors::RustOrJsError::Js(_)) => Err(errors::worker_init_failed()),
+      Err((errors::RustOrJsError::Js(_), _worker)) => {
-      Err(errors::RustOrJsError::Rust(err)) => Err(err),
+        Err(errors::worker_init_failed())
+      }
+      Err((errors::RustOrJsError::Rust(err), _worker)) => Err(err),
     }
   }()))
 }

cli/state.rs

@@ -3,7 +3,6 @@ use crate::deno_dir;
 use crate::errors::DenoResult;
 use crate::flags;
 use crate::global_timer::GlobalTimer;
-use crate::modules::Modules;
 use crate::ops;
 use crate::permissions::DenoPermissions;
 use crate::resources;
@@ -54,7 +53,6 @@ pub struct State {
   pub permissions: DenoPermissions,
   pub flags: flags::DenoFlags,
   pub metrics: Metrics,
-  pub modules: Mutex<Modules>,
   pub worker_channels: Mutex<WorkerChannels>,
   pub global_timer: Mutex<GlobalTimer>,
   pub workers: Mutex<UserWorkerTable>,
@@ -106,7 +104,6 @@ impl ThreadSafeState {
       permissions: DenoPermissions::from_flags(&flags),
       flags,
       metrics: Metrics::default(),
-      modules: Mutex::new(Modules::new()),
       worker_channels: Mutex::new(internal_channels),
       global_timer: Mutex::new(GlobalTimer::new()),
       workers: Mutex::new(UserWorkerTable::new()),

cli/worker.rs

@@ -9,19 +9,21 @@ use crate::msg;
 use crate::state::ThreadSafeState;
 use crate::tokio_util;
 use deno;
-use deno::deno_mod;
 use deno::JSError;
+use deno::Loader;
 use deno::StartupData;
 use futures::future::Either;
 use futures::Async;
 use futures::Future;
 use std::sync::atomic::Ordering;
+use url::Url;
 
 /// Wraps deno::Isolate to provide source maps, ops for the CLI, and
 /// high-level module loading
 pub struct Worker {
   inner: deno::Isolate<ThreadSafeState>,
-  state: ThreadSafeState,
+  pub modules: deno::Modules,
+  pub state: ThreadSafeState,
 }
 
 impl Worker {
@@ -33,6 +35,7 @@ impl Worker {
     let state_ = state.clone();
     Self {
       inner: deno::Isolate::new(startup_data, state_),
+      modules: deno::Modules::new(),
       state,
     }
   }
@@ -52,138 +55,44 @@ impl Worker {
     self.inner.execute(js_filename, js_source)
   }
 
-  // TODO(ry) make this return a future.
+  /// Consumes worker. Executes the provided JavaScript module.
-  fn mod_load_deps(&self, id: deno_mod) -> Result<(), RustOrJsError> {
+  pub fn execute_mod_async(
-    // basically iterate over the imports, start loading them.
+    self,
-
+    js_url: &Url,
-    let referrer_name = {
+    is_prefetch: bool,
-      let g = self.state.modules.lock().unwrap();
+  ) -> impl Future<Item = Self, Error = (RustOrJsError, Self)> {
-      g.get_name(id).unwrap().clone()
+    let recursive_load = deno::RecursiveLoad::new(js_url.as_str(), self);
-    };
+    recursive_load.and_then(
-
+      move |(id, mut self_)| -> Result<Self, (deno::JSErrorOr<DenoError>, Self)> {
-    for specifier in self.inner.mod_get_imports(id) {
+        if is_prefetch {
-      let (name, _local_filename) = self
+          Ok(self_)
-        .state
+        } else {
-        .dir
+          let result = self_.inner.mod_evaluate(id);
-        .resolve_module(&specifier, &referrer_name)
+          if let Err(err) = result {
-        .map_err(DenoError::from)
+            Err((deno::JSErrorOr::JSError(err), self_))
-        .map_err(RustOrJsError::from)?;
+          } else {
-
+            Ok(self_)
-      debug!("mod_load_deps {}", name);
+          }
-
-      if !self.state.modules.lock().unwrap().is_registered(&name) {
-        let out = fetch_module_meta_data_and_maybe_compile(
-          &self.state,
-          &specifier,
-          &referrer_name,
-        )?;
-        let child_id = self.mod_new_and_register(
-          false,
-          &out.module_name.clone(),
-          &out.js_source(),
-        )?;
-
-        // The resolved module is an alias to another module (due to redirects).
-        // Save such alias to the module map.
-        if out.module_redirect_source_name.is_some() {
-          self.mod_alias(
-            &out.module_redirect_source_name.clone().unwrap(),
-            &out.module_name,
-          );
         }
-
+      },
-        self.mod_load_deps(child_id)?;
+    )
-      }
+    .map_err(|(err, self_)| {
-    }
+      // Convert to RustOrJsError AND apply_source_map.
-
+      let err = match err {
-    Ok(())
+        deno::JSErrorOr::JSError(err) => RustOrJsError::Js(self_.apply_source_map(err)),
+        deno::JSErrorOr::Other(err) => RustOrJsError::Rust(err),
+      };
+      (err, self_)
+    })
   }
 
-  /// Executes the provided JavaScript module.
+  /// Consumes worker. Executes the provided JavaScript module.
   pub fn execute_mod(
-    &mut self,
+    self,
-    js_filename: &str,
+    js_url: &Url,
     is_prefetch: bool,
-  ) -> Result<(), RustOrJsError> {
+  ) -> Result<Self, (RustOrJsError, Self)> {
-    // TODO move state::execute_mod impl here.
+    tokio_util::block_on(self.execute_mod_async(js_url, is_prefetch))
-    self
-      .execute_mod_inner(js_filename, is_prefetch)
-      .map_err(|err| match err {
-        RustOrJsError::Js(err) => RustOrJsError::Js(self.apply_source_map(err)),
-        x => x,
-      })
-  }
-
-  /// High-level way to execute modules.
-  /// This will issue HTTP requests and file system calls.
-  /// Blocks. TODO(ry) Don't block.
-  fn execute_mod_inner(
-    &mut self,
-    url: &str,
-    is_prefetch: bool,
-  ) -> Result<(), RustOrJsError> {
-    let out = fetch_module_meta_data_and_maybe_compile(&self.state, url, ".")
-      .map_err(RustOrJsError::from)?;
-
-    // Be careful.
-    // url might not match the actual out.module_name
-    // due to the mechanism of redirection.
-
-    let id = self
-      .mod_new_and_register(true, &out.module_name.clone(), &out.js_source())
-      .map_err(RustOrJsError::from)?;
-
-    // The resolved module is an alias to another module (due to redirects).
-    // Save such alias to the module map.
-    if out.module_redirect_source_name.is_some() {
-      self.mod_alias(
-        &out.module_redirect_source_name.clone().unwrap(),
-        &out.module_name,
-      );
-    }
-
-    self.mod_load_deps(id)?;
-
-    let state = self.state.clone();
-
-    let mut resolve = move |specifier: &str, referrer: deno_mod| -> deno_mod {
-      state.metrics.resolve_count.fetch_add(1, Ordering::Relaxed);
-      let mut modules = state.modules.lock().unwrap();
-      modules.resolve_cb(&state.dir, specifier, referrer)
-    };
-
-    self
-      .inner
-      .mod_instantiate(id, &mut resolve)
-      .map_err(RustOrJsError::from)?;
-    if !is_prefetch {
-      self.inner.mod_evaluate(id).map_err(RustOrJsError::from)?;
-    }
-    Ok(())
-  }
-
-  /// Wraps Isolate::mod_new but registers with modules.
-  fn mod_new_and_register(
-    &self,
-    main: bool,
-    name: &str,
-    source: &str,
-  ) -> Result<deno_mod, JSError> {
-    let id = self.inner.mod_new(main, name, source)?;
-    self.state.modules.lock().unwrap().register(id, &name);
-    Ok(id)
-  }
-
-  /// Create an alias for another module.
-  /// The alias could later be used to grab the module
-  /// which `target` points to.
-  fn mod_alias(&self, name: &str, target: &str) {
-    self.state.modules.lock().unwrap().alias(name, target);
-  }
-
-  pub fn print_file_info(&self, module: &str) {
-    let m = self.state.modules.lock().unwrap();
-    m.print_file_info(&self.state.dir, module.to_string());
   }
 
   /// Applies source map to the error.
@@ -192,6 +101,90 @@ impl Worker {
   }
 }
 
+// https://html.spec.whatwg.org/multipage/webappapis.html#resolve-a-module-specifier
+// TODO(ry) Add tests.
+// TODO(ry) Move this to core?
+pub fn resolve_module_spec(
+  specifier: &str,
+  base: &str,
+) -> Result<String, url::ParseError> {
+  // 1. Apply the URL parser to specifier. If the result is not failure, return
+  //    the result.
+  // let specifier = parse_local_or_remote(specifier)?.to_string();
+  if let Ok(specifier_url) = Url::parse(specifier) {
+    return Ok(specifier_url.to_string());
+  }
+
+  // 2. If specifier does not start with the character U+002F SOLIDUS (/), the
+  //    two-character sequence U+002E FULL STOP, U+002F SOLIDUS (./), or the
+  //    three-character sequence U+002E FULL STOP, U+002E FULL STOP, U+002F
+  //    SOLIDUS (../), return failure.
+  if !specifier.starts_with("/")
+    && !specifier.starts_with("./")
+    && !specifier.starts_with("../")
+  {
+    // TODO(ry) This is (probably) not the correct error to return here.
+    return Err(url::ParseError::RelativeUrlWithCannotBeABaseBase);
+  }
+
+  // 3. Return the result of applying the URL parser to specifier with base URL
+  //    as the base URL.
+  let base_url = Url::parse(base)?;
+  let u = base_url.join(&specifier)?;
+  Ok(u.to_string())
+}
+
+/// Takes a string representing a path or URL to a module, but of the type
+/// passed through the command-line interface for the main module. This is
+/// slightly different than specifiers used in import statements: "foo.js" for
+/// example is allowed here, whereas in import statements a leading "./" is
+/// required ("./foo.js"). This function is aware of the current working
+/// directory and returns an absolute URL.
+pub fn root_specifier_to_url(
+  root_specifier: &str,
+) -> Result<Url, url::ParseError> {
+  let maybe_url = Url::parse(root_specifier);
+  if let Ok(url) = maybe_url {
+    Ok(url)
+  } else {
+    let cwd = std::env::current_dir().unwrap();
+    let base = Url::from_directory_path(cwd).unwrap();
+    base.join(root_specifier)
+  }
+}
+
+impl Loader for Worker {
+  type Dispatch = ThreadSafeState;
+  type Error = DenoError;
+
+  fn resolve(specifier: &str, referrer: &str) -> Result<String, Self::Error> {
+    resolve_module_spec(specifier, referrer)
+      .map_err(|url_err| DenoError::from(url_err))
+  }
+
+  /// Given an absolute url, load its source code.
+  fn load(&mut self, url: &str) -> Box<deno::SourceCodeFuture<Self::Error>> {
+    self
+      .state
+      .metrics
+      .resolve_count
+      .fetch_add(1, Ordering::SeqCst);
+    Box::new(
+      fetch_module_meta_data_and_maybe_compile_async(&self.state, url, ".")
+        .map_err(|err| {
+          eprintln!("{}", err);
+          err
+        }).map(|module_meta_data| module_meta_data.js_source()),
+    )
+  }
+
+  fn isolate_and_modules<'a: 'b + 'c, 'b, 'c>(
+    &'a mut self,
+  ) -> (&'b mut deno::Isolate<Self::Dispatch>, &'c mut deno::Modules) {
+    (&mut self.inner, &mut self.modules)
+  }
+}
+
 impl Future for Worker {
   type Item = ();
   type Error = JSError;
@@ -236,7 +229,7 @@ fn fetch_module_meta_data_and_maybe_compile_async(
     })
 }
 
-fn fetch_module_meta_data_and_maybe_compile(
+pub fn fetch_module_meta_data_and_maybe_compile(
   state: &ThreadSafeState,
   specifier: &str,
   referrer: &str,
@@ -260,46 +253,54 @@ mod tests {
   use std::sync::atomic::Ordering;
 
   #[test]
-  fn execute_mod() {
+  fn execute_mod_esm_imports_a() {
     let filename = std::env::current_dir()
       .unwrap()
       .join("tests/esm_imports_a.js");
-    let filename = filename.to_str().unwrap().to_string();
+    let js_url = Url::from_file_path(filename).unwrap();
 
-    let argv = vec![String::from("./deno"), filename.clone()];
+    let argv = vec![String::from("./deno"), js_url.to_string()];
     let (flags, rest_argv) = flags::set_flags(argv).unwrap();
 
     let state = ThreadSafeState::new(flags, rest_argv, op_selector_std);
     let state_ = state.clone();
     tokio_util::run(lazy(move || {
-      let mut worker =
+      let worker = Worker::new("TEST".to_string(), StartupData::None, state);
-        Worker::new("TEST".to_string(), StartupData::None, state);
+      let result = worker.execute_mod(&js_url, false);
-      if let Err(err) = worker.execute_mod(&filename, false) {
+      let worker = match result {
-        eprintln!("execute_mod err {:?}", err);
+        Err((err, worker)) => {
-      }
+          eprintln!("execute_mod err {:?}", err);
+          worker
+        }
+        Ok(worker) => worker,
+      };
       tokio_util::panic_on_error(worker)
     }));
 
     let metrics = &state_.metrics;
-    assert_eq!(metrics.resolve_count.load(Ordering::SeqCst), 1);
+    assert_eq!(metrics.resolve_count.load(Ordering::SeqCst), 2);
   }
 
   #[test]
   fn execute_mod_circular() {
     let filename = std::env::current_dir().unwrap().join("tests/circular1.js");
-    let filename = filename.to_str().unwrap().to_string();
+    let js_url = Url::from_file_path(filename).unwrap();
 
-    let argv = vec![String::from("./deno"), filename.clone()];
+    let argv = vec![String::from("./deno"), js_url.to_string()];
     let (flags, rest_argv) = flags::set_flags(argv).unwrap();
 
     let state = ThreadSafeState::new(flags, rest_argv, op_selector_std);
     let state_ = state.clone();
     tokio_util::run(lazy(move || {
-      let mut worker =
+      let worker = Worker::new("TEST".to_string(), StartupData::None, state);
-        Worker::new("TEST".to_string(), StartupData::None, state);
+      let result = worker.execute_mod(&js_url, false);
-      if let Err(err) = worker.execute_mod(&filename, false) {
+      let worker = match result {
-        eprintln!("execute_mod err {:?}", err);
+        Err((err, worker)) => {
-      }
+          eprintln!("execute_mod err {:?}", err);
+          worker
+        }
+        Ok(worker) => worker,
+      };
       tokio_util::panic_on_error(worker)
     }));
 
@@ -372,7 +373,7 @@ mod tests {
     tokio_util::init(|| {
       let mut worker = create_test_worker();
       js_check(
-        worker.execute("onmessage = () => { delete window['onmessage']; }"),
+        worker.execute("onmessage = () => { delete window.onmessage; }"),
       );
 
       let resource = worker.state.resource.clone();
@@ -400,4 +401,23 @@ mod tests {
       assert_eq!(resources::get_type(rid), None);
     })
   }
+
+  #[test]
+  fn execute_mod_resolve_error() {
+    // "foo" is not a vailid module specifier so this should return an error.
+    let worker = create_test_worker();
+    let js_url = root_specifier_to_url("does-not-exist").unwrap();
+    let result = worker.execute_mod_async(&js_url, false).wait();
+    assert!(result.is_err());
+  }
+
+  #[test]
+  fn execute_mod_002_hello() {
+    // This assumes cwd is project root (an assumption made throughout the
+    // tests).
+    let worker = create_test_worker();
+    let js_url = root_specifier_to_url("./tests/002_hello.ts").unwrap();
+    let result = worker.execute_mod_async(&js_url, false).wait();
+    assert!(result.is_ok());
+  }
 }

core/modules.rs

@@ -12,8 +12,11 @@ use crate::libdeno::deno_mod;
 use futures::Async;
 use futures::Future;
 use futures::Poll;
-use std::collections::hash_map::Entry;
 use std::collections::HashMap;
+use std::collections::HashSet;
+use std::error::Error;
+use std::fmt;
+use std::marker::PhantomData;
 
 pub type SourceCodeFuture<E> = dyn Future<Item = String, Error = E> + Send;
 
@@ -25,7 +28,7 @@ pub trait Loader {
   /// 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
-  fn resolve(specifier: &str, referrer: &str) -> String;
+  fn resolve(specifier: &str, referrer: &str) -> Result<String, Self::Error>;
 
   /// Given an absolute url, load its source code.
   fn load(&mut self, url: &str) -> Box<SourceCodeFuture<Self::Error>>;
@@ -45,135 +48,185 @@ pub trait Loader {
   }
 }
 
+struct PendingLoad<E: Error> {
+  url: String,
+  is_root: bool,
+  source_code_future: Box<SourceCodeFuture<E>>,
+}
+
+/// This future is used to implement parallel async module loading without
+/// complicating the Isolate API. Note that RecursiveLoad will take ownership of
+/// an Isolate during load.
+pub struct RecursiveLoad<L: Loader> {
+  loader: Option<L>,
+  pending: Vec<PendingLoad<L::Error>>,
+  is_pending: HashSet<String>,
+  phantom: PhantomData<L>,
+  // TODO(ry) The following can all be combined into a single enum State type.
+  root: Option<String>,           // Empty before polled.
+  root_specifier: Option<String>, // Empty after first poll
+  root_id: Option<deno_mod>,
+}
+
+impl<L: Loader> RecursiveLoad<L> {
+  /// Starts a new parallel load of the given URL.
+  pub fn new(url: &str, loader: L) -> Self {
+    Self {
+      loader: Some(loader),
+      root: None,
+      root_specifier: Some(url.to_string()),
+      root_id: None,
+      pending: Vec::new(),
+      is_pending: HashSet::new(),
+      phantom: PhantomData,
+    }
+  }
+
+  fn take_loader(&mut self) -> L {
+    self.loader.take().unwrap()
+  }
+
+  fn add(
+    &mut self,
+    specifier: &str,
+    referrer: &str,
+    parent_id: Option<deno_mod>,
+  ) -> Result<String, L::Error> {
+    let url = L::resolve(specifier, referrer)?;
+
+    let is_root = if let Some(parent_id) = parent_id {
+      let loader = self.loader.as_mut().unwrap();
+      let modules = loader.modules();
+      modules.add_child(parent_id, &url);
+      false
+    } else {
+      true
+    };
+
+    if !self.is_pending.contains(&url) {
+      self.is_pending.insert(url.clone());
+      let source_code_future = {
+        let loader = self.loader.as_mut().unwrap();
+        loader.load(&url)
+      };
+      self.pending.push(PendingLoad {
+        url: url.clone(),
+        source_code_future,
+        is_root,
+      });
+    }
+
+    Ok(url)
+  }
+}
+
 // TODO(ry) This is basically the same thing as RustOrJsError. They should be
 // combined into one type.
-pub enum Either<E> {
+#[derive(Debug, PartialEq)]
+pub enum JSErrorOr<E> {
   JSError(JSError),
   Other(E),
 }
 
-/// This future is used to implement parallel async module loading without
+impl<L: Loader> Future for RecursiveLoad<L> {
-/// complicating the Isolate API.
+  type Item = (deno_mod, L);
-pub struct RecursiveLoad<'l, L: Loader> {
+  type Error = (JSErrorOr<L::Error>, L);
-  loader: &'l mut L,
-  pending: HashMap<String, Box<SourceCodeFuture<<L as Loader>::Error>>>,
-  root: String,
-}
-
-impl<'l, L: Loader> RecursiveLoad<'l, L> {
-  /// Starts a new parallel load of the given URL.
-  pub fn new(url: &str, loader: &'l mut L) -> Self {
-    let root = L::resolve(url, ".");
-    let mut recursive_load = Self {
-      loader,
-      root: root.clone(),
-      pending: HashMap::new(),
-    };
-    recursive_load
-      .pending
-      .insert(root.clone(), recursive_load.loader.load(&root));
-    recursive_load
-  }
-}
-
-impl<'l, L: Loader> Future for RecursiveLoad<'l, L> {
-  type Item = deno_mod;
-  type Error = Either<L::Error>;
 
   fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
-    let loader = &mut self.loader;
+    if self.root.is_none() && self.root_specifier.is_some() {
-    let pending = &mut self.pending;
+      let s = self.root_specifier.take().unwrap();
-    let root = self.root.as_str();
+      match self.add(&s, ".", None) {
+        Err(err) => {
+          return Err((JSErrorOr::Other(err), self.take_loader()));
+        }
+        Ok(root) => {
+          self.root = Some(root);
+        }
+      }
+    }
+    assert!(self.root_specifier.is_none());
+    assert!(self.root.is_some());
 
-    // Find all finished futures (those that are ready or that have errored).
+    let mut i = 0;
-    // Turn it into a list of (url, source_code) tuples.
+    while i < self.pending.len() {
-    let mut finished_loads: Vec<(String, String)> = pending
+      let pending = &mut self.pending[i];
-      .iter_mut()
+      match pending.source_code_future.poll() {
-      .filter_map(|(url, fut)| match fut.poll() {
+        Err(err) => {
-        Ok(Async::NotReady) => None,
+          return Err((JSErrorOr::Other(err), self.take_loader()));
-        Ok(Async::Ready(source_code)) => Some(Ok((url.clone(), source_code))),
+        }
-        Err(err) => Some(Err(Either::Other(err))),
+        Ok(Async::NotReady) => {
-      }).collect::<Result<_, _>>()?;
+          i += 1;
+        }
+        Ok(Async::Ready(source_code)) => {
+          // We have completed loaded one of the modules.
+          let completed = self.pending.remove(i);
 
-    while !finished_loads.is_empty() {
+          let result = {
-      // Instantiate and register the loaded modules, and discover new imports.
+            let loader = self.loader.as_mut().unwrap();
-      // Build a list of (parent_url, Vec<child_url>) tuples.
+            let isolate = loader.isolate();
-      let parent_and_child_urls: Vec<(&str, Vec<String>)> = finished_loads
+            isolate.mod_new(completed.is_root, &completed.url, &source_code)
-        .iter()
+          };
-        .map(|(url, source_code)| {
+          if let Err(err) = result {
-          // Instantiate and register the module.
+            return Err((JSErrorOr::JSError(err), self.take_loader()));
-          let mod_id = loader
+          }
-            .isolate()
+          let mod_id = result.unwrap();
-            .mod_new(url == root, &url, &source_code)
+          if completed.is_root {
-            .map_err(Either::JSError)?;
+            assert!(self.root_id.is_none());
-          loader.modules().register(mod_id, &url);
+            self.root_id = Some(mod_id);
+          }
 
-          // Find child modules imported by the newly registered module.
+          let referrer = &completed.url.clone();
-          // Resolve all child import specifiers to URLs. Register all
-          // imports as a children; however any modules that are already
-          // known to the modules registry won't be stored in `child_urls`.
-          let child_urls: Vec<String> = loader
-            .isolate()
-            .mod_get_imports(mod_id)
-            .into_iter()
-            .map(|specifier| L::resolve(&specifier, &url))
-            .filter(|child_url| !loader.modules().add_child(mod_id, &child_url))
-            .collect();
-          Ok((url.as_str(), child_urls))
-        }).collect::<Result<_, _>>()?;
 
-      // Make updates to the `pending` hash map. If we find any more finished
+          {
-      // futures, we'll loop and process `finished_loads` again.
+            let loader = self.loader.as_mut().unwrap();
-      finished_loads = parent_and_child_urls
+            let modules = loader.modules();
-        .into_iter()
+            modules.register(mod_id, &completed.url);
-        .flat_map(|(url, child_urls)| {
+          }
-          // Remove the parent module url that is done loading from `pending`.
-          pending.remove(url);
 
-          // Look for newly discovered child module imports.
+          // Now we must iterate over all imports of the module and load them.
-          child_urls
+          let imports = {
-            .into_iter()
+            let loader = self.loader.as_mut().unwrap();
-            .filter_map(|child_url| {
+            let isolate = loader.isolate();
-              // If the url isn't present in the pending load table, create a
+            isolate.mod_get_imports(mod_id)
-              // load future and associate it with the url in the hash map.
+          };
-              match pending.entry(child_url.clone()) {
+          for specifier in imports {
-                Entry::Occupied(_) => None,
+            self
-                Entry::Vacant(entry) => {
+              .add(&specifier, referrer, Some(mod_id))
-                  Some(entry.insert(Box::new(loader.load(&child_url))).poll())
+              .map_err(|e| (JSErrorOr::Other(e), self.take_loader()))?;
-                }
+          }
-              }
+        }
-              // Immediately poll any newly created futures and gather the
+      }
-              // ones that are immediately ready or errored.
-              .and_then(|poll_result| match poll_result {
-                Ok(Async::NotReady) => None,
-                Ok(Async::Ready(source_code)) => {
-                  Some(Ok((child_url.clone(), source_code)))
-                }
-                Err(err) => Some(Err(Either::Other(err))),
-              })
-            }).collect::<Vec<_>>()
-        }).collect::<Result<_, _>>()?;
     }
 
     if !self.pending.is_empty() {
       return Ok(Async::NotReady);
     }
 
+    let root_id = self.root_id.unwrap().clone();
+    let mut loader = self.take_loader();
     let (isolate, modules) = loader.isolate_and_modules();
-    let root_id = modules.get_id(root).unwrap();
+    let result = {
-    let mut resolve = |specifier: &str, referrer_id: deno_mod| -> deno_mod {
+      let mut resolve_cb =
-      let referrer = modules.get_name(referrer_id).unwrap();
+        |specifier: &str, referrer_id: deno_mod| -> deno_mod {
-      let url = L::resolve(specifier, referrer);
+          let referrer = modules.get_name(referrer_id).unwrap();
-      match modules.get_id(&url) {
+          match L::resolve(specifier, &referrer) {
-        Some(id) => id,
+            Ok(url) => match modules.get_id(&url) {
-        None => 0,
+              Some(id) => id,
-      }
+              None => 0,
-    };
+            },
-    isolate
+            // We should have already resolved and loaded this module, so
-      .mod_instantiate(root_id, &mut resolve)
+            // resolve() will not fail this time.
-      .map_err(Either::JSError)?;
+            Err(_err) => unreachable!(),
+          }
+        };
 
-    Ok(Async::Ready(root_id))
+      isolate.mod_instantiate(root_id, &mut resolve_cb)
+    };
+
+    match result {
+      Err(err) => Err((JSErrorOr::JSError(err), loader)),
+      Ok(()) => Ok(Async::Ready((root_id, loader))),
+    }
   }
 }
 
@@ -220,21 +273,23 @@ impl Modules {
     self.get_id(name).and_then(|id| self.get_children(id))
   }
 
-  pub fn get_name(&self, id: deno_mod) -> Option<&str> {
+  pub fn get_name(&self, id: deno_mod) -> Option<&String> {
-    self.info.get(&id).map(|i| i.name.as_str())
+    self.info.get(&id).map(|i| &i.name)
   }
 
   pub fn is_registered(&self, name: &str) -> bool {
     self.by_name.get(name).is_some()
   }
 
-  // Returns true if the child name is a registered module, false otherwise.
   pub fn add_child(&mut self, parent_id: deno_mod, child_name: &str) -> bool {
-    let parent = self.info.get_mut(&parent_id).unwrap();
+    self
-    if !parent.has_child(&child_name) {
+      .info
-      parent.children.push(child_name.to_string());
+      .get_mut(&parent_id)
-    }
+      .map(move |i| {
-    self.is_registered(child_name)
+        if !i.has_child(&child_name) {
+          i.children.push(child_name.to_string());
+        }
+      }).is_some()
   }
 
   pub fn register(&mut self, id: deno_mod, name: &str) {
@@ -252,6 +307,86 @@ impl Modules {
       },
     );
   }
+
+  pub fn deps(&self, url: &str) -> Deps {
+    Deps::new(self, url)
+  }
+}
+
+pub struct Deps {
+  pub name: String,
+  pub deps: Option<Vec<Deps>>,
+  prefix: String,
+  is_last: bool,
+}
+
+impl Deps {
+  pub fn new(modules: &Modules, module_name: &str) -> Deps {
+    let mut seen = HashSet::new();
+    Self::helper(&mut seen, "".to_string(), true, modules, module_name)
+  }
+
+  fn helper(
+    seen: &mut HashSet<String>,
+    prefix: String,
+    is_last: bool,
+    modules: &Modules,
+    name: &str, // TODO(ry) rename url
+  ) -> Deps {
+    if seen.contains(name) {
+      Deps {
+        name: name.to_string(),
+        prefix,
+        deps: None,
+        is_last,
+      }
+    } else {
+      seen.insert(name.to_string());
+      let children = modules.get_children2(name).unwrap();
+      let child_count = children.iter().count();
+      let deps = children
+        .iter()
+        .enumerate()
+        .map(|(index, dep_name)| {
+          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)
+        }).collect();
+      Deps {
+        name: name.to_string(),
+        prefix,
+        deps: Some(deps),
+        is_last,
+      }
+    }
+  }
+}
+
+impl fmt::Display for Deps {
+  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+    let mut has_children = false;
+    if let Some(ref deps) = self.deps {
+      has_children = !deps.is_empty();
+    }
+    write!(
+      f,
+      "{}{}─{} {}",
+      self.prefix,
+      if self.is_last { "└" } else { "├" },
+      if has_children { "┬" } else { "─" },
+      self.name
+    )?;
+
+    if let Some(ref deps) = self.deps {
+      for d in deps {
+        write!(f, "\n{}", d)?;
+      }
+    }
+    Ok(())
+  }
 }
 
 #[cfg(test)]
@@ -259,6 +394,7 @@ mod tests {
   use super::*;
   use crate::isolate::js_check;
   use crate::isolate::tests::*;
+  use std::fmt;
 
   struct MockLoader {
     pub loads: Vec<String>,
@@ -278,28 +414,86 @@ mod tests {
     }
   }
 
+  fn mock_source_code(url: &str) -> Option<&'static str> {
+    match url {
+      "a.js" => Some(A_SRC),
+      "b.js" => Some(B_SRC),
+      "c.js" => Some(C_SRC),
+      "d.js" => Some(D_SRC),
+      "circular1.js" => Some(CIRCULAR1_SRC),
+      "circular2.js" => Some(CIRCULAR2_SRC),
+      "circular3.js" => Some(CIRCULAR3_SRC),
+      "slow.js" => Some(SLOW_SRC),
+      "never_ready.js" => Some("should never be loaded"),
+      "main.js" => Some(MAIN_SRC),
+      "bad_import.js" => Some(BAD_IMPORT_SRC),
+      _ => None,
+    }
+  }
+
+  #[derive(Debug, PartialEq)]
+  enum MockError {
+    ResolveErr,
+    LoadErr,
+  }
+
+  impl fmt::Display for MockError {
+    fn fmt(&self, _f: &mut fmt::Formatter) -> fmt::Result {
+      unimplemented!()
+    }
+  }
+
+  impl Error for MockError {
+    fn cause(&self) -> Option<&Error> {
+      unimplemented!()
+    }
+  }
+
+  struct DelayedSourceCodeFuture {
+    url: String,
+    counter: u32,
+  }
+
+  impl Future for DelayedSourceCodeFuture {
+    type Item = String;
+    type Error = MockError;
+
+    fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
+      self.counter += 1;
+      if self.url == "never_ready.js" {
+        // never_ready.js is never ready.
+        return Ok(Async::NotReady);
+      } else if self.url == "slow.js" {
+        if self.counter < 2 {
+          return Ok(Async::NotReady);
+        }
+      }
+      match mock_source_code(&self.url) {
+        Some(src) => Ok(Async::Ready(src.to_string())),
+        None => Err(MockError::LoadErr),
+      }
+    }
+  }
+
   impl Loader for MockLoader {
     type Dispatch = TestDispatch;
-    type Error = std::io::Error;
+    type Error = MockError;
 
-    fn resolve(specifier: &str, _referrer: &str) -> String {
+    fn resolve(
-      specifier.to_string()
+      specifier: &str,
+      _referrer: &str,
+    ) -> Result<String, Self::Error> {
+      if mock_source_code(specifier).is_some() {
+        Ok(specifier.to_string())
+      } else {
+        Err(MockError::ResolveErr)
+      }
     }
 
     fn load(&mut self, url: &str) -> Box<SourceCodeFuture<Self::Error>> {
-      use std::io::{Error, ErrorKind};
       self.loads.push(url.to_string());
-      let result = match url {
+      let url = url.to_string();
-        "a.js" => Ok(A_SRC),
+      Box::new(DelayedSourceCodeFuture { url, counter: 0 })
-        "b.js" => Ok(B_SRC),
-        "c.js" => Ok(C_SRC),
-        "d.js" => Ok(D_SRC),
-        "circular1.js" => Ok(CIRCULAR1_SRC),
-        "circular2.js" => Ok(CIRCULAR2_SRC),
-        _ => Err(Error::new(ErrorKind::Other, "oh no!")),
-      };
-      let result = result.map(|src| src.to_string());
-      Box::new(futures::future::result(result))
     }
 
     fn isolate_and_modules<'a: 'b + 'c, 'b, 'c>(
@@ -342,12 +536,12 @@ mod tests {
 
   #[test]
   fn test_recursive_load() {
-    let mut loader = MockLoader::new();
+    let loader = MockLoader::new();
-    let mut recursive_load = RecursiveLoad::new("a.js", &mut loader);
+    let mut recursive_load = RecursiveLoad::new("a.js", loader);
 
     let result = recursive_load.poll();
     assert!(result.is_ok());
-    if let Async::Ready(a_id) = result.ok().unwrap() {
+    if let Async::Ready((a_id, mut loader)) = result.ok().unwrap() {
       js_check(loader.isolate.mod_evaluate(a_id));
       assert_eq!(loader.loads, vec!["a.js", "b.js", "c.js", "d.js"]);
 
@@ -366,7 +560,7 @@ mod tests {
       assert_eq!(modules.get_children(c_id), Some(&vec!["d.js".to_string()]));
       assert_eq!(modules.get_children(d_id), Some(&vec![]));
     } else {
-      panic!("Future should be ready")
+      assert!(false);
     }
   }
 
@@ -376,20 +570,29 @@ mod tests {
   "#;
 
   const CIRCULAR2_SRC: &str = r#"
-    import "circular1.js";
+    import "circular3.js";
     Deno.core.print("circular2");
   "#;
 
+  const CIRCULAR3_SRC: &str = r#"
+    import "circular1.js";
+    import "circular2.js";
+    Deno.core.print("circular3");
+  "#;
+
   #[test]
   fn test_circular_load() {
-    let mut loader = MockLoader::new();
+    let loader = MockLoader::new();
-    let mut recursive_load = RecursiveLoad::new("circular1.js", &mut loader);
+    let mut recursive_load = RecursiveLoad::new("circular1.js", loader);
 
     let result = recursive_load.poll();
     assert!(result.is_ok());
-    if let Async::Ready(circular1_id) = result.ok().unwrap() {
+    if let Async::Ready((circular1_id, mut loader)) = result.ok().unwrap() {
       js_check(loader.isolate.mod_evaluate(circular1_id));
-      assert_eq!(loader.loads, vec!["circular1.js", "circular2.js"]);
+      assert_eq!(
+        loader.loads,
+        vec!["circular1.js", "circular2.js", "circular3.js"]
+      );
 
       let modules = &loader.modules;
 
@@ -403,10 +606,127 @@ mod tests {
 
       assert_eq!(
         modules.get_children(circular2_id),
-        Some(&vec!["circular1.js".to_string()])
+        Some(&vec!["circular3.js".to_string()])
+      );
+
+      assert!(modules.get_id("circular3.js").is_some());
+      let circular3_id = modules.get_id("circular3.js").unwrap();
+      assert_eq!(
+        modules.get_children(circular3_id),
+        Some(&vec![
+          "circular1.js".to_string(),
+          "circular2.js".to_string()
+        ])
       );
     } else {
-      panic!("Future should be ready")
+      assert!(false);
     }
   }
+
+  // main.js
+  const MAIN_SRC: &str = r#"
+    // never_ready.js never loads.
+    import "never_ready.js";
+    // slow.js resolves after one tick.
+    import "slow.js";
+  "#;
+
+  // slow.js
+  const SLOW_SRC: &str = r#"
+    // Circular import of never_ready.js
+    // Does this trigger two Loader calls? It shouldn't.
+    import "never_ready.js";
+    import "a.js";
+  "#;
+
+  #[test]
+  fn slow_never_ready_modules() {
+    let loader = MockLoader::new();
+    let mut recursive_load = RecursiveLoad::new("main.js", loader);
+
+    let result = recursive_load.poll();
+    assert!(result.is_ok());
+    assert!(result.ok().unwrap().is_not_ready());
+
+    {
+      let loader = recursive_load.loader.as_ref().unwrap();
+      assert_eq!(loader.loads, vec!["main.js", "never_ready.js", "slow.js"]);
+    }
+
+    let result = recursive_load.poll();
+    assert!(result.is_ok());
+    assert!(result.ok().unwrap().is_not_ready());
+
+    {
+      let loader = recursive_load.loader.as_ref().unwrap();
+      assert_eq!(
+        loader.loads,
+        vec![
+          "main.js",
+          "never_ready.js",
+          "slow.js",
+          "a.js",
+          "b.js",
+          "c.js",
+          "d.js"
+        ]
+      );
+    }
+
+    let result = recursive_load.poll();
+    assert!(result.is_ok());
+    assert!(result.ok().unwrap().is_not_ready());
+
+    {
+      let loader = recursive_load.loader.as_ref().unwrap();
+      assert_eq!(
+        loader.loads,
+        vec![
+          "main.js",
+          "never_ready.js",
+          "slow.js",
+          "a.js",
+          "b.js",
+          "c.js",
+          "d.js"
+        ]
+      );
+    }
+
+    let result = recursive_load.poll();
+    assert!(result.is_ok());
+    assert!(result.ok().unwrap().is_not_ready());
+
+    {
+      let loader = recursive_load.loader.as_ref().unwrap();
+      assert_eq!(
+        loader.loads,
+        vec![
+          "main.js",
+          "never_ready.js",
+          "slow.js",
+          "a.js",
+          "b.js",
+          "c.js",
+          "d.js"
+        ]
+      );
+    }
+  }
+
+  // bad_import.js
+  const BAD_IMPORT_SRC: &str = r#"
+    import "foo";
+  "#;
+
+  #[test]
+  fn loader_disappears_after_error() {
+    let loader = MockLoader::new();
+    let mut recursive_load = RecursiveLoad::new("bad_import.js", loader);
+    let result = recursive_load.poll();
+    assert!(result.is_err());
+    let (either_err, _loader) = result.err().unwrap();
+    assert_eq!(either_err, JSErrorOr::Other(MockError::ResolveErr));
+    assert!(recursive_load.loader.is_none());
+  }
 }

js/unit_tests.ts

@@ -49,4 +49,4 @@ import "./version_test.ts";
 
 import "../website/app_test.js";
 
-import "deps/https/deno.land/std/testing/main.ts";
+import "./deps/https/deno.land/std/testing/main.ts";

tests/026_workers.ts

@@ -1,5 +1,5 @@
-const jsWorker = new Worker("tests/subdir/test_worker.js");
+const jsWorker = new Worker("./tests/subdir/test_worker.js");
-const tsWorker = new Worker("tests/subdir/test_worker.ts");
+const tsWorker = new Worker("./tests/subdir/test_worker.ts");
 
 tsWorker.onmessage = e => {
   console.log("Received ts: " + e.data);

tests/circular1.js

@@ -1,2 +1,2 @@
-import "circular2.js";
+import "./circular2.js";
 console.log("circular1");

tests/circular2.js

@@ -1,2 +1,2 @@
-import "circular1.js";
+import "./circular1.js";
 console.log("circular2");

tests/import_meta.ts

@@ -1,3 +1,3 @@
 console.log("import_meta", import.meta.url, import.meta.main);
 
-import "import_meta2.ts";
+import "./import_meta2.ts";