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 1 commit
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
Prev Previous commit
Next Next commit
add ModuleRequest struct
  • Loading branch information
bartlomieju committed Dec 15, 2021
commit e4ad65754715ada59af484519e8b13f90f90fe32
2 changes: 1 addition & 1 deletion core/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ pub extern "C" fn host_import_module_dynamically_callback(

{
let tc_scope = &mut v8::TryCatch::new(scope);
validate_import_assertions(tc_scope, assertions);
validate_import_assertions(tc_scope, &assertions);
if tc_scope.has_caught() {
let e = tc_scope.exception().unwrap();
resolver.reject(tc_scope, e);
Expand Down
181 changes: 125 additions & 56 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const SUPPORTED_TYPE_ASSERTIONS: &[&str] = &["json"];
/// Throws V8 exception if assertions are invalid
pub(crate) fn validate_import_assertions(
scope: &mut v8::HandleScope,
assertions: HashMap<String, String>,
assertions: &HashMap<String, String>,
) {
for (key, value) in assertions {
if !SUPPORTED_TYPE_ASSERTIONS.contains(&key.as_str()) {
Expand All @@ -56,6 +56,10 @@ pub(crate) fn parse_import_assertions(
) -> HashMap<String, String> {
let mut assertions: HashMap<String, String> = HashMap::default();

if no_of_assertions == 0 {
return assertions;
}

let assertions_per_line = import_assertions.length() / no_of_assertions;
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved

for i in 0..no_of_assertions {
Expand All @@ -77,6 +81,21 @@ pub(crate) fn parse_import_assertions(
assertions
}

fn get_module_type_from_assertions(
assertions: &HashMap<String, String>,
) -> ModuleType {
assertions
.get("type")
.map(|ty| {
if ty == "json" {
ModuleType::Json
} else {
ModuleType::JavaScript
}
})
.unwrap_or(ModuleType::JavaScript)
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
}

// Clippy thinks the return value doesn't need to be an Option, it's unaware
// of the mapping that MapFnFrom<F> does for ResolveModuleCallback.
#[allow(clippy::unnecessary_wraps)]
Expand Down Expand Up @@ -472,24 +491,27 @@ impl RecursiveModuleLoad {
let imports = self
.module_map_rc
.borrow()
.get_children(module_id)
.get_requested_modules(module_id)
.unwrap()
.clone();
for specifier in imports {
if !self.visited.contains(&specifier) {
if let Some(module_id) =
self.module_map_rc.borrow().get_id(specifier.as_str())
for module_request in imports {
if !self.visited.contains(&module_request.specifier) {
if let Some(module_id) = self
.module_map_rc
.borrow()
.get_id(module_request.specifier.as_str())
{
already_registered.push_back((module_id, specifier.clone()));
already_registered
.push_back((module_id, module_request.specifier.clone()));
} else {
let fut = self.loader.load(
&specifier,
&module_request.specifier,
Some(referrer.clone()),
self.is_dynamic_import(),
);
self.pending.push(fut.boxed_local());
}
self.visited.insert(specifier);
self.visited.insert(module_request.specifier);
}
}
}
Expand Down Expand Up @@ -567,12 +589,18 @@ impl Stream for RecursiveModuleLoad {
}
}

#[derive(Clone, Debug, PartialEq)]
pub struct ModuleRequest {
pub specifier: ModuleSpecifier,
pub module_type: ModuleType,
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
}

pub struct ModuleInfo {
pub id: ModuleId,
// Used in "bindings.rs" for "import.meta.main" property value.
pub main: bool,
pub name: String,
pub import_specifiers: Vec<ModuleSpecifier>,
pub requests: Vec<ModuleRequest>,
pub module_type: ModuleType,
}

Expand Down Expand Up @@ -698,7 +726,7 @@ impl ModuleMap {
id,
main: false,
name: name.to_string(),
import_specifiers: vec![],
requests: vec![],
module_type: ModuleType::Json,
},
);
Expand Down Expand Up @@ -732,7 +760,7 @@ impl ModuleMap {

let module = maybe_module.unwrap();

let mut import_specifiers: Vec<ModuleSpecifier> = vec![];
let mut requests: Vec<ModuleRequest> = vec![];
let module_requests = module.get_module_requests();
for i in 0..module_requests.length() {
let module_request = v8::Local::<v8::ModuleRequest>::try_from(
Expand All @@ -752,15 +780,20 @@ impl ModuleMap {

// 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);
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);
let module_type = get_module_type_from_assertions(&assertions);
let request = ModuleRequest {
specifier: module_specifier,
module_type,
};
requests.push(request);
}

if main {
Expand Down Expand Up @@ -788,16 +821,19 @@ impl ModuleMap {
id,
main,
name: name.to_string(),
import_specifiers,
requests,
module_type: ModuleType::JavaScript,
},
);

Ok(id)
}

pub fn get_children(&self, id: ModuleId) -> Option<&Vec<ModuleSpecifier>> {
self.info.get(&id).map(|i| &i.import_specifiers)
pub fn get_requested_modules(
&self,
id: ModuleId,
) -> Option<&Vec<ModuleRequest>> {
self.info.get(&id).map(|i| &i.requests)
}

pub fn is_registered(&self, specifier: &ModuleSpecifier) -> bool {
Expand Down Expand Up @@ -908,16 +944,7 @@ impl ModuleMap {
.resolve(specifier, referrer, false)
.expect("Module should have been already resolved");

let module_type = import_assertions
.get("type")
.map(|ty| {
if ty == "json" {
ModuleType::Json
} else {
ModuleType::JavaScript
}
})
.unwrap_or(ModuleType::JavaScript);
let module_type = get_module_type_from_assertions(&import_assertions);

if let Some(id) = self.get_id(resolved_specifier.as_str()) {
let info = self.get_info_by_id(&id).unwrap();
Expand Down Expand Up @@ -1151,21 +1178,33 @@ mod tests {
let c_id = modules.get_id("file:///c.js").unwrap();
let d_id = modules.get_id("file:///d.js").unwrap();
assert_eq!(
modules.get_children(a_id),
modules.get_requested_modules(a_id),
Some(&vec![
crate::resolve_url("file:///b.js").unwrap(),
crate::resolve_url("file:///c.js").unwrap()
ModuleRequest {
specifier: crate::resolve_url("file:///b.js").unwrap(),
module_type: ModuleType::JavaScript,
},
ModuleRequest {
specifier: crate::resolve_url("file:///c.js").unwrap(),
module_type: ModuleType::JavaScript,
},
])
);
assert_eq!(
modules.get_children(b_id),
Some(&vec![crate::resolve_url("file:///c.js").unwrap()])
modules.get_requested_modules(b_id),
Some(&vec![ModuleRequest {
specifier: crate::resolve_url("file:///c.js").unwrap(),
module_type: ModuleType::JavaScript,
},])
);
assert_eq!(
modules.get_children(c_id),
Some(&vec![crate::resolve_url("file:///d.js").unwrap()])
modules.get_requested_modules(c_id),
Some(&vec![ModuleRequest {
specifier: crate::resolve_url("file:///d.js").unwrap(),
module_type: ModuleType::JavaScript,
},])
);
assert_eq!(modules.get_children(d_id), Some(&vec![]));
assert_eq!(modules.get_requested_modules(d_id), Some(&vec![]));
}

const CIRCULAR1_SRC: &str = r#"
Expand Down Expand Up @@ -1272,10 +1311,13 @@ mod tests {
.unwrap();

assert_eq!(dispatch_count.load(Ordering::Relaxed), 0);
let imports = module_map.get_children(mod_a);
let imports = module_map.get_requested_modules(mod_a);
assert_eq!(
imports,
Some(&vec![crate::resolve_url("file:///b.js").unwrap()])
Some(&vec![ModuleRequest {
specifier: crate::resolve_url("file:///b.js").unwrap(),
module_type: ModuleType::JavaScript,
},])
);

let mod_b = module_map
Expand All @@ -1286,7 +1328,7 @@ mod tests {
"export function b() { return 'b' }",
)
.unwrap();
let imports = module_map.get_children(mod_b).unwrap();
let imports = module_map.get_requested_modules(mod_b).unwrap();
assert_eq!(imports.len(), 0);
(mod_a, mod_b)
};
Expand Down Expand Up @@ -1374,10 +1416,13 @@ mod tests {
)
.unwrap();

let imports = module_map.get_children(mod_a);
let imports = module_map.get_requested_modules(mod_a);
assert_eq!(
imports,
Some(&vec![crate::resolve_url("file:///b.json").unwrap()])
Some(&vec![ModuleRequest {
specifier: crate::resolve_url("file:///b.json").unwrap(),
module_type: ModuleType::Json,
},])
);

let mod_b = module_map
Expand All @@ -1387,7 +1432,7 @@ mod tests {
"{\"a\": \"b\", \"c\": {\"d\": 10}}",
)
.unwrap();
let imports = module_map.get_children(mod_b).unwrap();
let imports = module_map.get_requested_modules(mod_b).unwrap();
assert_eq!(imports.len(), 0);
(mod_a, mod_b)
};
Expand Down Expand Up @@ -1704,22 +1749,34 @@ mod tests {
let circular2_id = modules.get_id("file:///circular2.js").unwrap();

assert_eq!(
modules.get_children(circular1_id),
Some(&vec![crate::resolve_url("file:///circular2.js").unwrap()])
modules.get_requested_modules(circular1_id),
Some(&vec![ModuleRequest {
specifier: crate::resolve_url("file:///circular2.js").unwrap(),
module_type: ModuleType::JavaScript,
}])
);

assert_eq!(
modules.get_children(circular2_id),
Some(&vec![crate::resolve_url("file:///circular3.js").unwrap()])
modules.get_requested_modules(circular2_id),
Some(&vec![ModuleRequest {
specifier: crate::resolve_url("file:///circular3.js").unwrap(),
module_type: ModuleType::JavaScript,
}])
);

assert!(modules.get_id("file:///circular3.js").is_some());
let circular3_id = modules.get_id("file:///circular3.js").unwrap();
assert_eq!(
modules.get_children(circular3_id),
modules.get_requested_modules(circular3_id),
Some(&vec![
crate::resolve_url("file:///circular1.js").unwrap(),
crate::resolve_url("file:///circular2.js").unwrap()
ModuleRequest {
specifier: crate::resolve_url("file:///circular1.js").unwrap(),
module_type: ModuleType::JavaScript,
},
ModuleRequest {
specifier: crate::resolve_url("file:///circular2.js").unwrap(),
module_type: ModuleType::JavaScript,
}
])
);
}
Expand Down Expand Up @@ -1925,21 +1982,33 @@ mod tests {
let d_id = modules.get_id("file:///d.js").unwrap();

assert_eq!(
modules.get_children(main_id),
modules.get_requested_modules(main_id),
Some(&vec![
crate::resolve_url("file:///b.js").unwrap(),
crate::resolve_url("file:///c.js").unwrap()
ModuleRequest {
specifier: crate::resolve_url("file:///b.js").unwrap(),
module_type: ModuleType::JavaScript,
},
ModuleRequest {
specifier: crate::resolve_url("file:///c.js").unwrap(),
module_type: ModuleType::JavaScript,
}
])
);
assert_eq!(
modules.get_children(b_id),
Some(&vec![crate::resolve_url("file:///c.js").unwrap()])
modules.get_requested_modules(b_id),
Some(&vec![ModuleRequest {
specifier: crate::resolve_url("file:///c.js").unwrap(),
module_type: ModuleType::JavaScript,
}])
);
assert_eq!(
modules.get_children(c_id),
Some(&vec![crate::resolve_url("file:///d.js").unwrap()])
modules.get_requested_modules(c_id),
Some(&vec![ModuleRequest {
specifier: crate::resolve_url("file:///d.js").unwrap(),
module_type: ModuleType::JavaScript,
}])
);
assert_eq!(modules.get_children(d_id), Some(&vec![]));
assert_eq!(modules.get_requested_modules(d_id), Some(&vec![]));
}

#[test]
Expand Down