refactor(core): definition of "ExtensionFileSource" (#17823)

This commit changes definition of "ExtensionFileSource", by changing
"code" field to being "ExtensionFileSourceCode" enum. Currently the enum
has only a single variant "IncludedInBinary". It is done in preparation
to allow embedders to decide if they want to include the source code in the
binary when snapshotting (in most cases they shouldn't do that).

In the follow up commit we'll add more variants to
"ExtensionFileSourceCode".

"include_js_files_dir!" macro was removed in favor "include_js_files!"
macro which can now accept "dir" option.

4 files changed, 66 insertions, 38 deletions

diff --git a/core/extensions.rs b/core/extensions.rs
index e497b8003..ab686d868 100644
--- a/core/extensions.rs
+++ b/core/extensions.rs
@@ -7,9 +7,20 @@ use std::task::Context;
 use v8::fast_api::FastFunction;
 
 #[derive(Clone, Debug)]
+pub enum ExtensionFileSourceCode {
+  /// Source code is included in the binary produced. Either by being defined
+  /// inline, or included using `include_str!()`. If you are snapshotting, this
+  /// will result in two copies of the source code being included - one in the
+  /// snapshot, the other the static string in the `Extension`.
+  IncludedInBinary(&'static str),
+  // TODO(bartlomieju): add more variants that allow to read file from the disk,
+  // and not include it in the binary.
+}
+
+#[derive(Clone, Debug)]
 pub struct ExtensionFileSource {
   pub specifier: String,
-  pub code: &'static str,
+  pub code: ExtensionFileSourceCode,
 }
 pub type OpFnRef = v8::FunctionCallback;
 pub type OpMiddlewareFn = dyn Fn(OpDecl) -> OpDecl;
@@ -180,6 +191,9 @@ impl ExtensionBuilder {
 
   pub fn js(&mut self, js_files: Vec<ExtensionFileSource>) -> &mut Self {
     let js_files =
+      // TODO(bartlomieju): if we're automatically remapping here, then we should
+      // use a different result struct that `ExtensionFileSource` as it's confusing
+      // when (and why) the remapping happens.
       js_files.into_iter().map(|file_source| ExtensionFileSource {
         specifier: format!("internal:{}/{}", self.name, file_source.specifier),
         code: file_source.code,
@@ -189,16 +203,15 @@ impl ExtensionBuilder {
   }
 
   pub fn esm(&mut self, esm_files: Vec<ExtensionFileSource>) -> &mut Self {
-    let esm_files =
-      esm_files
-        .into_iter()
-        .map(|file_source| ExtensionFileSource {
-          specifier: format!(
-            "internal:{}/{}",
-            self.name, file_source.specifier
-          ),
-          code: file_source.code,
-        });
+    let esm_files = esm_files
+      .into_iter()
+      // TODO(bartlomieju): if we're automatically remapping here, then we should
+      // use a different result struct that `ExtensionFileSource` as it's confusing
+      // when (and why) the remapping happens.
+      .map(|file_source| ExtensionFileSource {
+        specifier: format!("internal:{}/{}", self.name, file_source.specifier),
+        code: file_source.code,
+      });
     self.esm.extend(esm_files);
     self
   }
@@ -259,48 +272,53 @@ impl ExtensionBuilder {
 }
 
 /// Helps embed JS files in an extension. Returns a vector of
-/// `ExtensionFileSource`, that represent the filename and source code.
+/// `ExtensionFileSource`, that represent the filename and source code. All
+/// specified files are rewritten into "internal:<extension_name>/<file_name>".
 ///
-/// Example:
+/// An optional "dir" option can be specified to prefix all files with a
+/// directory name.
+///
+/// Example (for "my_extension"):
 /// ```ignore
 /// include_js_files!(
 ///   "01_hello.js",
 ///   "02_goodbye.js",
 /// )
-/// ```
-#[macro_export]
-macro_rules! include_js_files {
-  ($($file:literal,)+) => {
-    vec![
-      $($crate::ExtensionFileSource {
-        specifier: $file.to_string(),
-        code: include_str!($file),
-      },)+
-    ]
-  };
-}
-
-/// Helps embed JS files in an extension. Returns a vector of
-/// `ExtensionFileSource`, that represent the filename and source code.
-/// Additional "dir" option is required, that specifies which directory in the
-/// crate root contains the listed files. "dir" option will be prepended to
-/// each file name.
+/// // Produces following specifiers:
+/// - "internal:my_extension/01_hello.js"
+/// - "internal:my_extension/02_goodbye.js"
 ///
-/// Example:
+/// /// Example with "dir" option (for "my_extension"):
 /// ```ignore
-/// include_js_files_dir!(
-///   dir "example",
+/// include_js_files!(
+///   dir "js",
 ///   "01_hello.js",
 ///   "02_goodbye.js",
 /// )
+/// // Produces following specifiers:
+/// - "internal:my_extension/js/01_hello.js"
+/// - "internal:my_extension/js/02_goodbye.js"
 /// ```
 #[macro_export]
-macro_rules! include_js_files_dir {
+macro_rules! include_js_files {
   (dir $dir:literal, $($file:literal,)+) => {
     vec![
       $($crate::ExtensionFileSource {
         specifier: concat!($dir, "/", $file).to_string(),
-        code: include_str!(concat!($dir, "/", $file)),
+        code: $crate::ExtensionFileSourceCode::IncludedInBinary(
+          include_str!(concat!($dir, "/", $file)
+        )),
+      },)+
+    ]
+  };
+
+  ($($file:literal,)+) => {
+    vec![
+      $($crate::ExtensionFileSource {
+        specifier: $file.to_string(),
+        code: $crate::ExtensionFileSourceCode::IncludedInBinary(
+          include_str!($file)
+        ),
       },)+
     ]
   };
diff --git a/core/lib.rs b/core/lib.rs
index a77701339..51a03493d 100644
--- a/core/lib.rs
+++ b/core/lib.rs
@@ -55,6 +55,7 @@ pub use crate::async_cell::RcRef;
 pub use crate::extensions::Extension;
 pub use crate::extensions::ExtensionBuilder;
 pub use crate::extensions::ExtensionFileSource;
+pub use crate::extensions::ExtensionFileSourceCode;
 pub use crate::extensions::OpDecl;
 pub use crate::extensions::OpMiddlewareFn;
 pub use crate::flags::v8_set_flags;
diff --git a/core/modules.rs b/core/modules.rs
index 1f427508a..43dabf411 100644
--- a/core/modules.rs
+++ b/core/modules.rs
@@ -3,6 +3,7 @@
 use crate::bindings;
 use crate::error::generic_error;
 use crate::extensions::ExtensionFileSource;
+use crate::extensions::ExtensionFileSourceCode;
 use crate::module_specifier::ModuleSpecifier;
 use crate::resolve_import;
 use crate::resolve_url;
@@ -402,7 +403,11 @@ impl ModuleLoader for InternalModuleLoader {
       let result = if let Some(load_callback) = &self.maybe_load_callback {
         load_callback(file_source)
       } else {
-        Ok(file_source.code.to_string())
+        match file_source.code {
+          ExtensionFileSourceCode::IncludedInBinary(code) => {
+            Ok(code.to_string())
+          }
+        }
       };
 
       return async move {
diff --git a/core/runtime.rs b/core/runtime.rs
index 25a09e85e..ae2b0489b 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -21,6 +21,7 @@ use crate::source_map::SourceMapCache;
 use crate::source_map::SourceMapGetter;
 use crate::Extension;
 use crate::ExtensionFileSource;
+use crate::ExtensionFileSourceCode;
 use crate::NoopModuleLoader;
 use crate::OpMiddlewareFn;
 use crate::OpResult;
@@ -868,11 +869,14 @@ impl JsRuntime {
       {
         let js_files = ext.get_js_sources();
         for file_source in js_files {
+          let ExtensionFileSourceCode::IncludedInBinary(code) =
+            file_source.code;
+
           // TODO(@AaronO): use JsRuntime::execute_static() here to move src off heap
           realm.execute_script(
             self.v8_isolate(),
             &file_source.specifier,
-            file_source.code,
+            code,
           )?;
         }
       }