fix(core): let V8 drive extension ESM loads (#18997)

This now allows circular imports across extensions.

Instead of load + eval of all ESM files in declaration order, all files
are only loaded. Eval is done recursively by V8, only evaluating
files that are listed in `Extension::esm_entry_point` fields.

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>

1 files changed, 69 insertions, 39 deletions

diff --git a/core/runtime.rs b/core/runtime.rs
index 3c2e6f3e1..b56ef5d65 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -8,6 +8,7 @@ use crate::extensions::OpDecl;
 use crate::extensions::OpEventLoopFn;
 use crate::inspector::JsRuntimeInspector;
 use crate::module_specifier::ModuleSpecifier;
+use crate::modules::AssertedModuleType;
 use crate::modules::ExtModuleLoaderCb;
 use crate::modules::ModuleCode;
 use crate::modules::ModuleError;
@@ -23,7 +24,6 @@ use crate::snapshot_util;
 use crate::source_map::SourceMapCache;
 use crate::source_map::SourceMapGetter;
 use crate::Extension;
-use crate::ExtensionFileSource;
 use crate::NoopModuleLoader;
 use crate::OpMiddlewareFn;
 use crate::OpResult;
@@ -687,46 +687,49 @@ impl JsRuntime {
     module_map
   }
 
-  /// Initializes JS of provided Extensions in the given realm
+  /// Initializes JS of provided Extensions in the given realm.
   fn init_extension_js(&mut self, realm: &JsRealm) -> Result<(), Error> {
-    fn load_and_evaluate_module(
-      runtime: &mut JsRuntime,
-      file_source: &ExtensionFileSource,
-    ) -> Result<(), Error> {
-      futures::executor::block_on(async {
-        let id = runtime
-          .load_side_module(
-            &ModuleSpecifier::parse(file_source.specifier)?,
-            None,
-          )
-          .await?;
-        let receiver = runtime.mod_evaluate(id);
-        runtime.run_event_loop(false).await?;
-        receiver.await?
-      })
-      .with_context(|| format!("Couldn't execute '{}'", file_source.specifier))
-    }
+    // Initalization of JS happens in phases:
+    // 1. Iterate through all extensions:
+    //  a. Execute all extension "script" JS files
+    //  b. Load all extension "module" JS files (but do not execute them yet)
+    // 2. Iterate through all extensions:
+    //  a. If an extension has a `esm_entry_point`, execute it.
+
+    let mut esm_entrypoints = vec![];
 
     // Take extensions to avoid double-borrow
     let extensions = std::mem::take(&mut self.extensions);
-    for ext in extensions.borrow().iter() {
-      {
-        if let Some(esm_files) = ext.get_esm_sources() {
-          if let Some(entry_point) = ext.get_esm_entry_point() {
-            let file_source = esm_files
-              .iter()
-              .find(|file| file.specifier == entry_point)
-              .unwrap();
-            load_and_evaluate_module(self, file_source)?;
-          } else {
-            for file_source in esm_files {
-              load_and_evaluate_module(self, file_source)?;
-            }
+
+    futures::executor::block_on(async {
+      let num_of_extensions = extensions.borrow().len();
+      for i in 0..num_of_extensions {
+        let (maybe_esm_files, maybe_esm_entry_point) = {
+          let exts = extensions.borrow();
+          (
+            exts[i].get_esm_sources().map(|e| e.to_owned()),
+            exts[i].get_esm_entry_point(),
+          )
+        };
+
+        if let Some(esm_files) = maybe_esm_files {
+          for file_source in esm_files {
+            self
+              .load_side_module(
+                &ModuleSpecifier::parse(file_source.specifier)?,
+                None,
+              )
+              .await?;
           }
         }
-      }
 
-      {
+        if let Some(entry_point) = maybe_esm_entry_point {
+          esm_entrypoints.push(entry_point);
+        }
+
+        let exts = extensions.borrow();
+        let ext = &exts[i];
+
         if let Some(js_files) = ext.get_js_sources() {
           for file_source in js_files {
             realm.execute_script(
@@ -736,14 +739,41 @@ impl JsRuntime {
             )?;
           }
         }
+
+        if ext.is_core {
+          self.init_cbs(realm);
+        }
       }
 
-      // TODO(bartlomieju): this not great that we need to have this conditional
-      // here, but I haven't found a better way to do it yet.
-      if ext.is_core {
-        self.init_cbs(realm);
+      for specifier in esm_entrypoints {
+        let mod_id = {
+          let module_map = self.module_map.as_ref().unwrap();
+
+          module_map
+            .borrow()
+            .get_id(specifier, AssertedModuleType::JavaScriptOrWasm)
+            .unwrap_or_else(|| {
+              panic!("{} not present in the module map", specifier)
+            })
+        };
+        let receiver = self.mod_evaluate(mod_id);
+        self.run_event_loop(false).await?;
+        receiver
+          .await?
+          .with_context(|| format!("Couldn't execute '{specifier}'"))?;
       }
-    }
+
+      #[cfg(debug_assertions)]
+      {
+        let module_map_rc = self.module_map.clone().unwrap();
+        let mut scope = realm.handle_scope(self.v8_isolate());
+        let module_map = module_map_rc.borrow();
+        module_map.assert_all_modules_evaluated(&mut scope);
+      }
+
+      Ok::<_, anyhow::Error>(())
+    })?;
+
     // Restore extensions
     self.extensions = extensions;