refactor: rename structures related to Modules (#4217)

* rename structures related to ES Modules; add "Modules" prefix
* remove unneeded Unpin trait requirement for "ModuleLoader"

cli/ops/compiler.rs

@@ -6,7 +6,7 @@ use crate::futures::future::try_join_all;
 use crate::msg;
 use crate::op_error::OpError;
 use crate::state::State;
-use deno_core::Loader;
+use deno_core::ModuleLoader;
 use deno_core::*;
 use futures::future::FutureExt;
 

cli/state.rs

@@ -12,7 +12,7 @@ use crate::worker::WorkerHandle;
 use deno_core::Buf;
 use deno_core::CoreOp;
 use deno_core::ErrBox;
-use deno_core::Loader;
+use deno_core::ModuleLoader;
 use deno_core::ModuleSpecifier;
 use deno_core::Op;
 use deno_core::ResourceTable;
@@ -151,7 +151,7 @@ impl State {
   }
 }
 
-impl Loader for State {
+impl ModuleLoader for State {
   fn resolve(
     &self,
     specifier: &str,
@@ -178,7 +178,7 @@ impl Loader for State {
     module_specifier: &ModuleSpecifier,
     maybe_referrer: Option<ModuleSpecifier>,
     is_dyn_import: bool,
-  ) -> Pin<Box<deno_core::SourceCodeInfoFuture>> {
+  ) -> Pin<Box<deno_core::ModuleSourceFuture>> {
     let module_specifier = module_specifier.clone();
     if is_dyn_import {
       if let Err(e) = self.check_dyn_import(&module_specifier) {
@@ -196,7 +196,7 @@ impl Loader for State {
       let compiled_module = global_state
         .fetch_compiled_module(module_specifier, maybe_referrer, target_lib)
         .await?;
-      Ok(deno_core::SourceCodeInfo {
+      Ok(deno_core::ModuleSource {
         // Real module name, might be different from initial specifier
         // due to redirections.
         code: compiled_module.code,

core/es_isolate.rs

@@ -31,34 +31,23 @@ use crate::isolate::Isolate;
 use crate::isolate::StartupData;
 use crate::module_specifier::ModuleSpecifier;
 use crate::modules::LoadState;
-use crate::modules::Loader;
+use crate::modules::ModuleLoader;
+use crate::modules::ModuleSource;
 use crate::modules::Modules;
 use crate::modules::RecursiveModuleLoad;
 
 pub type ModuleId = i32;
 pub type DynImportId = i32;
-/// Represent result of fetching the source code of a module. Found module URL
-/// might be different from specified URL used for loading due to redirections
-/// (like HTTP 303). E.G. Both https://example.com/a.ts and
-/// https://example.com/b.ts may point to https://example.com/c.ts
-/// By keeping track of specified and found URL we can alias modules and avoid
-/// recompiling the same code 3 times.
-#[derive(Debug, Eq, PartialEq)]
-pub struct SourceCodeInfo {
-  pub code: String,
-  pub module_url_specified: String,
-  pub module_url_found: String,
-}
 
 /// More specialized version of `Isolate` that provides loading
 /// and execution of ES Modules.
 ///
 /// Creating `EsIsolate` requires to pass `loader` argument
-/// that implements `Loader` trait - that way actual resolution and
+/// that implements `ModuleLoader` trait - that way actual resolution and
 /// loading of modules can be customized by the implementor.
 pub struct EsIsolate {
   core_isolate: Box<Isolate>,
-  loader: Rc<dyn Loader + Unpin>,
+  loader: Rc<dyn ModuleLoader>,
   pub modules: Modules,
   pub(crate) next_dyn_import_id: DynImportId,
   pub(crate) dyn_import_map:
@@ -84,7 +73,7 @@ impl DerefMut for EsIsolate {
 
 impl EsIsolate {
   pub fn new(
-    loader: Rc<dyn Loader + Unpin>,
+    loader: Rc<dyn ModuleLoader>,
     startup_data: StartupData,
     will_snapshot: bool,
   ) -> Box<Self> {
@@ -429,10 +418,10 @@ impl EsIsolate {
 
   fn register_during_load(
     &mut self,
-    info: SourceCodeInfo,
+    info: ModuleSource,
     load: &mut RecursiveModuleLoad,
   ) -> Result<(), ErrBox> {
-    let SourceCodeInfo {
+    let ModuleSource {
       code,
       module_url_specified,
       module_url_found,
@@ -553,7 +542,7 @@ pub mod tests {
   use crate::isolate::js_check;
   use crate::isolate::tests::run_in_task;
   use crate::isolate::ZeroCopyBuf;
-  use crate::modules::SourceCodeInfoFuture;
+  use crate::modules::ModuleSourceFuture;
   use crate::ops::*;
   use std::io;
   use std::sync::atomic::{AtomicUsize, Ordering};
@@ -566,7 +555,7 @@ pub mod tests {
       pub count: Arc<AtomicUsize>,
     }
 
-    impl Loader for ModsLoader {
+    impl ModuleLoader for ModsLoader {
       fn resolve(
         &self,
         specifier: &str,
@@ -585,7 +574,7 @@ pub mod tests {
         _module_specifier: &ModuleSpecifier,
         _maybe_referrer: Option<ModuleSpecifier>,
         _is_dyn_import: bool,
-      ) -> Pin<Box<SourceCodeInfoFuture>> {
+      ) -> Pin<Box<ModuleSourceFuture>> {
         unreachable!()
       }
     }
@@ -665,7 +654,7 @@ pub mod tests {
       pub count: Arc<AtomicUsize>,
     }
 
-    impl Loader for DynImportErrLoader {
+    impl ModuleLoader for DynImportErrLoader {
       fn resolve(
         &self,
         specifier: &str,
@@ -684,7 +673,7 @@ pub mod tests {
         _module_specifier: &ModuleSpecifier,
         _maybe_referrer: Option<ModuleSpecifier>,
         _is_dyn_import: bool,
-      ) -> Pin<Box<SourceCodeInfoFuture>> {
+      ) -> Pin<Box<ModuleSourceFuture>> {
         async { Err(ErrBox::from(io::Error::from(io::ErrorKind::NotFound))) }
           .boxed()
       }
@@ -726,7 +715,7 @@ pub mod tests {
       pub count: Arc<AtomicUsize>,
     }
 
-    impl Loader for DynImportErr2Loader {
+    impl ModuleLoader for DynImportErr2Loader {
       fn resolve(
         &self,
         specifier: &str,
@@ -750,8 +739,8 @@ pub mod tests {
         &self,
         specifier: &ModuleSpecifier,
         _maybe_referrer: Option<ModuleSpecifier>,
-      ) -> Pin<Box<SourceCodeInfoFuture>> {
-        let info = SourceCodeInfo {
+      ) -> Pin<Box<ModuleSourceFuture>> {
+        let info = ModuleSource {
           module_url_specified: specifier.to_string(),
           module_url_found: specifier.to_string(),
           code: "# not valid JS".to_owned(),
@@ -811,7 +800,7 @@ pub mod tests {
       pub load_count: Arc<AtomicUsize>,
     }
 
-    impl Loader for DynImportOkLoader {
+    impl ModuleLoader for DynImportOkLoader {
       fn resolve(
         &self,
         specifier: &str,
@@ -834,9 +823,9 @@ pub mod tests {
         specifier: &ModuleSpecifier,
         _maybe_referrer: Option<ModuleSpecifier>,
         _is_dyn_import: bool,
-      ) -> Pin<Box<SourceCodeInfoFuture>> {
+      ) -> Pin<Box<ModuleSourceFuture>> {
         self.load_count.fetch_add(1, Ordering::Relaxed);
-        let info = SourceCodeInfo {
+        let info = ModuleSource {
           module_url_specified: specifier.to_string(),
           module_url_found: specifier.to_string(),
           code: "export function b() { return 'b' }".to_owned(),

core/modules.rs

@@ -5,7 +5,6 @@ use rusty_v8 as v8;
 use crate::any_error::ErrBox;
 use crate::es_isolate::DynImportId;
 use crate::es_isolate::ModuleId;
-use crate::es_isolate::SourceCodeInfo;
 use crate::module_specifier::ModuleSpecifier;
 use futures::future::FutureExt;
 use futures::stream::FuturesUnordered;
@@ -20,10 +19,31 @@ use std::rc::Rc;
 use std::task::Context;
 use std::task::Poll;
 
-pub type SourceCodeInfoFuture =
-  dyn Future<Output = Result<SourceCodeInfo, ErrBox>>;
+/// EsModule source code that will be loaded into V8.
+///
+/// Users can implement `Into<ModuleInfo>` for different file types that
+/// can be transpiled to valid EsModule.
+///
+/// Found module URL might be different from specified URL
+/// used for loading due to redirections (like HTTP 303).
+/// Eg. Both "https://example.com/a.ts" and
+/// "https://example.com/b.ts" may point to "https://example.com/c.ts"
+/// By keeping track of specified and found URL we can alias modules and avoid
+/// recompiling the same code 3 times.
+// TODO(bartlomieju): I have a strong opinion we should store all redirects
+// that happened; not only first and final target. It would simplify a lot
+// of things throughout the codebase otherwise we may end up requesting
+// intermediate redirects from file loader.
+#[derive(Debug, Eq, PartialEq)]
+pub struct ModuleSource {
+  pub code: String,
+  pub module_url_specified: String,
+  pub module_url_found: String,
+}
+
+pub type ModuleSourceFuture = dyn Future<Output = Result<ModuleSource, ErrBox>>;
 
-pub trait Loader {
+pub trait ModuleLoader {
   /// Returns an absolute URL.
   /// When implementing an spec-complaint VM, this should be exactly the
   /// algorithm described here:
@@ -47,7 +67,7 @@ pub trait Loader {
     module_specifier: &ModuleSpecifier,
     maybe_referrer: Option<ModuleSpecifier>,
     is_dyn_import: bool,
-  ) -> Pin<Box<SourceCodeInfoFuture>>;
+  ) -> Pin<Box<ModuleSourceFuture>>;
 }
 
 #[derive(Debug, Eq, PartialEq)]
@@ -74,8 +94,8 @@ pub struct RecursiveModuleLoad {
   // Kind::Main
   pub dyn_import_id: Option<DynImportId>,
   pub state: LoadState,
-  pub loader: Rc<dyn Loader + Unpin>,
-  pub pending: FuturesUnordered<Pin<Box<SourceCodeInfoFuture>>>,
+  pub loader: Rc<dyn ModuleLoader>,
+  pub pending: FuturesUnordered<Pin<Box<ModuleSourceFuture>>>,
   pub is_pending: HashSet<ModuleSpecifier>,
 }
 
@@ -84,7 +104,7 @@ impl RecursiveModuleLoad {
   pub fn main(
     specifier: &str,
     code: Option<String>,
-    loader: Rc<dyn Loader + Unpin>,
+    loader: Rc<dyn ModuleLoader>,
   ) -> Self {
     let kind = Kind::Main;
     let state = LoadState::ResolveMain(specifier.to_owned(), code);
@@ -95,7 +115,7 @@ impl RecursiveModuleLoad {
     id: DynImportId,
     specifier: &str,
     referrer: &str,
-    loader: Rc<dyn Loader + Unpin>,
+    loader: Rc<dyn ModuleLoader>,
   ) -> Self {
     let kind = Kind::DynamicImport;
     let state =
@@ -110,7 +130,7 @@ impl RecursiveModuleLoad {
   fn new(
     kind: Kind,
     state: LoadState,
-    loader: Rc<dyn Loader + Unpin>,
+    loader: Rc<dyn ModuleLoader>,
     dyn_import_id: Option<DynImportId>,
   ) -> Self {
     Self {
@@ -138,7 +158,7 @@ impl RecursiveModuleLoad {
 
     let load_fut = match &self.state {
       LoadState::ResolveMain(_, Some(code)) => {
-        futures::future::ok(SourceCodeInfo {
+        futures::future::ok(ModuleSource {
           code: code.to_owned(),
           module_url_specified: module_specifier.to_string(),
           module_url_found: module_specifier.to_string(),
@@ -174,7 +194,7 @@ impl RecursiveModuleLoad {
 }
 
 impl Stream for RecursiveModuleLoad {
-  type Item = Result<SourceCodeInfo, ErrBox>;
+  type Item = Result<ModuleSource, ErrBox>;
 
   fn poll_next(
     self: Pin<&mut Self>,
@@ -544,7 +564,7 @@ mod tests {
   }
 
   impl Future for DelayedSourceCodeFuture {
-    type Output = Result<SourceCodeInfo, ErrBox>;
+    type Output = Result<ModuleSource, ErrBox>;
 
     fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
       let inner = self.get_mut();
@@ -559,7 +579,7 @@ mod tests {
         return Poll::Pending;
       }
       match mock_source_code(&inner.url) {
-        Some(src) => Poll::Ready(Ok(SourceCodeInfo {
+        Some(src) => Poll::Ready(Ok(ModuleSource {
           code: src.0.to_owned(),
           module_url_specified: inner.url.clone(),
           module_url_found: src.1.to_owned(),
@@ -569,7 +589,7 @@ mod tests {
     }
   }
 
-  impl Loader for MockLoader {
+  impl ModuleLoader for MockLoader {
     fn resolve(
       &self,
       specifier: &str,
@@ -602,7 +622,7 @@ mod tests {
       module_specifier: &ModuleSpecifier,
       _maybe_referrer: Option<ModuleSpecifier>,
       _is_dyn_import: bool,
-    ) -> Pin<Box<SourceCodeInfoFuture>> {
+    ) -> Pin<Box<ModuleSourceFuture>> {
       let mut loads = self.loads.lock().unwrap();
       loads.push(module_specifier.to_string());
       let url = module_specifier.to_string();
@@ -835,7 +855,7 @@ mod tests {
   // slow.js
   const SLOW_SRC: &str = r#"
     // Circular import of never_ready.js
-    // Does this trigger two Loader calls? It shouldn't.
+    // Does this trigger two ModuleLoader calls? It shouldn't.
     import "/never_ready.js";
     import "/a.js";
   "#;