Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for import assertions and JSON modules #12866

Merged
merged 43 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
a5b25bb
add deno_core::ModuleType enum
bartlomieju Sep 24, 2021
0c97bcf
remove atomic in core/modules.rs
bartlomieju Sep 24, 2021
a4ec823
Merge branch 'main' into import_assertions
bartlomieju Oct 5, 2021
2cb253c
Merge branch 'main' into import_assertions
bartlomieju Nov 22, 2021
53016b0
Merge branch 'main' into import_assertions
bartlomieju Dec 9, 2021
cf72c70
Merge branch 'main' into import_assertions
bartlomieju Dec 11, 2021
d8eeb8f
Merge branch 'main' into import_assertions
bartlomieju Dec 12, 2021
4098ebd
use ModuleType
bartlomieju Dec 12, 2021
a1c5f54
check passed assertion value
bartlomieju Dec 12, 2021
1e6f134
validate import assertions
bartlomieju Dec 12, 2021
795dcce
example(core): add example for FS module loading
bartlomieju Dec 12, 2021
cf7c69d
fix cargo lock, add ModuleType::Json
bartlomieju Dec 12, 2021
ce0117f
fix
bartlomieju Dec 12, 2021
f06d9ba
Merge branch 'core_example_loading' into import_assertions
bartlomieju Dec 12, 2021
4b0cf46
update WPT expectations
bartlomieju Dec 12, 2021
694ae08
Merge branch 'main' into import_assertions
bartlomieju Dec 13, 2021
73f20db
JSON module evaluation
bartlomieju Dec 13, 2021
f881a94
fmt
bartlomieju Dec 14, 2021
54fa3e8
Merge branch 'main' into import_assertions
bartlomieju Dec 14, 2021
2cabd3c
parsing of assertions
bartlomieju Dec 15, 2021
e4ad657
add ModuleRequest struct
bartlomieju Dec 15, 2021
acc2ac0
import functions
bartlomieju Dec 15, 2021
b1ae3f2
wire up in the CLI
bartlomieju Dec 15, 2021
4535edb
fix validation of assertions
bartlomieju Dec 15, 2021
6a549ca
lint
bartlomieju Dec 15, 2021
1cb9bdf
add tests, set resolveJsonModule in tsc
bartlomieju Dec 15, 2021
37a464c
review comments
bartlomieju Dec 15, 2021
da0ba53
update TS config for LSP
bartlomieju Dec 15, 2021
6c8be58
add wildcard to test output
bartlomieju Dec 15, 2021
519de28
work around bug in deno_ast
kitsonk Dec 15, 2021
853982d
revert change to display
bartlomieju Dec 15, 2021
48590d8
ignore emit for json modules
kitsonk Dec 15, 2021
9627341
update output assertions
bartlomieju Dec 15, 2021
fb8295f
add test for type checking JSON module
bartlomieju Dec 15, 2021
98fdbf7
fmt
bartlomieju Dec 15, 2021
c533c65
use SyntaxError for invalid JSON
bartlomieju Dec 15, 2021
3fe6133
update WPT
bartlomieju Dec 15, 2021
50e9299
update WPT2
bartlomieju Dec 15, 2021
0428fa6
lint
bartlomieju Dec 15, 2021
265a408
pass another WPT test
bartlomieju Dec 15, 2021
fab5d86
review comments
bartlomieju Dec 15, 2021
a5dbc9c
fix tests
bartlomieju Dec 15, 2021
f0662ff
Merge branch 'main' into import_assertions
bartlomieju Dec 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use deno_core::url::Url;
use deno_core::CompiledWasmModuleStore;
use deno_core::ModuleSource;
use deno_core::ModuleSpecifier;
use deno_core::ModuleType;
use deno_core::SharedArrayBufferStore;
use deno_graph::create_graph;
use deno_graph::Dependency;
Expand Down Expand Up @@ -722,6 +723,9 @@ impl ProcState {
code: code.clone(),
module_url_specified: specifier.to_string(),
module_url_found: found_specifier.to_string(),
// FIXME(bartlomieju): this should actually get module type from
// the media type of the file.
module_type: ModuleType::JavaScript,
}),
_ => Err(anyhow!(
"Loading unprepared module: {}",
Expand Down
1 change: 1 addition & 0 deletions cli/standalone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ impl ModuleLoader for EmbeddedModuleLoader {

Ok(deno_core::ModuleSource {
code,
module_type: deno_core::ModuleType::JavaScript,
module_url_specified: module_specifier.to_string(),
module_url_found: module_specifier.to_string(),
})
Expand Down
29 changes: 28 additions & 1 deletion core/bindings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.

use crate::error::is_instance_of_error;
use crate::modules::validate_import_assertions;
use crate::modules::ModuleMap;
use crate::resolve_url_or_path;
use crate::JsRuntime;
Expand All @@ -17,6 +18,7 @@ use serde::Deserialize;
use serde::Serialize;
use serde_v8::to_v8;
use std::cell::RefCell;
use std::collections::HashMap;
use std::option::Option;
use url::Url;
use v8::HandleScope;
Expand Down Expand Up @@ -243,7 +245,7 @@ pub extern "C" fn host_import_module_dynamically_callback(
context: v8::Local<v8::Context>,
referrer: v8::Local<v8::ScriptOrModule>,
specifier: v8::Local<v8::String>,
_import_assertions: v8::Local<v8::FixedArray>,
import_assertions: v8::Local<v8::FixedArray>,
) -> *mut v8::Promise {
let scope = &mut unsafe { v8::CallbackScope::new(context) };

Expand All @@ -267,6 +269,31 @@ pub extern "C" fn host_import_module_dynamically_callback(
let resolver = v8::PromiseResolver::new(scope).unwrap();
let promise = resolver.get_promise(scope);

let mut assertions: HashMap<String, String> = HashMap::default();
// "type" keyword, value
let no_of_assertions = import_assertions.length() / 2;
for i in 0..no_of_assertions {
let assert_key = import_assertions.get(scope, 2 * i).unwrap();
let assert_key_val = v8::Local::<v8::Value>::try_from(assert_key).unwrap();
let assert_value = import_assertions.get(scope, (2 * i) + 1).unwrap();
let assert_value_val =
v8::Local::<v8::Value>::try_from(assert_value).unwrap();
// skip parsing offset from assertion
assertions.insert(
assert_key_val.to_rust_string_lossy(scope),
assert_value_val.to_rust_string_lossy(scope),
);
}

{
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 resolver_handle = v8::Global::new(scope, resolver);
{
let state_rc = JsRuntime::state(scope);
Expand Down
37 changes: 37 additions & 0 deletions core/examples/fs_module_loader.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.

use deno_core::anyhow::Error;
use deno_core::FsModuleLoader;
use deno_core::JsRuntime;
use deno_core::RuntimeOptions;
use std::rc::Rc;

fn main() -> Result<(), Error> {
let args: Vec<String> = std::env::args().collect();
if args.len() < 2 {
println!("Usage: target/examples/debug/fs_module_loader <path_to_module>");
std::process::exit(1);
}
let main_url = args[1].clone();
println!("Run {}", main_url);

let mut js_runtime = JsRuntime::new(RuntimeOptions {
module_loader: Some(Rc::new(FsModuleLoader)),
..Default::default()
});

let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()?;

let main_module = deno_core::resolve_path(&main_url)?;

let future = async move {
let mod_id = js_runtime.load_main_module(&main_module, None).await?;

let mut receiver = js_runtime.mod_evaluate(mod_id);
js_runtime.run_event_loop(false).await?;
Ok(())
};
runtime.block_on(future)
}
1 change: 1 addition & 0 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub use crate::modules::ModuleLoadId;
pub use crate::modules::ModuleLoader;
pub use crate::modules::ModuleSource;
pub use crate::modules::ModuleSourceFuture;
pub use crate::modules::ModuleType;
pub use crate::modules::NoopModuleLoader;
pub use crate::runtime::CompiledWasmModuleStore;
pub use crate::runtime::SharedArrayBufferStore;
Expand Down
104 changes: 97 additions & 7 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,50 @@ use std::collections::VecDeque;
use std::future::Future;
use std::pin::Pin;
use std::rc::Rc;
use std::sync::atomic::AtomicI32;
use std::sync::atomic::Ordering;
use std::task::Context;
use std::task::Poll;

lazy_static::lazy_static! {
pub static ref NEXT_LOAD_ID: AtomicI32 = AtomicI32::new(0);
}

pub type ModuleId = i32;
pub type ModuleLoadId = i32;

/// Throws V8 exception if assertions are invalid
pub(crate) fn validate_import_assertions(
scope: &mut v8::HandleScope,
assertions: HashMap<String, String>,
) {
for (key, value) in assertions {
if key == "type" {
// TODO(bartlomieju): store in a const list of supported values
if value != "json" {
let message = v8::String::new(
scope,
&format!("\"{}\" is not a valid module type.", value),
)
.unwrap();
let exception = v8::Exception::type_error(scope, message);
scope.throw_exception(exception);
return;
}
}
}
}

/// A type of module to be executed.
///
/// Currently only `JavaScript` variant is supported,
/// but in future types like `JSON` and `WASM` will be
/// supported.
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
///
/// For non-`JavaScript` modules, this value doesn't tell
/// how to interpret the module; it is only used to validate
/// the module against an import assertion (if one is present
/// in the import statement).
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum ModuleType {
JavaScript,
Json,
}

/// EsModule source code that will be loaded into V8.
///
/// Users can implement `Into<ModuleInfo>` for different file types that
Expand All @@ -49,6 +81,7 @@ pub type ModuleLoadId = i32;
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct ModuleSource {
pub code: String,
pub module_type: ModuleType,
pub module_url_specified: String,
pub module_url_found: String,
}
Expand Down Expand Up @@ -159,9 +192,20 @@ impl ModuleLoader for FsModuleLoader {
module_specifier
))
})?;
let module_type = if let Some(extension) = path.extension() {
if extension == "json" {
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
ModuleType::Json
} else {
ModuleType::JavaScript
}
} else {
ModuleType::JavaScript
};
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved

let code = std::fs::read_to_string(path)?;
let module = ModuleSource {
code,
module_type,
module_url_specified: module_specifier.to_string(),
module_url_found: module_specifier.to_string(),
};
Expand Down Expand Up @@ -232,10 +276,16 @@ impl RecursiveModuleLoad {
}

fn new(init: LoadInit, module_map_rc: Rc<RefCell<ModuleMap>>) -> Self {
let id = {
let mut module_map = module_map_rc.borrow_mut();
let id = module_map.next_load_id;
module_map.next_load_id += 1;
id
};
let op_state = module_map_rc.borrow().op_state.clone();
let loader = module_map_rc.borrow().loader.clone();
let mut load = Self {
id: NEXT_LOAD_ID.fetch_add(1, Ordering::SeqCst),
id,
root_module_id: None,
init,
state: LoadState::Init,
Expand Down Expand Up @@ -419,6 +469,7 @@ impl Stream for RecursiveModuleLoad {
// The code will be discarded, since this module is already in the
// module map.
code: Default::default(),
module_type: ModuleType::JavaScript,
})
.boxed()
} else {
Expand Down Expand Up @@ -475,6 +526,7 @@ pub struct ModuleMap {
info: HashMap<ModuleId, ModuleInfo>,
by_name: HashMap<String, SymbolicModule>,
next_module_id: ModuleId,
next_load_id: ModuleLoadId,

// Handling of futures for loading module sources
pub loader: Rc<dyn ModuleLoader>,
Expand All @@ -498,6 +550,7 @@ impl ModuleMap {
info: HashMap::new(),
by_name: HashMap::new(),
next_module_id: 1,
next_load_id: 1,
loader,
op_state,
dynamic_import_map: HashMap::new(),
Expand Down Expand Up @@ -557,6 +610,38 @@ impl ModuleMap {
let import_specifier = module_request
.get_specifier()
.to_rust_string_lossy(tc_scope);

let import_assertions = module_request.get_import_assertions();

let mut assertions = HashMap::default();

// "type" keyword, value and source offset of assertion
let no_of_assertions = import_assertions.length() / 3;
for i in 0..no_of_assertions {
let assert_key = import_assertions.get(tc_scope, 3 * i).unwrap();
let assert_key_val =
v8::Local::<v8::Value>::try_from(assert_key).unwrap();
let assert_key_str = assert_key_val.to_rust_string_lossy(tc_scope);

let assert_value =
import_assertions.get(tc_scope, (3 * i) + 1).unwrap();
let assert_value_val =
v8::Local::<v8::Value>::try_from(assert_value).unwrap();
let assert_value_str = assert_value_val.to_rust_string_lossy(tc_scope);

// we're not interested in source code offset, so skipping it

assertions.insert(assert_key_str, assert_value_str);
}

// FIXME(bartomieju): there are no stack frames if exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something to be fixed in this review or later?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later, it's not clear how to fix it

// is thrown here
validate_import_assertions(tc_scope, assertions);
if tc_scope.has_caught() {
let e = tc_scope.exception().unwrap();
return exception_to_err_result(tc_scope, e, false);
}

let module_specifier =
self.loader.resolve(&import_specifier, name, false)?;
import_specifiers.push(module_specifier);
Expand Down Expand Up @@ -818,6 +903,7 @@ mod tests {
match mock_source_code(&inner.url) {
Some(src) => Poll::Ready(Ok(ModuleSource {
code: src.0.to_owned(),
module_type: ModuleType::JavaScript,
module_url_specified: inner.url.clone(),
module_url_found: src.1.to_owned(),
})),
Expand Down Expand Up @@ -1174,6 +1260,7 @@ mod tests {
module_url_specified: specifier.to_string(),
module_url_found: specifier.to_string(),
code: "export function b() { return 'b' }".to_owned(),
module_type: ModuleType::JavaScript,
};
async move { Ok(info) }.boxed()
}
Expand Down Expand Up @@ -1320,6 +1407,7 @@ mod tests {
module_url_specified: specifier.to_string(),
module_url_found: specifier.to_string(),
code: code.to_owned(),
module_type: ModuleType::JavaScript,
};
async move { Ok(info) }.boxed()
}
Expand Down Expand Up @@ -1648,11 +1736,13 @@ mod tests {
module_url_specified: "file:///main_module.js".to_string(),
module_url_found: "file:///main_module.js".to_string(),
code: "if (!import.meta.main) throw Error();".to_owned(),
module_type: ModuleType::JavaScript,
}),
"file:///side_module.js" => Ok(ModuleSource {
module_url_specified: "file:///side_module.js".to_string(),
module_url_found: "file:///side_module.js".to_string(),
code: "if (import.meta.main) throw Error();".to_owned(),
module_type: ModuleType::JavaScript,
}),
_ => unreachable!(),
};
Expand Down
2 changes: 2 additions & 0 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1630,6 +1630,7 @@ pub mod tests {
use crate::error::custom_error;
use crate::modules::ModuleSource;
use crate::modules::ModuleSourceFuture;
use crate::modules::ModuleType;
use crate::op_async;
use crate::op_sync;
use crate::ZeroCopyBuf;
Expand Down Expand Up @@ -2642,6 +2643,7 @@ assertEquals(1, notify_return_value);
code: "console.log('hello world');".to_string(),
module_url_specified: "file:///main.js".to_string(),
module_url_found: "file:///main.js".to_string(),
module_type: ModuleType::JavaScript,
})
}
.boxed_local()
Expand Down
9 changes: 1 addition & 8 deletions tools/wpt/expectation.json
Original file line number Diff line number Diff line change
Expand Up @@ -6834,14 +6834,7 @@
"semantics": {
"scripting-1": {
"the-script-element": {
"import-assertions": {
"dynamic-import-with-assertion-argument.any.html": [
"Dynamic import with an unsupported type assertion should fail"
],
"dynamic-import-with-assertion-argument.any.worker.html": [
"Dynamic import with an unsupported type assertion should fail"
]
},
"import-assertions": true,
"json-module": {
"charset-bom.any.html": [
"UTF-8 BOM should be stripped when decoding JSON module script",
Expand Down