refactor: Use ES modules for internal runtime code (denoland#17648)

This PR refactors all internal js files (except core) to be written as ES modules. `__bootstrap`has been mostly replaced with static imports in form in `internal:[path to file from repo root]`. To specify if files are ESM, an `esm` method has been added to `Extension`, similar to the `js` method. A new ModuleLoader called `InternalModuleLoader` has been added to enable the loading of internal specifiers, which is used in all situations except when a snapshot is only loaded, and not a new one is created from it. --------- Co-authored-by: Bartek Iwańczuk <[email protected]>
DavidDeSimone · Feb 7, 2023 · b4aa153 · b4aa153
1 parent 65500f3
commit b4aa153
Show file tree

Hide file tree

Showing 123 changed files with 39,975 additions and 40,114 deletions.
diff --git a/bench_util/js_runtime.rs b/bench_util/js_runtime.rs
@@ -10,6 +10,9 @@ use crate::profiling::is_profiling;
 pub fn create_js_runtime(setup: impl FnOnce() -> Vec<Extension>) -> JsRuntime {
   JsRuntime::new(RuntimeOptions {
     extensions_with_js: setup(),
+    module_loader: Some(std::rc::Rc::new(
+      deno_core::InternalModuleLoader::new(None),
+    )),
     ..Default::default()
   })
 }

diff --git a/cli/build.rs b/cli/build.rs
@@ -275,6 +275,7 @@ mod ts {
         .build()],
       extensions_with_js: vec![],
       additional_files: files,
+      additional_esm_files: vec![],
       compression_cb: Some(Box::new(|vec, snapshot_slice| {
         vec.extend_from_slice(
           &zstd::bulk::compress(snapshot_slice, 22)
@@ -306,7 +307,7 @@ mod ts {
   }
 }
 
-fn create_cli_snapshot(snapshot_path: PathBuf, files: Vec<PathBuf>) {
+fn create_cli_snapshot(snapshot_path: PathBuf, esm_files: Vec<PathBuf>) {
   let extensions: Vec<Extension> = vec![
     deno_webidl::init(),
     deno_console::init(),
@@ -343,7 +344,8 @@ fn create_cli_snapshot(snapshot_path: PathBuf, files: Vec<PathBuf>) {
     startup_snapshot: Some(deno_runtime::js::deno_isolate_init()),
     extensions,
     extensions_with_js: vec![],
-    additional_files: files,
+    additional_files: vec![],
+    additional_esm_files: esm_files,
     compression_cb: Some(Box::new(|vec, snapshot_slice| {
       lzzzz::lz4_hc::compress_to_vec(
         snapshot_slice,
@@ -448,13 +450,13 @@ fn main() {
   let o = PathBuf::from(env::var_os("OUT_DIR").unwrap());
 
   let compiler_snapshot_path = o.join("COMPILER_SNAPSHOT.bin");
-  let js_files = get_js_files(env!("CARGO_MANIFEST_DIR"), "tsc");
+  let js_files = get_js_files(env!("CARGO_MANIFEST_DIR"), "tsc", None);
   ts::create_compiler_snapshot(compiler_snapshot_path, js_files, &c);
 
   let cli_snapshot_path = o.join("CLI_SNAPSHOT.bin");
-  let mut js_files = get_js_files(env!("CARGO_MANIFEST_DIR"), "js");
-  js_files.push(deno_runtime::js::get_99_main());
-  create_cli_snapshot(cli_snapshot_path, js_files);
+  let mut esm_files = get_js_files(env!("CARGO_MANIFEST_DIR"), "js", None);
+  esm_files.push(deno_runtime::js::get_99_main());
+  create_cli_snapshot(cli_snapshot_path, esm_files);
 
   #[cfg(target_os = "windows")]
   {

diff --git a/cli/js/40_testing.js b/cli/js/40_testing.js
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
@@ -3844,3 +3844,15 @@ itest!(node_prefix_missing {
   envs: env_vars_for_npm_tests_no_sync_download(),
   exit_code: 1,
 });
+
+itest!(internal_import {
+  args: "run run/internal_import.ts",
+  output: "run/internal_import.ts.out",
+  exit_code: 1,
+});
+
+itest!(internal_dynamic_import {
+  args: "run run/internal_dynamic_import.ts",
+  output: "run/internal_dynamic_import.ts.out",
+  exit_code: 1,
+});
diff --git a/cli/tests/testdata/run/event_listener_error_immediate_exit.ts.out b/cli/tests/testdata/run/event_listener_error_immediate_exit.ts.out
@@ -1,5 +1,4 @@
 1
-queueMicrotask
 error: Uncaught Error: bar
   throw new Error("bar");
         ^

diff --git a/cli/tests/testdata/run/internal_dynamic_import.ts b/cli/tests/testdata/run/internal_dynamic_import.ts
@@ -0,0 +1 @@
+await import("internal:runtime/js/01_build.js");
diff --git a/cli/tests/testdata/run/internal_dynamic_import.ts.out b/cli/tests/testdata/run/internal_dynamic_import.ts.out
@@ -0,0 +1,4 @@
+error: Uncaught TypeError: Cannot load internal module from external code
+await import("internal:runtime/js/01_build.js");
+^
+    at [WILDCARD]/internal_dynamic_import.ts:1:1
diff --git a/cli/tests/testdata/run/internal_import.ts b/cli/tests/testdata/run/internal_import.ts
@@ -0,0 +1 @@
+import "internal:runtime/js/01_build.js";
diff --git a/cli/tests/testdata/run/internal_import.ts.out b/cli/tests/testdata/run/internal_import.ts.out
@@ -0,0 +1,8 @@
+error: Unsupported scheme "internal" for module "internal:runtime/js/01_build.js". Supported schemes: [
+    "data",
+    "blob",
+    "file",
+    "http",
+    "https",
+]
+    at [WILDCARD]
diff --git a/core/01_core.js b/core/01_core.js
@@ -427,6 +427,8 @@
   });
 
   ObjectAssign(globalThis.__bootstrap, { core });
+  const internals = {};
+  ObjectAssign(globalThis.__bootstrap, { internals });
   ObjectAssign(globalThis.Deno, { core });
 
   // Direct bindings on `globalThis`

diff --git a/core/bindings.rs b/core/bindings.rs
@@ -267,45 +267,47 @@ pub fn host_import_module_dynamically_callback<'s>(
     .unwrap()
     .to_rust_string_lossy(scope);
 
+  let is_internal_module = specifier_str.starts_with("internal:");
   let resolver = v8::PromiseResolver::new(scope).unwrap();
   let promise = resolver.get_promise(scope);
 
-  let assertions = parse_import_assertions(
-    scope,
-    import_assertions,
-    ImportAssertionsKind::DynamicImport,
-  );
+  if !is_internal_module {
+    let assertions = parse_import_assertions(
+      scope,
+      import_assertions,
+      ImportAssertionsKind::DynamicImport,
+    );
 
-  {
-    let tc_scope = &mut v8::TryCatch::new(scope);
-    validate_import_assertions(tc_scope, &assertions);
-    if tc_scope.has_caught() {
-      let e = tc_scope.exception().unwrap();
-      resolver.reject(tc_scope, e);
+    {
+      let tc_scope = &mut v8::TryCatch::new(scope);
+      validate_import_assertions(tc_scope, &assertions);
+      if tc_scope.has_caught() {
+        let e = tc_scope.exception().unwrap();
+        resolver.reject(tc_scope, e);
+      }
     }
-  }
-  let asserted_module_type =
-    get_asserted_module_type_from_assertions(&assertions);
+    let asserted_module_type =
+      get_asserted_module_type_from_assertions(&assertions);
 
-  let resolver_handle = v8::Global::new(scope, resolver);
-  {
-    let state_rc = JsRuntime::state(scope);
-    let module_map_rc = JsRuntime::module_map(scope);
+    let resolver_handle = v8::Global::new(scope, resolver);
+    {
+      let state_rc = JsRuntime::state(scope);
+      let module_map_rc = JsRuntime::module_map(scope);
 
-    debug!(
-      "dyn_import specifier {} referrer {} ",
-      specifier_str, referrer_name_str
-    );
-    ModuleMap::load_dynamic_import(
-      module_map_rc,
-      &specifier_str,
-      &referrer_name_str,
-      asserted_module_type,
-      resolver_handle,
-    );
-    state_rc.borrow_mut().notify_new_dynamic_import();
+      debug!(
+        "dyn_import specifier {} referrer {} ",
+        specifier_str, referrer_name_str
+      );
+      ModuleMap::load_dynamic_import(
+        module_map_rc,
+        &specifier_str,
+        &referrer_name_str,
+        asserted_module_type,
+        resolver_handle,
+      );
+      state_rc.borrow_mut().notify_new_dynamic_import();
+    }
   }
-
   // Map errors from module resolution (not JS errors from module execution) to
   // ones rethrown from this scope, so they include the call stack of the
   // dynamic import site. Error objects without any stack frames are assumed to
@@ -317,6 +319,14 @@ pub fn host_import_module_dynamically_callback<'s>(
 
   let promise = promise.catch(scope, map_err).unwrap();
 
+  if is_internal_module {
+    let message =
+      v8::String::new(scope, "Cannot load internal module from external code")
+        .unwrap();
+    let exception = v8::Exception::type_error(scope, message);
+    resolver.reject(scope, exception);
+  }
+
   Some(promise)
 }
 

diff --git a/core/extensions.rs b/core/extensions.rs
@@ -38,6 +38,7 @@ impl OpDecl {
 #[derive(Default)]
 pub struct Extension {
   js_files: Option<Vec<SourcePair>>,
+  esm_files: Option<Vec<SourcePair>>,
   ops: Option<Vec<OpDecl>>,
   opstate_fn: Option<Box<OpStateFn>>,
   middleware_fn: Option<Box<OpMiddlewareFn>>,
@@ -81,13 +82,20 @@ impl Extension {
 
   /// returns JS source code to be loaded into the isolate (either at snapshotting,
   /// or at startup).  as a vector of a tuple of the file name, and the source code.
-  pub fn init_js(&self) -> &[SourcePair] {
+  pub fn get_js_sources(&self) -> &[SourcePair] {
     match &self.js_files {
       Some(files) => files,
       None => &[],
     }
   }
 
+  pub fn get_esm_sources(&self) -> &[SourcePair] {
+    match &self.esm_files {
+      Some(files) => files,
+      None => &[],
+    }
+  }
+
   /// Called at JsRuntime startup to initialize ops in the isolate.
   pub fn init_ops(&mut self) -> Option<Vec<OpDecl>> {
     // TODO(@AaronO): maybe make op registration idempotent
@@ -145,6 +153,7 @@ impl Extension {
 #[derive(Default)]
 pub struct ExtensionBuilder {
   js: Vec<SourcePair>,
+  esm: Vec<SourcePair>,
   ops: Vec<OpDecl>,
   state: Option<Box<OpStateFn>>,
   middleware: Option<Box<OpMiddlewareFn>>,
@@ -164,6 +173,11 @@ impl ExtensionBuilder {
     self
   }
 
+  pub fn esm(&mut self, js_files: Vec<SourcePair>) -> &mut Self {
+    self.esm.extend(js_files);
+    self
+  }
+
   pub fn ops(&mut self, ops: Vec<OpDecl>) -> &mut Self {
     self.ops.extend(ops);
     self
@@ -195,10 +209,12 @@ impl ExtensionBuilder {
 
   pub fn build(&mut self) -> Extension {
     let js_files = Some(std::mem::take(&mut self.js));
+    let esm_files = Some(std::mem::take(&mut self.esm));
     let ops = Some(std::mem::take(&mut self.ops));
     let deps = Some(std::mem::take(&mut self.deps));
     Extension {
       js_files,
+      esm_files,
       ops,
       opstate_fn: self.state.take(),
       middleware_fn: self.middleware.take(),

diff --git a/core/lib.rs b/core/lib.rs
@@ -73,6 +73,7 @@ pub use crate::module_specifier::ModuleResolutionError;
 pub use crate::module_specifier::ModuleSpecifier;
 pub use crate::module_specifier::DUMMY_SPECIFIER;
 pub use crate::modules::FsModuleLoader;
+pub use crate::modules::InternalModuleLoader;
 pub use crate::modules::ModuleId;
 pub use crate::modules::ModuleLoader;
 pub use crate::modules::ModuleSource;

diff --git a/core/modules.rs b/core/modules.rs
@@ -292,6 +292,69 @@ impl ModuleLoader for NoopModuleLoader {
   }
 }
 
+pub struct InternalModuleLoader(Rc<dyn ModuleLoader>);
+
+impl InternalModuleLoader {
+  pub fn new(module_loader: Option<Rc<dyn ModuleLoader>>) -> Self {
+    InternalModuleLoader(
+      module_loader.unwrap_or_else(|| Rc::new(NoopModuleLoader)),
+    )
+  }
+}
+
+impl ModuleLoader for InternalModuleLoader {
+  fn resolve(
+    &self,
+    specifier: &str,
+    referrer: &str,
+    kind: ResolutionKind,
+  ) -> Result<ModuleSpecifier, Error> {
+    if let Ok(url_specifier) = ModuleSpecifier::parse(specifier) {
+      if url_specifier.scheme() == "internal" {
+        let referrer_specifier = ModuleSpecifier::parse(referrer).ok();
+        if referrer == "." || referrer_specifier.unwrap().scheme() == "internal"
+        {
+          return Ok(url_specifier);
+        } else {
+          return Err(generic_error(
+            "Cannot load internal module from external code",
+          ));
+        };
+      }
+    }
+
+    self.0.resolve(specifier, referrer, kind)
+  }
+
+  fn load(
+    &self,
+    module_specifier: &ModuleSpecifier,
+    maybe_referrer: Option<ModuleSpecifier>,
+    is_dyn_import: bool,
+  ) -> Pin<Box<ModuleSourceFuture>> {
+    self.0.load(module_specifier, maybe_referrer, is_dyn_import)
+  }
+
+  fn prepare_load(
+    &self,
+    op_state: Rc<RefCell<OpState>>,
+    module_specifier: &ModuleSpecifier,
+    maybe_referrer: Option<String>,
+    is_dyn_import: bool,
+  ) -> Pin<Box<dyn Future<Output = Result<(), Error>>>> {
+    if module_specifier.scheme() == "internal" {
+      return async { Ok(()) }.boxed_local();
+    }
+
+    self.0.prepare_load(
+      op_state,
+      module_specifier,
+      maybe_referrer,
+      is_dyn_import,
+    )
+  }
+}
+
 /// Basic file system module loader.
 ///
 /// Note that this loader will **block** event loop
@@ -2508,4 +2571,33 @@ if (import.meta.url != 'file:///main_with_code.js') throw Error();
       )
       .unwrap();
   }
+
+  #[test]
+  fn internal_module_loader() {
+    let loader = InternalModuleLoader::new(None);
+    assert!(loader
+      .resolve("internal:foo", "internal:bar", ResolutionKind::Import)
+      .is_ok());
+    assert_eq!(
+      loader
+        .resolve("internal:foo", "file://bar", ResolutionKind::Import)
+        .err()
+        .map(|e| e.to_string()),
+      Some("Cannot load internal module from external code".to_string())
+    );
+    assert_eq!(
+      loader
+        .resolve("file://foo", "file://bar", ResolutionKind::Import)
+        .err()
+        .map(|e| e.to_string()),
+      Some("Module loading is not supported".to_string())
+    );
+    assert_eq!(
+      loader
+        .resolve("file://foo", "internal:bar", ResolutionKind::Import)
+        .err()
+        .map(|e| e.to_string()),
+      Some("Module loading is not supported".to_string())
+    );
+  }
 }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		await import("internal:runtime/js/01_build.js");