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 all 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: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cli/config_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ pub const IGNORED_COMPILER_OPTIONS: &[&str] = &[
"paths",
"preserveConstEnums",
"reactNamespace",
"resolveJsonModule",
"rootDir",
"rootDirs",
"skipLibCheck",
Expand Down
24 changes: 15 additions & 9 deletions cli/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ pub(crate) fn get_ts_config(
"isolatedModules": true,
"lib": lib,
"module": "esnext",
"resolveJsonModule": true,
"strict": true,
"target": "esnext",
"tsBuildInfoFile": "deno:///.tsbuildinfo",
Expand Down Expand Up @@ -186,6 +187,7 @@ pub(crate) fn get_ts_config(
"jsx": "react",
"jsxFactory": "React.createElement",
"jsxFragmentFactory": "React.Fragment",
"resolveJsonModule": true,
})),
ConfigType::RuntimeEmit { tsc_emit } => {
let mut ts_config = TsConfig::new(json!({
Expand Down Expand Up @@ -403,14 +405,15 @@ pub(crate) fn check_and_maybe_emit(
log::debug!("module missing, skipping emit for {}", specifier);
continue;
};
// Sometimes if `tsc` sees a CommonJS file it will _helpfully_ output it
// to ESM, which we don't really want to do unless someone has enabled
// check_js.
if !check_js
&& matches!(
media_type,
MediaType::JavaScript | MediaType::Cjs | MediaType::Mjs
)
// Sometimes if `tsc` sees a CommonJS file or a JSON module, it will
// _helpfully_ output it, which we don't really want to do unless
// someone has enabled check_js.
if matches!(media_type, MediaType::Json)
|| (!check_js
&& matches!(
media_type,
MediaType::JavaScript | MediaType::Cjs | MediaType::Mjs
))
{
log::debug!("skipping emit for {}", specifier);
continue;
Expand All @@ -429,7 +432,10 @@ pub(crate) fn check_and_maybe_emit(
MediaType::Dts | MediaType::Dcts | MediaType::Dmts => {
cache.set(CacheType::Declaration, &specifier, emit.data)?;
}
_ => unreachable!(),
_ => unreachable!(
"unexpected media_type {} {}",
emit.media_type, specifier
),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,7 @@ impl Inner {
"lib": ["deno.ns", "deno.window"],
"module": "esnext",
"noEmit": true,
"resolveJsonModule": true,
"strict": true,
"target": "esnext",
"useDefineForClassFields": true,
Expand Down
17 changes: 15 additions & 2 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 @@ -68,6 +69,7 @@ pub struct ProcState(Arc<Inner>);
enum ModuleEntry {
Module {
code: String,
media_type: MediaType,
dependencies: BTreeMap<String, Dependency>,
},
Error(ModuleGraphError),
Expand Down Expand Up @@ -584,6 +586,7 @@ impl ProcState {
| MediaType::Unknown
| MediaType::Cjs
| MediaType::Mjs
| MediaType::Json
) {
module.maybe_source().unwrap_or("").to_string()
// The emit may also be missing when a declaration file is in the
Expand All @@ -602,7 +605,11 @@ impl ProcState {
module.maybe_dependencies().cloned().unwrap_or_default();
graph_data.modules.insert(
specifier.clone(),
ModuleEntry::Module { code, dependencies },
ModuleEntry::Module {
code,
dependencies,
media_type: *media_type,
},
);
if let Some(dependencies) = module.maybe_dependencies() {
for dep in dependencies.values() {
Expand Down Expand Up @@ -724,10 +731,16 @@ impl ProcState {
_ => &specifier,
};
match graph_data.modules.get(found_specifier) {
Some(ModuleEntry::Module { code, .. }) => Ok(ModuleSource {
Some(ModuleEntry::Module {
code, media_type, ..
}) => Ok(ModuleSource {
code: code.clone(),
module_url_specified: specifier.to_string(),
module_url_found: found_specifier.to_string(),
module_type: match media_type {
MediaType::Json => ModuleType::Json,
_ => 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
33 changes: 33 additions & 0 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2407,3 +2407,36 @@ fn issue12807() {
.unwrap();
assert!(status.success());
}

itest!(import_assertions_static_import {
args: "run --allow-read import_assertions/static_import.ts",
output: "import_assertions/static_import.out",
});

itest!(import_assertions_static_export {
args: "run --allow-read import_assertions/static_export.ts",
output: "import_assertions/static_export.out",
});

itest!(import_assertions_static_error {
args: "run --allow-read import_assertions/static_error.ts",
output: "import_assertions/static_error.out",
exit_code: 1,
});

itest!(import_assertions_dynamic_import {
args: "run --allow-read import_assertions/dynamic_import.ts",
output: "import_assertions/dynamic_import.out",
});

itest!(import_assertions_dynamic_error {
args: "run --allow-read import_assertions/dynamic_error.ts",
output: "import_assertions/dynamic_error.out",
exit_code: 1,
});

itest!(import_assertions_type_check {
args: "run --allow-read import_assertions/type_check.ts",
output: "import_assertions/type_check.out",
exit_code: 1,
});
6 changes: 6 additions & 0 deletions cli/tests/testdata/import_assertions/data.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"a": "b",
"c": {
"d": 10
}
}
5 changes: 5 additions & 0 deletions cli/tests/testdata/import_assertions/dynamic_error.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[WILDCARD]
error: Uncaught (in promise) TypeError: Expected a "JavaScript" module but loaded a "JSON" module.
const data = await import("./data.json");
^
at async [WILDCARD]dynamic_error.ts:1:14
3 changes: 3 additions & 0 deletions cli/tests/testdata/import_assertions/dynamic_error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const data = await import("./data.json");

console.log(data);
2 changes: 2 additions & 0 deletions cli/tests/testdata/import_assertions/dynamic_import.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[WILDCARD]
Module { default: { a: "b", c: { d: 10 } } }
3 changes: 3 additions & 0 deletions cli/tests/testdata/import_assertions/dynamic_import.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const data = await import("./data.json", { assert: { type: "json" } });

console.log(data);
5 changes: 5 additions & 0 deletions cli/tests/testdata/import_assertions/static_error.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[WILDCARD]
error: An unsupported media type was attempted to be imported as a module.
Specifier: [WILDCARD]data.json
MediaType: Json
at [WILDCARD]static_error.ts:1:18
3 changes: 3 additions & 0 deletions cli/tests/testdata/import_assertions/static_error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import data from "./data.json";

console.log(data);
2 changes: 2 additions & 0 deletions cli/tests/testdata/import_assertions/static_export.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[WILDCARD]
{ a: "b", c: { d: 10 } }
3 changes: 3 additions & 0 deletions cli/tests/testdata/import_assertions/static_export.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import data from "./static_reexport.ts";

console.log(data);
2 changes: 2 additions & 0 deletions cli/tests/testdata/import_assertions/static_import.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[WILDCARD]
{ a: "b", c: { d: 10 } }
3 changes: 3 additions & 0 deletions cli/tests/testdata/import_assertions/static_import.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import data from "./data.json" assert { type: "json" };

console.log(data);
1 change: 1 addition & 0 deletions cli/tests/testdata/import_assertions/static_reexport.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from "./data.json" assert { type: "json" };
5 changes: 5 additions & 0 deletions cli/tests/testdata/import_assertions/type_check.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[WILDCARD]
error: TS2339 [ERROR]: Property 'foo' does not exist on type '{ a: string; c: { d: number; }; }'.
console.log(data.foo);
~~~
at [WILDCARD]type_check.ts:3:18
3 changes: 3 additions & 0 deletions cli/tests/testdata/import_assertions/type_check.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import data from "./data.json" assert { type: "json" };

console.log(data.foo);
20 changes: 19 additions & 1 deletion cli/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,24 @@ struct LoadArgs {
specifier: String,
}

fn as_ts_script_kind(media_type: &MediaType) -> i32 {
match media_type {
MediaType::JavaScript => 1,
MediaType::Jsx => 2,
MediaType::Mjs => 1,
MediaType::Cjs => 1,
MediaType::TypeScript => 3,
MediaType::Mts => 3,
MediaType::Cts => 3,
MediaType::Dts => 3,
MediaType::Dmts => 3,
MediaType::Dcts => 3,
MediaType::Tsx => 4,
MediaType::Json => 6,
_ => 0,
}
}

fn op_load(state: &mut State, args: Value) -> Result<Value, AnyError> {
let v: LoadArgs = serde_json::from_value(args)
.context("Invalid request from JavaScript for \"op_load\".")?;
Expand Down Expand Up @@ -395,7 +413,7 @@ fn op_load(state: &mut State, args: Value) -> Result<Value, AnyError> {
};

Ok(
json!({ "data": data, "hash": hash, "scriptKind": media_type.as_ts_script_kind() }),
json!({ "data": data, "hash": hash, "scriptKind": as_ts_script_kind(&media_type) }),
)
}

Expand Down
58 changes: 52 additions & 6 deletions core/bindings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.

use crate::error::is_instance_of_error;
use crate::modules::get_module_type_from_assertions;
use crate::modules::parse_import_assertions;
use crate::modules::validate_import_assertions;
use crate::modules::ImportAssertionsKind;
use crate::modules::ModuleMap;
use crate::resolve_url_or_path;
use crate::JsRuntime;
Expand Down Expand Up @@ -243,7 +247,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 +271,22 @@ pub extern "C" fn host_import_module_dynamically_callback(
let resolver = v8::PromiseResolver::new(scope).unwrap();
let promise = resolver.get_promise(scope);

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 module_type = get_module_type_from_assertions(&assertions);

let resolver_handle = v8::Global::new(scope, resolver);
{
let state_rc = JsRuntime::state(scope);
Expand All @@ -280,6 +300,7 @@ pub extern "C" fn host_import_module_dynamically_callback(
module_map_rc,
&specifier_str,
&referrer_name_str,
module_type,
resolver_handle,
);
state_rc.borrow_mut().notify_new_dynamic_import();
Expand All @@ -294,13 +315,29 @@ pub extern "C" fn host_import_module_dynamically_callback(
_rv: v8::ReturnValue| {
let arg = args.get(0);
if is_instance_of_error(scope, arg) {
let e: crate::error::NativeJsError =
serde_v8::from_v8(scope, arg).unwrap();
let name = e.name.unwrap_or_else(|| "Error".to_string());
let message = v8::Exception::create_message(scope, arg);
if message.get_stack_trace(scope).unwrap().get_frame_count() == 0 {
let arg: v8::Local<v8::Object> = arg.try_into().unwrap();
let message_key = v8::String::new(scope, "message").unwrap();
let message = arg.get(scope, message_key.into()).unwrap();
let exception =
v8::Exception::type_error(scope, message.try_into().unwrap());
let exception = match name.as_str() {
"RangeError" => {
v8::Exception::range_error(scope, message.try_into().unwrap())
}
"TypeError" => {
v8::Exception::type_error(scope, message.try_into().unwrap())
}
"SyntaxError" => {
v8::Exception::syntax_error(scope, message.try_into().unwrap())
}
"ReferenceError" => {
v8::Exception::reference_error(scope, message.try_into().unwrap())
}
_ => v8::Exception::error(scope, message.try_into().unwrap()),
};
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Seems like something that could be extracted out into a function to help reduce the detail here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only used in a single place, and the whole function seems like a code smell, I'd rather address this in separate PR that would cleanup modules implementation

let code_key = v8::String::new(scope, "code").unwrap();
let code_value =
v8::String::new(scope, "ERR_MODULE_NOT_FOUND").unwrap();
Expand Down Expand Up @@ -1311,7 +1348,7 @@ fn create_host_object(
pub fn module_resolve_callback<'s>(
context: v8::Local<'s, v8::Context>,
specifier: v8::Local<'s, v8::String>,
_import_assertions: v8::Local<'s, v8::FixedArray>,
import_assertions: v8::Local<'s, v8::FixedArray>,
referrer: v8::Local<'s, v8::Module>,
) -> Option<v8::Local<'s, v8::Module>> {
let scope = &mut unsafe { v8::CallbackScope::new(context) };
Expand All @@ -1328,8 +1365,17 @@ pub fn module_resolve_callback<'s>(

let specifier_str = specifier.to_rust_string_lossy(scope);

let maybe_module =
module_map.resolve_callback(scope, &specifier_str, &referrer_name);
let assertions = parse_import_assertions(
scope,
import_assertions,
ImportAssertionsKind::StaticImport,
);
let maybe_module = module_map.resolve_callback(
scope,
&specifier_str,
&referrer_name,
assertions,
);
if let Some(module) = maybe_module {
return Some(module);
}
Expand Down
6 changes: 3 additions & 3 deletions core/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ fn get_property<'a>(
}

#[derive(serde::Deserialize)]
struct NativeJsError {
name: Option<String>,
message: Option<String>,
pub(crate) struct NativeJsError {
pub name: Option<String>,
pub message: Option<String>,
// Warning! .stack is special so handled by itself
// stack: Option<String>,
}
Expand Down
Loading