foster/deno
0
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2025-03-03 09:31:22 -05:00
Code Issues Packages 1 Wiki Activity

Revert "use byte array instead of string for code fetch (#1307)" (#1455)

Browse source 
This reverts commit e976b3e041.

There is nothing technically wrong with this commit, but it's adding
complexity to a big refactor (native ES modules #975). Since it's not
necessary and simply a philosophical preference, I will revert for now
and try to bring it back later.
This commit is contained in:
Ryan Dahl 2019-01-03 22:11:01 -05:00 • committed by GitHub
parent 6be1164d89
commit ea6c9f2f36
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 88 additions and 97 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
js/compiler.ts
View file

@ -233,7 +233,6 @@ export class Compiler
// We query Rust with a CodeFetch message. It will load the sourceCode,
// and if there is any outputCode cached, will return that as well.
const fetchResponse = this._os.codeFetch(moduleSpecifier, containingFile);
assert(fetchResponse != null, "fetchResponse is null");
moduleId = fetchResponse.moduleName;
fileName = fetchResponse.filename;
mediaType = fetchResponse.mediaType;

11
js/os.ts
View file

@ -4,7 +4,6 @@ import { assert } from "./util";
import * as util from "./util";
import * as flatbuffers from "./flatbuffers";
import { sendSync } from "./dispatch";
import { TextDecoder } from "./text_encoding";
interface CodeInfo {
moduleName: string | undefined;
@ -46,17 +45,13 @@ export function codeFetch(specifier: string, referrer: string): CodeInfo {
assert(baseRes!.inner(codeFetchRes) != null);
// flatbuffers returns `null` for an empty value, this does not fit well with
// idiomatic TypeScript under strict null checks, so converting to `undefined`
const sourceCode = codeFetchRes.sourceCodeArray() || undefined;
const outputCode = codeFetchRes.outputCodeArray() || undefined;
const sourceMap = codeFetchRes.sourceMapArray() || undefined;
const decoder = new TextDecoder();
return {
moduleName: codeFetchRes.moduleName() || undefined,
filename: codeFetchRes.filename() || undefined,
mediaType: codeFetchRes.mediaType(),
sourceCode: sourceCode && decoder.decode(sourceCode),
outputCode: outputCode && decoder.decode(outputCode),
sourceMap: sourceMap && decoder.decode(sourceMap)
sourceCode: codeFetchRes.sourceCode() || undefined,
outputCode: codeFetchRes.outputCode() || undefined,
sourceMap: codeFetchRes.sourceMap() || undefined
};
}

127
src/deno_dir.rs
View file

@ -11,6 +11,7 @@ use msg;
use ring;
use std;
use std::fmt::Write;
use std::fs;
use std::path::Path;
use std::path::PathBuf;
@ -23,13 +24,13 @@ pub struct CodeFetchOutput {
pub module_name: String,
pub filename: String,
pub media_type: msg::MediaType,
pub source_code: Vec<u8>,
pub maybe_output_code: Option<Vec<u8>>,
pub maybe_source_map: Option<Vec<u8>>,
pub source_code: String,
pub maybe_output_code: Option<String>,
pub maybe_source_map: Option<String>,
}
impl CodeFetchOutput {
pub fn js_source<'a>(&'a self) -> &'a Vec<u8> {
pub fn js_source<'a>(&'a self) -> &'a String {
match self.maybe_output_code {
None => &self.source_code,
Some(ref output_code) => output_code,
@ -108,7 +109,7 @@ impl DenoDir {
pub fn cache_path(
self: &Self,
filename: &str,
source_code: &[u8],
source_code: &str,
) -> (PathBuf, PathBuf) {
let cache_key = source_code_hash(filename, source_code);
(
@ -120,25 +121,25 @@ impl DenoDir {
fn load_cache(
self: &Self,
filename: &str,
source_code: &[u8],
) -> Result<(Vec<u8>, Vec<u8>), std::io::Error> {
source_code: &str,
) -> Result<(String, String), std::io::Error> {
let (output_code, source_map) = self.cache_path(filename, source_code);
debug!(
"load_cache code: {} map: {}",
output_code.display(),
source_map.display()
);
let read_output_code = fs::read(&output_code)?;
let read_source_map = fs::read(&source_map)?;
let read_output_code = fs::read_to_string(&output_code)?;
let read_source_map = fs::read_to_string(&source_map)?;
Ok((read_output_code, read_source_map))
}
pub fn code_cache(
self: &Self,
filename: &str,
source_code: &[u8],
output_code: &[u8],
source_map: &[u8],
source_code: &str,
output_code: &str,
source_map: &str,
) -> std::io::Result<()> {
let (cache_path, source_map_path) = self.cache_path(filename, source_code);
// TODO(ry) This is a race condition w.r.t to exists() -- probably should
@ -148,8 +149,8 @@ impl DenoDir {
if cache_path.exists() && source_map_path.exists() {
Ok(())
} else {
fs::write(cache_path, output_code)?;
fs::write(source_map_path, source_map)?;
fs::write(cache_path, output_code.as_bytes())?;
fs::write(source_map_path, source_map.as_bytes())?;
Ok(())
}
}
@ -221,7 +222,7 @@ impl DenoDir {
return Err(e.into());
}
}
Ok(c) => c,
Ok(c) => String::from_utf8(c).unwrap(),
};
// .mime file might not exists
// this is okay for local source: maybe_content_type_str will be None
@ -318,7 +319,8 @@ impl DenoDir {
out.source_code = filter_shebang(out.source_code);
let result = self.load_cache(out.filename.as_str(), &out.source_code);
let result =
self.load_cache(out.filename.as_str(), out.source_code.as_str());
match result {
Err(err) => {
if err.kind() == std::io::ErrorKind::NotFound {
@ -433,7 +435,7 @@ impl DenoDir {
}
impl SourceMapGetter for DenoDir {
fn get_source_map(&self, script_name: &str) -> Option<Vec<u8>> {
fn get_source_map(&self, script_name: &str) -> Option<String> {
match self.code_fetch(script_name, ".") {
Err(_e) => {
return None;
@ -468,21 +470,15 @@ fn get_cache_filename(basedir: &Path, url: &Url) -> PathBuf {
}
// https://github.com/denoland/deno/blob/golang/deno_dir.go#L25-L30
fn source_code_hash(filename: &str, source_code: &[u8]) -> String {
fn source_code_hash(filename: &str, source_code: &str) -> String {
let mut ctx = ring::digest::Context::new(&ring::digest::SHA1);
ctx.update(filename.as_bytes());
ctx.update(source_code);
ctx.update(source_code.as_bytes());
let digest = ctx.finish();
let mut out = String::with_capacity(digest.as_ref().len() * 2);
const DIGIT_TO_CHAR: [char; 16] = [
'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e',
'f',
];
let mut out = String::new();
// TODO There must be a better way to do this...
for byte in digest.as_ref() {
let low = (byte & 0x0F) as usize;
let high = (byte >> 4) as usize;
out.push(DIGIT_TO_CHAR[high]);
out.push(DIGIT_TO_CHAR[low]);
write!(&mut out, "{:02x}", byte).unwrap();
}
out
}
@ -543,15 +539,15 @@ fn map_content_type(path: &Path, content_type: Option<&str>) -> msg::MediaType {
}
}
fn filter_shebang(code: Vec<u8>) -> Vec<u8> {
if !code.starts_with(b"#!") {
fn filter_shebang(code: String) -> String {
if !code.starts_with("#!") {
return code;
}
if let Some(i) = code.iter().position(|&x| x == b'\n') {
let rest = &code[i..];
rest.to_vec()
if let Some(i) = code.find('\n') {
let (_, rest) = code.split_at(i);
String::from(rest)
} else {
Vec::new()
String::from("")
}
}
@ -604,7 +600,7 @@ mod tests {
.path()
.join("gen/a3e29aece8d35a19bf9da2bb1c086af71fb36ed5.js.map")
),
deno_dir.cache_path("hello.ts", b"1+2")
deno_dir.cache_path("hello.ts", "1+2")
);
}
@ -613,9 +609,9 @@ mod tests {
let (_temp_dir, deno_dir) = test_setup();
let filename = "hello.js";
let source_code = b"1+2";
let output_code = b"1+2 // output code";
let source_map = b"{}";
let source_code = "1+2";
let output_code = "1+2 // output code";
let source_map = "{}";
let (cache_path, source_map_path) =
deno_dir.cache_path(filename, source_code);
assert!(
@ -629,24 +625,24 @@ mod tests {
let r = deno_dir.code_cache(filename, source_code, output_code, source_map);
r.expect("code_cache error");
assert!(cache_path.exists());
assert_eq!(output_code, &fs::read(&cache_path).unwrap()[..]);
assert_eq!(output_code, fs::read_to_string(&cache_path).unwrap());
}
#[test]
fn test_source_code_hash() {
assert_eq!(
"a3e29aece8d35a19bf9da2bb1c086af71fb36ed5",
source_code_hash("hello.ts", b"1+2")
source_code_hash("hello.ts", "1+2")
);
// Different source_code should result in different hash.
assert_eq!(
"914352911fc9c85170908ede3df1128d690dda41",
source_code_hash("hello.ts", b"1")
source_code_hash("hello.ts", "1")
);
// Different filename should result in different hash.
assert_eq!(
"2e396bc66101ecc642db27507048376d972b1b70",
source_code_hash("hi.ts", b"1+2")
source_code_hash("hi.ts", "1+2")
);
}
@ -668,9 +664,10 @@ mod tests {
println!("module_name {} filename {}", module_name, filename);
assert!(result.is_ok());
let r = result.unwrap();
let expected: &[u8] =
b"export { printHello } from \"./print_hello.ts\";\n";
assert_eq!(r.source_code, expected);
assert_eq!(
&(r.source_code),
"export { printHello } from \"./print_hello.ts\";\n"
);
assert_eq!(&(r.media_type), &msg::MediaType::TypeScript);
// Should not create .mime file due to matching ext
assert!(fs::read_to_string(&mime_file_name).is_err());
@ -680,9 +677,10 @@ mod tests {
let result2 = deno_dir.get_source_code(module_name, &filename);
assert!(result2.is_ok());
let r2 = result2.unwrap();
let expected2: &[u8] =
b"export { printHello } from \"./print_hello.ts\";\n";
assert_eq!(r2.source_code, expected2);
assert_eq!(
&(r2.source_code),
"export { printHello } from \"./print_hello.ts\";\n"
);
// If get_source_code does not call remote, this should be JavaScript
// as we modified before! (we do not overwrite .mime due to no http fetch)
assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript);
@ -697,8 +695,7 @@ mod tests {
let result3 = deno_dir.get_source_code(module_name, &filename);
assert!(result3.is_ok());
let r3 = result3.unwrap();
let expected3: &[u8] =
b"export { printHello } from \"./print_hello.ts\";\n";
let expected3 = "export { printHello } from \"./print_hello.ts\";\n";
assert_eq!(r3.source_code, expected3);
// Now the old .mime file should have gone! Resolved back to TypeScript
assert_eq!(&(r3.media_type), &msg::MediaType::TypeScript);
@ -724,7 +721,7 @@ mod tests {
println!("module_name {} filename {}", module_name, filename);
assert!(result.is_ok());
let r = result.unwrap();
let expected: &[u8] = b"export const loaded = true;\n";
let expected = "export const loaded = true;\n";
assert_eq!(r.source_code, expected);
// Mismatch ext with content type, create .mime
assert_eq!(&(r.media_type), &msg::MediaType::JavaScript);
@ -738,7 +735,7 @@ mod tests {
let result2 = deno_dir.get_source_code(module_name, &filename);
assert!(result2.is_ok());
let r2 = result2.unwrap();
let expected2: &[u8] = b"export const loaded = true;\n";
let expected2 = "export const loaded = true;\n";
assert_eq!(r2.source_code, expected2);
// If get_source_code does not call remote, this should be TypeScript
// as we modified before! (we do not overwrite .mime due to no http fetch)
@ -754,7 +751,7 @@ mod tests {
let result3 = deno_dir.get_source_code(module_name, &filename);
assert!(result3.is_ok());
let r3 = result3.unwrap();
let expected3: &[u8] = b"export const loaded = true;\n";
let expected3 = "export const loaded = true;\n";
assert_eq!(r3.source_code, expected3);
// Now the old .mime file should be overwritten back to JavaScript!
// (due to http fetch)
@ -785,7 +782,7 @@ mod tests {
let result = deno_dir.fetch_remote_source(module_name, &filename);
assert!(result.is_ok());
let r = result.unwrap().unwrap();
assert_eq!(&(r.source_code), b"export const loaded = true;\n");
assert_eq!(&(r.source_code), "export const loaded = true;\n");
assert_eq!(&(r.media_type), &msg::MediaType::TypeScript);
// matching ext, no .mime file created
assert!(fs::read_to_string(&mime_file_name).is_err());
@ -795,7 +792,7 @@ mod tests {
let result2 = deno_dir.fetch_local_source(module_name, &filename);
assert!(result2.is_ok());
let r2 = result2.unwrap().unwrap();
assert_eq!(&(r2.source_code), b"export const loaded = true;\n");
assert_eq!(&(r2.source_code), "export const loaded = true;\n");
// Not MediaType::TypeScript due to .mime modification
assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript);
});
@ -818,7 +815,7 @@ mod tests {
let result = deno_dir.fetch_remote_source(module_name, &filename);
assert!(result.is_ok());
let r = result.unwrap().unwrap();
assert_eq!(&(r.source_code), b"export const loaded = true;\n");
assert_eq!(&(r.source_code), "export const loaded = true;\n");
assert_eq!(&(r.media_type), &msg::MediaType::TypeScript);
// no ext, should create .mime file
assert_eq!(
@ -837,7 +834,7 @@ mod tests {
let result_2 = deno_dir.fetch_remote_source(module_name_2, &filename_2);
assert!(result_2.is_ok());
let r2 = result_2.unwrap().unwrap();
assert_eq!(&(r2.source_code), b"export const loaded = true;\n");
assert_eq!(&(r2.source_code), "export const loaded = true;\n");
assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript);
// mismatch ext, should create .mime file
assert_eq!(
@ -857,7 +854,7 @@ mod tests {
let result_3 = deno_dir.fetch_remote_source(module_name_3, &filename_3);
assert!(result_3.is_ok());
let r3 = result_3.unwrap().unwrap();
assert_eq!(&(r3.source_code), b"export const loaded = true;\n");
assert_eq!(&(r3.source_code), "export const loaded = true;\n");
assert_eq!(&(r3.media_type), &msg::MediaType::TypeScript);
// unknown ext, should create .mime file
assert_eq!(
@ -880,7 +877,7 @@ mod tests {
let result = deno_dir.fetch_local_source(module_name, &filename);
assert!(result.is_ok());
let r = result.unwrap().unwrap();
assert_eq!(&(r.source_code), b"export const loaded = true;\n");
assert_eq!(&(r.source_code), "export const loaded = true;\n");
assert_eq!(&(r.media_type), &msg::MediaType::TypeScript);
}
@ -1286,11 +1283,11 @@ mod tests {
#[test]
fn test_filter_shebang() {
assert_eq!(filter_shebang(b"".to_vec()), b"");
assert_eq!(filter_shebang(b"#".to_vec()), b"#");
assert_eq!(filter_shebang(b"#!".to_vec()), b"");
assert_eq!(filter_shebang(b"#!\n\n".to_vec()), b"\n\n");
let code = b"#!/usr/bin/env deno\nconsole.log('hello');\n".to_vec();
assert_eq!(filter_shebang(code), b"\nconsole.log('hello');\n");
assert_eq!(filter_shebang("".to_string()), "");
assert_eq!(filter_shebang("#".to_string()), "#");
assert_eq!(filter_shebang("#!".to_string()), "");
assert_eq!(filter_shebang("#!\n\n".to_string()), "\n\n");
let code = "#!/usr/bin/env deno\nconsole.log('hello');\n".to_string();
assert_eq!(filter_shebang(code), "\nconsole.log('hello');\n");
}
}

8
src/http_util.rs
View file

@ -55,7 +55,7 @@ fn resolve_uri_from_location(base_uri: &Uri, location: &str) -> Uri {
// The CodeFetch message is used to load HTTP javascript resources and expects a
// synchronous response, this utility method supports that.
pub fn fetch_sync_string(module_name: &str) -> DenoResult<(Vec<u8>, String)> {
pub fn fetch_sync_string(module_name: &str) -> DenoResult<(String, String)> {
let url = module_name.parse::<Uri>().unwrap();
let client = get_client();
// TODO(kevinkassimo): consider set a max redirection counter
@ -93,11 +93,11 @@ pub fn fetch_sync_string(module_name: &str) -> DenoResult<(Vec<u8>, String)> {
let body = response
.into_body()
.concat2()
.map(|body| body.to_vec())
.map(|body| String::from_utf8(body.to_vec()).unwrap())
.map_err(DenoError::from);
body.join(future::ok(content_type))
}).and_then(|(body_bytes, maybe_content_type)| {
future::ok((body_bytes, maybe_content_type.unwrap()))
}).and_then(|(body_string, maybe_content_type)| {
future::ok((body_string, maybe_content_type.unwrap()))
});
tokio_util::block_on(fetch_future)

16
src/js_errors.rs
View file

@ -17,7 +17,7 @@ use std::collections::HashMap;
pub trait SourceMapGetter {
/// Returns the raw source map file.
fn get_source_map(&self, script_name: &str) -> Option<Vec<u8>>;
fn get_source_map(&self, script_name: &str) -> Option<String>;
}
struct SourceMap {
@ -287,9 +287,7 @@ fn parse_map_string(
}
_ => match getter.get_source_map(script_name) {
None => None,
Some(raw_source_map) => SourceMap::from_json(
&String::from_utf8(raw_source_map).expect("SourceMap is not utf-8"),
),
Some(raw_source_map) => SourceMap::from_json(&raw_source_map),
},
}
}
@ -346,13 +344,13 @@ mod tests {
struct MockSourceMapGetter {}
impl SourceMapGetter for MockSourceMapGetter {
fn get_source_map(&self, script_name: &str) -> Option<Vec<u8>> {
let s: &[u8] = match script_name {
"foo_bar.ts" => br#"{"sources": ["foo_bar.ts"], "mappings":";;;IAIA,OAAO,CAAC,GAAG,CAAC,qBAAqB,EAAE,EAAE,CAAC,OAAO,CAAC,CAAC;IAC/C,OAAO,CAAC,GAAG,CAAC,eAAe,EAAE,IAAI,CAAC,QAAQ,CAAC,IAAI,CAAC,CAAC;IACjD,OAAO,CAAC,GAAG,CAAC,WAAW,EAAE,IAAI,CAAC,QAAQ,CAAC,EAAE,CAAC,CAAC;IAE3C,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC"}"#,
"bar_baz.ts" => br#"{"sources": ["bar_baz.ts"], "mappings":";;;IAEA,CAAC,KAAK,IAAI,EAAE;QACV,MAAM,GAAG,GAAG,sDAAa,OAAO,2BAAC,CAAC;QAClC,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC;IACnB,CAAC,CAAC,EAAE,CAAC;IAEQ,QAAA,GAAG,GAAG,KAAK,CAAC;IAEzB,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC"}"#,
fn get_source_map(&self, script_name: &str) -> Option<String> {
let s = match script_name {
"foo_bar.ts" => r#"{"sources": ["foo_bar.ts"], "mappings":";;;IAIA,OAAO,CAAC,GAAG,CAAC,qBAAqB,EAAE,EAAE,CAAC,OAAO,CAAC,CAAC;IAC/C,OAAO,CAAC,GAAG,CAAC,eAAe,EAAE,IAAI,CAAC,QAAQ,CAAC,IAAI,CAAC,CAAC;IACjD,OAAO,CAAC,GAAG,CAAC,WAAW,EAAE,IAAI,CAAC,QAAQ,CAAC,EAAE,CAAC,CAAC;IAE3C,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC"}"#,
"bar_baz.ts" => r#"{"sources": ["bar_baz.ts"], "mappings":";;;IAEA,CAAC,KAAK,IAAI,EAAE;QACV,MAAM,GAAG,GAAG,sDAAa,OAAO,2BAAC,CAAC;QAClC,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC;IACnB,CAAC,CAAC,EAAE,CAAC;IAEQ,QAAA,GAAG,GAAG,KAAK,CAAC;IAEzB,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC"}"#,
_ => return None,
};
Some(s.to_vec())
Some(s.to_string())
}
}

14
src/msg.fbs
View file

@ -160,16 +160,18 @@ table CodeFetchRes {
module_name: string;
filename: string;
media_type: MediaType;
source_code: [ubyte];
output_code: [ubyte]; // Non-empty only if cached.
source_map: [ubyte]; // Non-empty only if cached.
// TODO These should be [ubyte].
// See: https://github.com/denoland/deno/issues/1113
source_code: string;
output_code: string; // Non-empty only if cached.
source_map: string; // Non-empty only if cached.
}
table CodeCache {
filename: string;
source_code: [ubyte];
output_code: [ubyte];
source_map: [ubyte];
source_code: string;
output_code: string;
source_map: string;
}
table Chdir {

8
src/ops.rs
View file

@ -268,14 +268,14 @@ fn op_code_fetch(
module_name: Some(builder.create_string(&out.module_name)),
filename: Some(builder.create_string(&out.filename)),
media_type: out.media_type,
source_code: Some(builder.create_vector(&out.source_code)),
source_code: Some(builder.create_string(&out.source_code)),
..Default::default()
};
if let Some(ref output_code) = out.maybe_output_code {
msg_args.output_code = Some(builder.create_vector(output_code));
msg_args.output_code = Some(builder.create_string(output_code));
}
if let Some(ref source_map) = out.maybe_source_map {
msg_args.source_map = Some(builder.create_vector(source_map));
msg_args.source_map = Some(builder.create_string(source_map));
}
let inner = msg::CodeFetchRes::create(builder, &msg_args);
Ok(serialize_response(
@ -305,7 +305,7 @@ fn op_code_cache(
Box::new(futures::future::result(|| -> OpResult {
state
.dir
.code_cache(filename, &source_code, &output_code, &source_map)?;
.code_cache(filename, source_code, output_code, source_map)?;
Ok(empty_buf())
}()))
}