remove dependency of isolate on modules

core/dyn_imports.rs

@@ -0,0 +1,75 @@
+// This is code shared between isolate and modules.
+// This is to keep with the goal that isolate should not depend on modules.
+// TODO(ry) rename this module to dyn_import.rs
+
+// Do not add any dependency to modules.rs!
+// modules.rs is complex and should remain decoupled from isolate.rs to keep the
+// Isolate struct from becoming too bloating for users who do not need
+// asynchronous module loading.
+
+use crate::any_error::ErrBox;
+use crate::isolate::Isolate;
+use crate::isolate::SourceCodeInfo;
+use crate::libdeno::deno_dyn_import_id;
+use crate::libdeno::deno_mod;
+use futures::stream::Stream;
+use futures::Async::*;
+use futures::Poll;
+use std::fmt;
+
+#[derive(Debug, Eq, PartialEq)]
+pub enum RecursiveLoadEvent {
+  Fetch(SourceCodeInfo),
+  Instantiate(deno_mod),
+}
+
+pub trait ImportStream: Stream {
+  fn register(
+    &mut self,
+    source_code_info: SourceCodeInfo,
+    isolate: &mut Isolate,
+  ) -> Result<(), ErrBox>;
+}
+
+pub type DynImportStream =
+  Box<dyn ImportStream<Item = RecursiveLoadEvent, Error = ErrBox> + Send>;
+
+pub type DynImportFn = Fn(deno_dyn_import_id, &str, &str) -> DynImportStream;
+
+/// Wraps DynImportStream to include the deno_dyn_import_id, so that it doesn't
+/// need to be exposed.
+#[derive(Debug)]
+pub struct DynImport {
+  pub id: deno_dyn_import_id,
+  pub inner: DynImportStream,
+}
+
+impl fmt::Debug for DynImportStream {
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    write!(f, "DynImportStream(..)")
+  }
+}
+
+impl Stream for DynImport {
+  type Item = (deno_dyn_import_id, RecursiveLoadEvent);
+  type Error = (deno_dyn_import_id, ErrBox);
+
+  fn poll(&mut self) -> Poll<Option<Self::Item>, Self::Error> {
+    match self.inner.poll() {
+      Ok(Ready(Some(event))) => Ok(Ready(Some((self.id, event)))),
+      Ok(Ready(None)) => Ok(Ready(None)), // Should never happen.
+      Err(e) => Err((self.id, e)),
+      Ok(NotReady) => Ok(NotReady),
+    }
+  }
+}
+
+impl ImportStream for DynImport {
+  fn register(
+    &mut self,
+    source_code_info: SourceCodeInfo,
+    isolate: &mut Isolate,
+  ) -> Result<(), ErrBox> {
+    self.inner.register(source_code_info, isolate)
+  }
+}

core/isolate.rs

@@ -1,10 +1,16 @@
 // Copyright 2018 the Deno authors. All rights reserved. MIT license.
 
-// Do not add dependenies to modules.rs. it should remain decoupled from the
-// isolate to keep the Isolate struct from becoming too bloating for users who
-// do not need asynchronous module loading.
+// Do not add any dependency to modules.rs!
+// modules.rs is complex and should remain decoupled from isolate.rs to keep the
+// Isolate struct from becoming too bloating for users who do not need
+// asynchronous module loading.
 
 use crate::any_error::ErrBox;
+use crate::dyn_imports::DynImport;
+use crate::dyn_imports::DynImportFn;
+use crate::dyn_imports::DynImportStream;
+use crate::dyn_imports::ImportStream;
+use crate::dyn_imports::RecursiveLoadEvent;
 use crate::js_errors::CoreJSError;
 use crate::js_errors::V8Exception;
 use crate::libdeno;
@@ -16,8 +22,6 @@ use crate::libdeno::OpId;
 use crate::libdeno::PinnedBuf;
 use crate::libdeno::Snapshot1;
 use crate::libdeno::Snapshot2;
-use crate::modules::RecursiveLoadEvent;
-use crate::modules::SourceCodeInfo;
 use crate::shared_queue::SharedQueue;
 use crate::shared_queue::RECOMMENDED_SIZE;
 use futures::stream::FuturesUnordered;
@@ -31,7 +35,6 @@ use libc::c_char;
 use libc::c_void;
 use std::ffi::CStr;
 use std::ffi::CString;
-use std::fmt;
 use std::ptr::null;
 use std::sync::{Arc, Mutex, Once};
 
@@ -62,6 +65,21 @@ pub struct Script<'a> {
   pub filename: &'a str,
 }
 
