Skip to content

Commit

Permalink
refactor: Use ES modules for internal runtime code (#17648)
Browse files Browse the repository at this point in the history
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 <biwanczuk@gmail.com>
  • Loading branch information
crowlKats and bartlomieju committed Feb 7, 2023
1 parent 65500f3 commit b4aa153
Show file tree
Hide file tree
Showing 123 changed files with 39,975 additions and 40,114 deletions.
3 changes: 3 additions & 0 deletions bench_util/js_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
}
Expand Down
14 changes: 8 additions & 6 deletions cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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")]
{
Expand Down
2,509 changes: 1,253 additions & 1,256 deletions cli/js/40_testing.js

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
1
queueMicrotask
error: Uncaught Error: bar
throw new Error("bar");
^
Expand Down
1 change: 1 addition & 0 deletions cli/tests/testdata/run/internal_dynamic_import.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
await import("internal:runtime/js/01_build.js");
4 changes: 4 additions & 0 deletions cli/tests/testdata/run/internal_dynamic_import.ts.out
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions cli/tests/testdata/run/internal_import.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "internal:runtime/js/01_build.js";
8 changes: 8 additions & 0 deletions cli/tests/testdata/run/internal_import.ts.out
Original file line number Diff line number Diff line change
@@ -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]
2 changes: 2 additions & 0 deletions core/01_core.js
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@
});

ObjectAssign(globalThis.__bootstrap, { core });
const internals = {};
ObjectAssign(globalThis.__bootstrap, { internals });
ObjectAssign(globalThis.Deno, { core });

// Direct bindings on `globalThis`
Expand Down
72 changes: 41 additions & 31 deletions core/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}

Expand Down
18 changes: 17 additions & 1 deletion core/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>>,
Expand All @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
92 changes: 92 additions & 0 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
);
}
}
Loading

0 comments on commit b4aa153

Please sign in to comment.