Skip to content

Commit

Permalink
fix: source maps in inspector (denoland#5223)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bartlomieju committed May 11, 2020
1 parent 73d8fa7 commit d062ffc
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 25 deletions.
4 changes: 0 additions & 4 deletions cli/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,3 @@ pub fn gen(v: Vec<&[u8]>) -> String {
}
out
}

pub fn gen2(s: &str) -> String {
gen(vec![s.as_bytes()])
}
8 changes: 4 additions & 4 deletions cli/global_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}),
}?;
Expand All @@ -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,
};
Expand Down
20 changes: 11 additions & 9 deletions cli/lockfile.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -43,28 +43,30 @@ 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<bool> {
if m.name.starts_with("file:") {
pub fn check(&mut self, url: &Url, code: Vec<u8>) -> Result<bool> {
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
})
}

// 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<u8>) -> 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()
}
}
4 changes: 2 additions & 2 deletions cli/tests/lock_check_ok.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"http:https://127.0.0.1:4545/cli/tests/subdir/print_hello.ts": "5c93c66125878389f47f4abcac003f4be1276c5223612c26302460d71841e287",
"http:https://127.0.0.1:4545/cli/tests/003_relative_import.ts": "da3b7f60f5ff635dbc27f3e5e05420f0f2c34676f080ef935ea547116424adeb"
"http:https://127.0.0.1:4545/cli/tests/subdir/print_hello.ts": "fe7bbccaedb6579200a8b582f905139296402d06b1b91109d6e12c41a23125da",
"http:https://127.0.0.1:4545/cli/tests/003_relative_import.ts": "aa9e16de824f81871a1c7164d5bd6857df7db2e18621750bd66b0bde4df07f21"
}
38 changes: 34 additions & 4 deletions cli/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<String>>();

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");
Expand Down Expand Up @@ -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(
Expand All @@ -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<String> =
source_code.split('\n').map(|s| s.to_string()).collect();
let last_line = lines.pop().unwrap();
assert!(last_line.starts_with("//# sourceMappingURL=file:https://"));
}

#[tokio::test]
Expand Down
4 changes: 2 additions & 2 deletions core/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit d062ffc

Please sign in to comment.