Skip to content

Commit

Permalink
feat: Add support for import assertions and JSON modules (#12866)
Browse files Browse the repository at this point in the history
This commit adds proper support for import assertions and JSON modules.

Implementation of "core/modules.rs" was changed to account for multiple possible
module types, instead of always assuming that the code is an "ES module". In
effect "ModuleMap" now has knowledge about each modules' type (stored via
"ModuleType" enum). Module loading pipeline now stores information about
expected module type for each request and validates that expected type matches
discovered module type based on file's "MediaType".

Relevant tests were added to "core/modules.rs" and integration tests,
additionally multiple WPT tests were enabled.

There are still some rough edges in the implementation and not all WPT were
enabled, due to:
a) unclear BOM handling in source code by "FileFetcher"
b) design limitation of Deno's "FileFetcher" that doesn't download the same
module multiple times in a single run

Co-authored-by: Kitson Kelly <[email protected]>
  • Loading branch information
bartlomieju and kitsonk authored Dec 15, 2021
1 parent ec7d906 commit a1f0796
Show file tree
Hide file tree
Showing 28 changed files with 916 additions and 228 deletions.
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()),
};
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

0 comments on commit a1f0796

Please sign in to comment.