+/// Represent result of fetching the source code of a module.
+/// Contains both module name and code.
+/// Module name might be different from initial URL used for loading
+/// due to redirections.
+/// e.g. Both https://example.com/a.ts and https://example.com/b.ts
+/// may point to https://example.com/c.ts. By specifying module_name
+/// all be https://example.com/c.ts in module_name (for aliasing),
+/// we avoid recompiling the same code for 3 different times.
+#[derive(Debug, Eq, PartialEq)]
+pub struct SourceCodeInfo {
+  pub code: String,
+  pub module_url_specified: String,
+  pub module_url_found: String,
+}
+
 // TODO(ry) It's ugly that we have both Script and OwnedScript. Ideally we
 // wouldn't expose such twiddly complexity.
 struct OwnedScript {
@@ -88,58 +106,8 @@ pub enum StartupData<'a> {
   None,
 }
 
-pub trait ImportStream: Stream {
-  fn register(
-    &mut self,
-    source_code_info: SourceCodeInfo,
-    isolate: &mut Isolate,
-  ) -> Result<(), ErrBox>;
-}
-
-pub type DynImportStream =
-  Box<dyn ImportStream<Item = RecursiveLoadEvent, Error = ErrBox> + Send>;
-type DynImportFn = Fn(deno_dyn_import_id, &str, &str) -> DynImportStream;
-
 type JSErrorCreateFn = dyn Fn(V8Exception) -> ErrBox;
 
-/// Wraps DynImportStream to include the deno_dyn_import_id, so that it doesn't
-/// need to be exposed.
-#[derive(Debug)]
-struct DynImport {
-  id: deno_dyn_import_id,
-  inner: DynImportStream,
-}
-
-impl fmt::Debug for DynImportStream {
-  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-    write!(f, "DynImportStream(..)")
-  }
-}
-
-impl Stream for DynImport {
-  type Item = (deno_dyn_import_id, RecursiveLoadEvent);
-  type Error = (deno_dyn_import_id, ErrBox);
-
-  fn poll(&mut self) -> Poll<Option<Self::Item>, Self::Error> {
-    match self.inner.poll() {
-      Ok(Ready(Some(event))) => Ok(Ready(Some((self.id, event)))),
-      Ok(Ready(None)) => Ok(Ready(None)), // Should never happen.
-      Err(e) => Err((self.id, e)),
-      Ok(NotReady) => Ok(NotReady),
-    }
-  }
-}
-
-impl ImportStream for DynImport {
-  fn register(
-    &mut self,
-    source_code_info: SourceCodeInfo,
-    isolate: &mut Isolate,
-  ) -> Result<(), ErrBox> {
-    self.inner.register(source_code_info, isolate)
-  }
-}
-
 /// A single execution context of JavaScript. Corresponds roughly to the "Web
 /// Worker" concept in the DOM. An Isolate is a Future that can be used with
 /// Tokio.  The Isolate future complete when there is an error or when all
@@ -979,7 +947,6 @@ pub mod tests {
       let count_ = count.clone();
 
       // Sometimes Rust is really annoying.
-      use std::sync::Mutex;
       let mod_b = Arc::new(Mutex::new(0));
       let mod_b2 = mod_b.clone();
 

core/lib.rs

@@ -5,6 +5,7 @@ extern crate futures;
 extern crate libc;
 
 mod any_error;
+mod dyn_imports;
 mod flags;
 mod isolate;
 mod js_errors;

core/modules.rs

@@ -7,8 +7,10 @@
 // synchronously. The isolate.rs module should never depend on this module.
 
 use crate::any_error::ErrBox;
-use crate::isolate::ImportStream;
+use crate::dyn_imports::ImportStream;
+use crate::dyn_imports::RecursiveLoadEvent as Event;
 use crate::isolate::Isolate;
+use crate::isolate::SourceCodeInfo;
 use crate::libdeno::deno_dyn_import_id;
 use crate::libdeno::deno_mod;
 use crate::module_specifier::ModuleSpecifier;
@@ -25,21 +27,6 @@ use std::fmt;
 use std::sync::Arc;
 use std::sync::Mutex;
 
-/// Represent result of fetching the source code of a module.
-/// Contains both module name and code.
-/// Module name might be different from initial URL used for loading
-/// due to redirections.
-/// e.g. Both https://example.com/a.ts and https://example.com/b.ts
-/// may point to https://example.com/c.ts. By specifying module_name
-/// all be https://example.com/c.ts in module_name (for aliasing),
-/// we avoid recompiling the same code for 3 different times.
-#[derive(Debug, Eq, PartialEq)]
-pub struct SourceCodeInfo {
-  pub code: String,
-  pub module_url_specified: String,
-  pub module_url_found: String,
-}
-
 pub type SourceCodeInfoFuture =
   dyn Future<Item = SourceCodeInfo, Error = ErrBox> + Send;
 
@@ -68,13 +55,6 @@ enum Kind {
   DynamicImport(i32),
 }
 
-#[derive(Debug, Eq, PartialEq)]
-pub enum RecursiveLoadEvent {
-  Fetch(SourceCodeInfo),
-  Instantiate(deno_mod),
-}
-use RecursiveLoadEvent as Event;
-
 #[derive(Debug, Eq, PartialEq)]
 enum State {
   ResolveMain(String),           // specifier