From 3d19fb493b82a8ab9b3fd0fa923fc81fd68acb4e Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Mon, 26 Oct 2020 07:17:58 +1100 Subject: [PATCH] fix(cli): properly handle roots with extensions that don't match media type (#8114) --- cli/module_graph2.rs | 7 ++- cli/tests/cache_extensionless.out | 2 + cli/tests/cache_random_extension.out | 2 + cli/tests/integration_tests.rs | 12 +++++ cli/tests/subdir/no_js_ext | 3 ++ cli/tsc2.rs | 71 +++++++++++++++++++++------- test_util/src/lib.rs | 15 +++++- 7 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 cli/tests/cache_extensionless.out create mode 100644 cli/tests/cache_random_extension.out create mode 100644 cli/tests/subdir/no_js_ext diff --git a/cli/module_graph2.rs b/cli/module_graph2.rs index 05bd407835..8ee60eb598 100644 --- a/cli/module_graph2.rs +++ b/cli/module_graph2.rs @@ -758,8 +758,11 @@ impl Graph2 { info!("{} {}", colors::green("Check"), specifier); } - let root_names: Vec = - self.roots.iter().map(|ms| ms.to_string()).collect(); + let root_names: Vec<(ModuleSpecifier, MediaType)> = self + .roots + .iter() + .map(|ms| (ms.clone(), self.get_media_type(ms).unwrap())) + .collect(); let maybe_tsbuildinfo = self.maybe_tsbuildinfo.clone(); let hash_data = vec![config.as_bytes(), version::DENO.as_bytes().to_owned()]; diff --git a/cli/tests/cache_extensionless.out b/cli/tests/cache_extensionless.out new file mode 100644 index 0000000000..6e35696890 --- /dev/null +++ b/cli/tests/cache_extensionless.out @@ -0,0 +1,2 @@ +[WILDCARD] +Check http://localhost:4545/cli/tests/subdir/no_js_ext diff --git a/cli/tests/cache_random_extension.out b/cli/tests/cache_random_extension.out new file mode 100644 index 0000000000..c508fbc602 --- /dev/null +++ b/cli/tests/cache_random_extension.out @@ -0,0 +1,2 @@ +[WILDCARD] +Check http://localhost:4545/cli/tests/subdir/no_js_ext@1.0.0 diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index d88309cb22..74fac3ab19 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -2722,6 +2722,18 @@ itest!(_053_import_compression { http_server: true, }); +itest!(cache_extensionless { + args: "cache --reload http://localhost:4545/cli/tests/subdir/no_js_ext", + output: "cache_extensionless.out", + http_server: true, +}); + +itest!(cache_random_extension { + args: "cache --reload http://localhost:4545/cli/tests/subdir/no_js_ext@1.0.0", + output: "cache_random_extension.out", + http_server: true, +}); + itest!(cafile_url_imports { args: "run --quiet --reload --cert tls/RootCA.pem cafile_url_imports.ts", output: "cafile_url_imports.ts.out", diff --git a/cli/tests/subdir/no_js_ext b/cli/tests/subdir/no_js_ext new file mode 100644 index 0000000000..8322a106fa --- /dev/null +++ b/cli/tests/subdir/no_js_ext @@ -0,0 +1,3 @@ +// @ts-check +import { printHello } from "./mod2.ts"; +printHello(); diff --git a/cli/tsc2.rs b/cli/tsc2.rs index 54e99a651c..44754c030a 100644 --- a/cli/tsc2.rs +++ b/cli/tsc2.rs @@ -20,8 +20,8 @@ use deno_core::OpFn; use deno_core::RuntimeOptions; use deno_core::Snapshot; use serde::Deserialize; -use serde::Serialize; use std::cell::RefCell; +use std::collections::HashMap; use std::rc::Rc; #[derive(Debug, Clone, Default, Eq, PartialEq)] @@ -32,23 +32,19 @@ pub struct EmittedFile { } /// A structure representing a request to be sent to the tsc runtime. -#[derive(Debug, Serialize)] -#[serde(rename_all = "camelCase")] +#[derive(Debug)] pub struct Request { /// The TypeScript compiler options which will be serialized and sent to /// tsc. pub config: TsConfig, /// Indicates to the tsc runtime if debug logging should occur. pub debug: bool, - #[serde(skip_serializing)] pub graph: Rc>, - #[serde(skip_serializing)] pub hash_data: Vec>, - #[serde(skip_serializing)] pub maybe_tsbuildinfo: Option, /// A vector of strings that represent the root/entry point modules for the /// program. - pub root_names: Vec, + pub root_names: Vec<(ModuleSpecifier, MediaType)>, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -69,6 +65,7 @@ struct State { graph: Rc>, maybe_tsbuildinfo: Option, maybe_response: Option, + root_map: HashMap, } impl State { @@ -76,6 +73,7 @@ impl State { graph: Rc>, hash_data: Vec>, maybe_tsbuildinfo: Option, + root_map: HashMap, ) -> Self { State { hash_data, @@ -83,6 +81,7 @@ impl State { graph, maybe_tsbuildinfo, maybe_response: None, + root_map, } } } @@ -138,7 +137,13 @@ fn emit(state: &mut State, args: Value) -> Result { maybe_specifiers: if let Some(specifiers) = &v.maybe_specifiers { let specifiers = specifiers .iter() - .map(|s| ModuleSpecifier::resolve_url_or_path(s).unwrap()) + .map(|s| { + if let Some(remapped_specifier) = state.root_map.get(s) { + remapped_specifier.clone() + } else { + ModuleSpecifier::resolve_url_or_path(s).unwrap() + } + }) .collect(); Some(specifiers) } else { @@ -174,6 +179,12 @@ fn load(state: &mut State, args: Value) -> Result { Some("declare var a: any;\nexport = a;\n".to_string()) } else { let graph = state.graph.borrow(); + let specifier = + if let Some(remapped_specifier) = state.root_map.get(&v.specifier) { + remapped_specifier.clone() + } else { + specifier + }; let maybe_source = graph.get_source(&specifier); media_type = if let Some(media_type) = graph.get_media_type(&specifier) { media_type @@ -207,9 +218,13 @@ fn resolve(state: &mut State, args: Value) -> Result { let v: ResolveArgs = serde_json::from_value(args) .context("Invalid request from JavaScript for \"op_resolve\".")?; let mut resolved: Vec<(String, String)> = Vec::new(); - let referrer = ModuleSpecifier::resolve_url_or_path(&v.base).context( - "Error converting a string module specifier for \"op_resolve\".", - )?; + let referrer = if let Some(remapped_base) = state.root_map.get(&v.base) { + remapped_base.clone() + } else { + ModuleSpecifier::resolve_url_or_path(&v.base).context( + "Error converting a string module specifier for \"op_resolve\".", + )? + }; for specifier in &v.specifiers { if specifier.starts_with("asset:///") { resolved.push(( @@ -272,6 +287,25 @@ pub fn exec( startup_snapshot: Some(snapshot), ..Default::default() }); + // tsc cannot handle root specifiers that don't have one of the "acceptable" + // extensions. Therefore, we have to check the root modules against their + // extensions and remap any that are unacceptable to tsc and add them to the + // op state so when requested, we can remap to the original specifier. + let mut root_map = HashMap::new(); + let root_names: Vec = request + .root_names + .iter() + .map(|(s, mt)| { + let ext_media_type = MediaType::from(&s.as_str().to_owned()); + if mt != &ext_media_type { + let new_specifier = format!("{}{}", s, mt.as_ts_extension()); + root_map.insert(new_specifier.clone(), s.clone()); + new_specifier + } else { + s.as_str().to_owned() + } + }) + .collect(); { let op_state = runtime.op_state(); @@ -280,6 +314,7 @@ pub fn exec( request.graph.clone(), request.hash_data.clone(), request.maybe_tsbuildinfo.clone(), + root_map, )); } @@ -290,8 +325,12 @@ pub fn exec( runtime.register_op("op_respond", op(respond)); let startup_source = "globalThis.startup({ legacyFlag: false })"; - let request_str = - serde_json::to_string(&request).context("Could not serialize request.")?; + let request_value = json!({ + "config": request.config, + "debug": request.debug, + "rootNames": root_names, + }); + let request_str = request_value.to_string(); let exec_source = format!("globalThis.exec({})", request_str); runtime @@ -354,7 +393,7 @@ mod tests { .await .expect("module not inserted"); let graph = Rc::new(RefCell::new(builder.get_graph())); - State::new(graph, hash_data, maybe_tsbuildinfo) + State::new(graph, hash_data, maybe_tsbuildinfo, HashMap::new()) } #[tokio::test] @@ -601,7 +640,7 @@ mod tests { graph, hash_data, maybe_tsbuildinfo: None, - root_names: vec!["https://deno.land/x/a.ts".to_string()], + root_names: vec![(specifier, MediaType::TypeScript)], }; let actual = exec(js::compiler_isolate_init(), request) .expect("exec should have not errored"); @@ -651,7 +690,7 @@ mod tests { graph, hash_data, maybe_tsbuildinfo: None, - root_names: vec!["file:///reexports.ts".to_string()], + root_names: vec![(specifier, MediaType::TypeScript)], }; let actual = exec(js::compiler_isolate_init(), request) .expect("exec should have not errored"); diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index 832c40f59c..eab8bcac6a 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -420,6 +420,19 @@ pub async fn run_all_servers() { HeaderValue::from_static("application/typescript"), ); res + })) + .or(warp::path!("cli"/"tests"/"subdir"/"no_js_ext@1.0.0").map(|| { + let mut res = Response::new(Body::from( + r#"import { printHello } from "./mod2.ts"; + printHello(); + "#, + )); + let h = res.headers_mut(); + h.insert( + "Content-type", + HeaderValue::from_static("application/javascript"), + ); + res })); let content_type_handler = warp::any() @@ -522,7 +535,7 @@ fn custom_headers(path: warp::path::Peek, f: warp::fs::File) -> Box { Some("application/x-www-form-urlencoded") } else if p.contains("unknown_ext") || p.contains("no_ext") { Some("text/typescript") - } else if p.contains("mismatch_ext") { + } else if p.contains("mismatch_ext") || p.contains("no_js_ext") { Some("text/javascript") } else if p.ends_with(".ts") || p.ends_with(".tsx") { Some("application/typescript")