From d062ffc1baeccca8bf168dc1ce4e94b929478142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 11 May 2020 23:48:36 +0200 Subject: [PATCH] fix: source maps in inspector (#5223) This commit fixes problems with source maps in Chrome Devtools by substituting source map URL generated by TS compiler with actual file URL pointing to DENO_DIR. Dummy value of "source_map_url" has been removed from "ScriptOrigin". Also fixes lock file which used compiled source code to generate lock hash; it now uses source code of the file that is being compiled. --- cli/checksum.rs | 4 ---- cli/global_state.rs | 8 ++++---- cli/lockfile.rs | 20 ++++++++++--------- cli/tests/lock_check_ok.json | 4 ++-- cli/tsc.rs | 38 ++++++++++++++++++++++++++++++++---- core/bindings.rs | 4 ++-- 6 files changed, 53 insertions(+), 25 deletions(-) diff --git a/cli/checksum.rs b/cli/checksum.rs index 232a19a49a1550..8696f93f7cb72a 100644 --- a/cli/checksum.rs +++ b/cli/checksum.rs @@ -13,7 +13,3 @@ pub fn gen(v: Vec<&[u8]>) -> String { } out } - -pub fn gen2(s: &str) -> String { - gen(vec![s.as_bytes()]) -} diff --git a/cli/global_state.rs b/cli/global_state.rs index 4e9bdbb99c9411..50c306a443808e 100644 --- a/cli/global_state.rs +++ b/cli/global_state.rs @@ -139,13 +139,13 @@ impl GlobalState { }; Ok(CompiledModule { - code: String::from_utf8(out.source_code)?, + code: String::from_utf8(out.source_code.clone())?, name: out.url.to_string(), }) } } _ => Ok(CompiledModule { - code: String::from_utf8(out.source_code)?, + code: String::from_utf8(out.source_code.clone())?, name: out.url.to_string(), }), }?; @@ -154,9 +154,9 @@ impl GlobalState { if let Some(ref lockfile) = state2.lockfile { let mut g = lockfile.lock().unwrap(); if state2.flags.lock_write { - g.insert(&compiled_module); + g.insert(&out.url, out.source_code); } else { - let check = match g.check(&compiled_module) { + let check = match g.check(&out.url, out.source_code) { Err(e) => return Err(ErrBox::from(e)), Ok(v) => v, }; diff --git a/cli/lockfile.rs b/cli/lockfile.rs index 5e43e3420f206b..854d3ea9d51312 100644 --- a/cli/lockfile.rs +++ b/cli/lockfile.rs @@ -1,8 +1,8 @@ -use crate::tsc::CompiledModule; use serde_json::json; pub use serde_json::Value; use std::collections::HashMap; use std::io::Result; +use url::Url; pub struct Lockfile { need_read: bool, @@ -43,16 +43,17 @@ impl Lockfile { /// Lazily reads the filename, checks the given module is included. /// Returns Ok(true) if check passed - pub fn check(&mut self, m: &CompiledModule) -> Result { - if m.name.starts_with("file:") { + pub fn check(&mut self, url: &Url, code: Vec) -> Result { + let url_str = url.to_string(); + if url_str.starts_with("file:") { return Ok(true); } if self.need_read { self.read()?; } assert!(!self.need_read); - Ok(if let Some(lockfile_checksum) = self.map.get(&m.name) { - let compiled_checksum = crate::checksum::gen2(&m.code); + Ok(if let Some(lockfile_checksum) = self.map.get(&url_str) { + let compiled_checksum = crate::checksum::gen(vec![&code]); lockfile_checksum == &compiled_checksum } else { false @@ -60,11 +61,12 @@ impl Lockfile { } // Returns true if module was not already inserted. - pub fn insert(&mut self, m: &CompiledModule) -> bool { - if m.name.starts_with("file:") { + pub fn insert(&mut self, url: &Url, code: Vec) -> bool { + let url_str = url.to_string(); + if url_str.starts_with("file:") { return false; } - let checksum = crate::checksum::gen2(&m.code); - self.map.insert(m.name.clone(), checksum).is_none() + let checksum = crate::checksum::gen(vec![&code]); + self.map.insert(url_str, checksum).is_none() } } diff --git a/cli/tests/lock_check_ok.json b/cli/tests/lock_check_ok.json index 85670a5d64721a..c4b68e158f18d8 100644 --- a/cli/tests/lock_check_ok.json +++ b/cli/tests/lock_check_ok.json @@ -1,4 +1,4 @@ { - "http://127.0.0.1:4545/cli/tests/subdir/print_hello.ts": "5c93c66125878389f47f4abcac003f4be1276c5223612c26302460d71841e287", - "http://127.0.0.1:4545/cli/tests/003_relative_import.ts": "da3b7f60f5ff635dbc27f3e5e05420f0f2c34676f080ef935ea547116424adeb" + "http://127.0.0.1:4545/cli/tests/subdir/print_hello.ts": "fe7bbccaedb6579200a8b582f905139296402d06b1b91109d6e12c41a23125da", + "http://127.0.0.1:4545/cli/tests/003_relative_import.ts": "aa9e16de824f81871a1c7164d5bd6857df7db2e18621750bd66b0bde4df07f21" } diff --git a/cli/tsc.rs b/cli/tsc.rs index f207558a5c6f1b..db9c19afefe163 100644 --- a/cli/tsc.rs +++ b/cli/tsc.rs @@ -619,6 +619,33 @@ impl TsCompiler { return Ok(()); } + // By default TSC output source map url that is relative; we need + // to substitute it manually to correct file URL in DENO_DIR. + let mut content_lines = contents + .split('\n') + .map(|s| s.to_string()) + .collect::>(); + + if !content_lines.is_empty() { + let last_line = content_lines.pop().unwrap(); + if last_line.starts_with("//# sourceMappingURL=") { + let source_map_key = self.disk_cache.get_cache_filename_with_extension( + module_specifier.as_url(), + "js.map", + ); + let source_map_path = self.disk_cache.location.join(source_map_key); + let source_map_file_url = Url::from_file_path(source_map_path) + .expect("Bad file URL for source map"); + let new_last_line = + format!("//# sourceMappingURL={}", source_map_file_url.to_string()); + content_lines.push(new_last_line); + } else { + content_lines.push(last_line); + } + } + + let contents = content_lines.join("\n"); + let js_key = self .disk_cache .get_cache_filename_with_extension(module_specifier.as_url(), "js"); @@ -878,7 +905,7 @@ mod tests { types_url: None, }; let mock_state = - GlobalState::mock(vec![String::from("deno"), String::from("hello.js")]); + GlobalState::mock(vec![String::from("deno"), String::from("hello.ts")]); let result = mock_state .ts_compiler .compile( @@ -889,11 +916,14 @@ mod tests { ) .await; assert!(result.is_ok()); - assert!(result - .unwrap() - .code + let source_code = result.unwrap().code; + assert!(source_code .as_bytes() .starts_with(b"\"use strict\";\nconsole.log(\"Hello World\");")); + let mut lines: Vec = + source_code.split('\n').map(|s| s.to_string()).collect(); + let last_line = lines.pop().unwrap(); + assert!(last_line.starts_with("//# sourceMappingURL=file://")); } #[tokio::test] diff --git a/core/bindings.rs b/core/bindings.rs index a081cccda4f107..16e78e368f0e10 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -60,7 +60,7 @@ pub fn script_origin<'a>( let resource_column_offset = v8::Integer::new(s, 0); let resource_is_shared_cross_origin = v8::Boolean::new(s, false); let script_id = v8::Integer::new(s, 123); - let source_map_url = v8::String::new(s, "source_map_url").unwrap(); + let source_map_url = v8::String::new(s, "").unwrap(); let resource_is_opaque = v8::Boolean::new(s, true); let is_wasm = v8::Boolean::new(s, false); let is_module = v8::Boolean::new(s, false); @@ -85,7 +85,7 @@ pub fn module_origin<'a>( let resource_column_offset = v8::Integer::new(s, 0); let resource_is_shared_cross_origin = v8::Boolean::new(s, false); let script_id = v8::Integer::new(s, 123); - let source_map_url = v8::String::new(s, "source_map_url").unwrap(); + let source_map_url = v8::String::new(s, "").unwrap(); let resource_is_opaque = v8::Boolean::new(s, true); let is_wasm = v8::Boolean::new(s, false); let is_module = v8::Boolean::new(s, true);