refactor(core): don't use extension macro for core js (#19616)

Seems like too much of a special case because `init_cbs()` needs to be called right after them. Removes the `Extension::is_core` stuff.
denoland · Jul 1, 2023 · 8f8dd82 · 8f8dd82
1 parent 79ed162
commit 8f8dd82
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 74 deletions.
diff --git a/core/extensions.rs b/core/extensions.rs
@@ -174,7 +174,6 @@ macro_rules! ops {
 ///  * bounds: a comma-separated list of additional type bounds, eg: `bounds = [ P::MyAssociatedType: MyTrait ]`
 ///  * ops: a comma-separated list of [`OpDecl`]s to provide, eg: `ops = [ op_foo, op_bar ]`
 ///  * esm: a comma-separated list of ESM module filenames (see [`include_js_files`]), eg: `esm = [ dir "dir", "my_file.js" ]`
-///  * esm_setup_script: see [`ExtensionBuilder::esm_setup_script`]
 ///  * js: a comma-separated list of JS filenames (see [`include_js_files`]), eg: `js = [ dir "dir", "my_file.js" ]`
 ///  * config: a structure-like definition for configuration parameters which will be required when initializing this extension, eg: `config = { my_param: Option<usize> }`
 ///  * middleware: an [`OpDecl`] middleware function with the signature `fn (OpDecl) -> OpDecl`
@@ -191,7 +190,6 @@ macro_rules! extension {
     $(, ops = [ $( $(#[$m:meta])* $( $op:ident )::+ $( < $( $op_param:ident ),* > )?  ),+ $(,)? ] )?
     $(, esm_entry_point = $esm_entry_point:literal )?
     $(, esm = [ $( dir $dir_esm:literal , )? $( $esm:literal ),* $(,)? ] )?
-    $(, esm_setup_script = $esm_setup_script:expr )?
     $(, js = [ $( dir $dir_js:literal , )? $( $js:literal ),* $(,)? ] )?
     $(, options = { $( $options_id:ident : $options_type:ty ),* $(,)? } )?
     $(, middleware = $middleware_fn:expr )?
@@ -220,12 +218,6 @@ macro_rules! extension {
         $( ext.esm(
           $crate::include_js_files!( $name $( dir $dir_esm , )? $( $esm , )* )
         ); )?
-        $(
-          ext.esm(vec![ExtensionFileSource {
-            specifier: "ext:setup",
-            code: ExtensionFileSourceCode::IncludedInBinary($esm_setup_script),
-          }]);
-        )?
         $(
           ext.esm_entry_point($esm_entry_point);
         )?
@@ -355,8 +347,8 @@ macro_rules! extension {
 #[derive(Default)]
 pub struct Extension {
   pub(crate) name: &'static str,
-  js_files: Option<Vec<ExtensionFileSource>>,
-  esm_files: Option<Vec<ExtensionFileSource>>,
+  js_files: Vec<ExtensionFileSource>,
+  esm_files: Vec<ExtensionFileSource>,
   esm_entry_point: Option<&'static str>,
   ops: Option<Vec<OpDecl>>,
   opstate_fn: Option<Box<OpStateFn>>,
@@ -365,7 +357,6 @@ pub struct Extension {
   initialized: bool,
   enabled: bool,
   deps: Option<&'static [&'static str]>,
-  pub(crate) is_core: bool,
 }
 
 // Note: this used to be a trait, but we "downgraded" it to a single concrete type
@@ -412,12 +403,12 @@ 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 get_js_sources(&self) -> Option<&Vec<ExtensionFileSource>> {
-    self.js_files.as_ref()
+  pub fn get_js_sources(&self) -> &Vec<ExtensionFileSource> {
+    &self.js_files
   }
 
-  pub fn get_esm_sources(&self) -> Option<&Vec<ExtensionFileSource>> {
-    self.esm_files.as_ref()
+  pub fn get_esm_sources(&self) -> &Vec<ExtensionFileSource> {
+    &self.esm_files
   }
 
   pub fn get_esm_entry_point(&self) -> Option<&'static str> {
@@ -488,7 +479,6 @@ pub struct ExtensionBuilder {
   event_loop_middleware: Option<Box<OpEventLoopFn>>,
   name: &'static str,
   deps: &'static [&'static str],
-  is_core: bool,
 }
 
 impl ExtensionBuilder {
@@ -538,13 +528,11 @@ impl ExtensionBuilder {
 
   /// Consume the [`ExtensionBuilder`] and return an [`Extension`].
   pub fn take(self) -> Extension {
-    let js_files = Some(self.js);
-    let esm_files = Some(self.esm);
     let ops = Some(self.ops);
     let deps = Some(self.deps);
     Extension {
-      js_files,
-      esm_files,
+      js_files: self.js,
+      esm_files: self.esm,
       esm_entry_point: self.esm_entry_point,
       ops,
       opstate_fn: self.state,
@@ -554,18 +542,15 @@ impl ExtensionBuilder {
       enabled: true,
       name: self.name,
       deps,
-      is_core: self.is_core,
     }
   }
 
   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,
+      js_files: std::mem::take(&mut self.js),
+      esm_files: std::mem::take(&mut self.esm),
       esm_entry_point: self.esm_entry_point.take(),
       ops,
       opstate_fn: self.state.take(),
@@ -575,15 +560,8 @@ impl ExtensionBuilder {
       enabled: true,
       name: self.name,
       deps,
-      is_core: self.is_core,
     }
   }
-
-  #[doc(hidden)]
-  pub(crate) fn deno_core(&mut self) -> &mut Self {
-    self.is_core = true;
-    self
-  }
 }
 
 /// Helps embed JS files in an extension. Returns a vector of

diff --git a/core/modules/loaders.rs b/core/modules/loaders.rs
@@ -119,7 +119,6 @@ impl ExtModuleLoader {
       extensions
         .iter()
         .flat_map(|e| e.get_esm_sources())
-        .flatten()
         .map(|s| (s.specifier.to_string(), s.clone())),
     );
     ExtModuleLoader {

diff --git a/core/ops_builtin.rs b/core/ops_builtin.rs
@@ -79,10 +79,6 @@ crate::extension!(
     ops_builtin_v8::op_has_pending_promise_rejection,
     ops_builtin_v8::op_arraybuffer_was_detached,
   ],
-  js = ["00_primordials.js", "01_core.js", "02_error.js"],
-  customizer = |ext: &mut crate::ExtensionBuilder| {
-    ext.deno_core();
-  }
 );
 
 /// Return map of resources with id as key

diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
@@ -10,6 +10,7 @@ use crate::error::GetErrorClassFn;
 use crate::error::JsError;
 use crate::extensions::OpDecl;
 use crate::extensions::OpEventLoopFn;
+use crate::include_js_files;
 use crate::inspector::JsRuntimeInspector;
 use crate::module_specifier::ModuleSpecifier;
 use crate::modules::AssertedModuleType;
@@ -27,6 +28,7 @@ use crate::runtime::JsRealm;
 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;
@@ -38,6 +40,7 @@ use anyhow::Error;
 use futures::channel::oneshot;
 use futures::future::poll_fn;
 use futures::stream::StreamExt;
+use once_cell::sync::Lazy;
 use smallvec::SmallVec;
 use std::any::Any;
 use std::cell::RefCell;
@@ -196,6 +199,16 @@ impl InitMode {
   }
 }
 
+pub(crate) static BUILTIN_SOURCES: Lazy<Vec<ExtensionFileSource>> =
+  Lazy::new(|| {
+    include_js_files!(
+      core
+      "00_primordials.js",
+      "01_core.js",
+      "02_error.js",
+    )
+  });
+
 /// A single execution context of JavaScript. Corresponds roughly to the "Web
 /// Worker" concept in the DOM.
 ////
@@ -504,7 +517,7 @@ impl JsRuntime {
     maybe_load_callback: Option<ExtModuleLoaderCb>,
   ) -> JsRuntime {
     let init_mode = InitMode::from_options(&options);
-    let (op_state, ops) = Self::create_opstate(&mut options, init_mode);
+    let (op_state, ops) = Self::create_opstate(&mut options);
     let op_state = Rc::new(RefCell::new(op_state));
 
     // Collect event-loop middleware
@@ -840,36 +853,39 @@ impl JsRuntime {
     let mut esm_entrypoints = vec![];
 
     futures::executor::block_on(async {
+      if self.init_mode == InitMode::New {
+        for file_source in &*BUILTIN_SOURCES {
+          realm.execute_script(
+            self.v8_isolate(),
+            file_source.specifier,
+            file_source.load()?,
+          )?;
+        }
+      }
+      self.init_cbs(realm);
+
       for extension in &extensions {
         let maybe_esm_entry_point = extension.get_esm_entry_point();
 
-        if let Some(esm_files) = extension.get_esm_sources() {
-          for file_source in esm_files {
-            self
-              .load_side_module(
-                &ModuleSpecifier::parse(file_source.specifier)?,
-                None,
-              )
-              .await?;
-          }
+        for file_source in extension.get_esm_sources() {
+          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);
         }
 
-        if let Some(js_files) = extension.get_js_sources() {
-          for file_source in js_files {
-            realm.execute_script(
-              self.v8_isolate(),
-              file_source.specifier,
-              file_source.load()?,
-            )?;
-          }
-        }
-
-        if extension.is_core {
-          self.init_cbs(realm);
+        for file_source in extension.get_js_sources() {
+          realm.execute_script(
+            self.v8_isolate(),
+            file_source.specifier,
+            file_source.load()?,
+          )?;
         }
       }
 
@@ -966,20 +982,11 @@ impl JsRuntime {
   }
 
   /// Initializes ops of provided Extensions
-  fn create_opstate(
-    options: &mut RuntimeOptions,
-    init_mode: InitMode,
-  ) -> (OpState, Vec<OpDecl>) {
+  fn create_opstate(options: &mut RuntimeOptions) -> (OpState, Vec<OpDecl>) {
     // Add built-in extension
-    if init_mode == InitMode::FromSnapshot {
-      options
-        .extensions
-        .insert(0, crate::ops_builtin::core::init_ops());
-    } else {
-      options
-        .extensions
-        .insert(0, crate::ops_builtin::core::init_ops_and_esm());
-    }
+    options
+      .extensions
+      .insert(0, crate::ops_builtin::core::init_ops());
 
     let ops = Self::collect_ops(&mut options.extensions);
 

diff --git a/core/runtime/snapshot_util.rs b/core/runtime/snapshot_util.rs
@@ -4,9 +4,11 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::time::Instant;
 
+use crate::runtime::jsruntime::BUILTIN_SOURCES;
 use crate::runtime::RuntimeSnapshotOptions;
 use crate::ExtModuleLoaderCb;
 use crate::Extension;
+use crate::ExtensionFileSourceCode;
 use crate::JsRuntimeForSnapshot;
 use crate::RuntimeOptions;
 use crate::Snapshot;
@@ -52,14 +54,19 @@ pub fn create_snapshot(
   mark = Instant::now();
 
   let mut files_loaded_during_snapshot = vec![];
+  for source in &*BUILTIN_SOURCES {
+    if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) =
+      &source.code
+    {
+      files_loaded_during_snapshot.push(path.clone());
+    }
+  }
   for source in js_runtime
     .extensions()
     .iter()
     .flat_map(|e| vec![e.get_esm_sources(), e.get_js_sources()])
     .flatten()
-    .flatten()
   {
-    use crate::ExtensionFileSourceCode;
     if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) =
       &source.code
     {