Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guess extensions on extension not provided #859

Merged
merged 11 commits into from
Oct 2, 2018
75 changes: 54 additions & 21 deletions src/deno_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,41 @@ impl DenoDir {
self: &DenoDir,
module_name: &str,
filename: &str,
) -> DenoResult<String> {
if is_remote(module_name) {
self.fetch_remote_source(module_name, filename)
} else if module_name.starts_with(ASSET_PREFIX) {
) -> DenoResult<CodeFetchOutput> {
if module_name.starts_with(ASSET_PREFIX) {
panic!("Asset resolution should be done in JS, not Rust.");
} else {
assert_eq!(
module_name, filename,
"if a module isn't remote, it should have the same filename"
);
let src = fs::read_to_string(Path::new(filename))?;
Ok(src)
}
let is_module_remote = is_remote(module_name);
let try_extension = |ext| {
let module_name = format!("{}{}", module_name, ext);
let filename = format!("{}{}", filename, ext);
let source_code = if is_module_remote {
self.fetch_remote_source(&module_name, &filename)?
} else {
assert_eq!(
module_name, filename,
"if a module isn't remote, it should have the same filename"
);
fs::read_to_string(Path::new(&filename))?
};
return Ok(CodeFetchOutput {
module_name: module_name.to_string(),
filename: filename.to_string(),
source_code,
maybe_output_code: None,
});
};
// Has extension, no guessing required
if module_name.ends_with(".ts") || module_name.ends_with(".js") {
kevinkassimo marked this conversation as resolved.
Show resolved Hide resolved
return try_extension("");
kevinkassimo marked this conversation as resolved.
Show resolved Hide resolved
}
debug!("Trying {}.ts...", module_name);
let ts_attempt = try_extension(".ts");
if let Ok(_) = ts_attempt {
return ts_attempt;
}
debug!("Trying {}.js...", module_name);
try_extension(".js")
}

pub fn code_fetch(
Expand All @@ -161,16 +183,7 @@ impl DenoDir {
let (module_name, filename) =
self.resolve_module(module_specifier, containing_file)?;

let result = self
.get_source_code(module_name.as_str(), filename.as_str())
.and_then(|source_code| {
Ok(CodeFetchOutput {
module_name,
filename,
source_code,
maybe_output_code: None,
})
});
let result = self.get_source_code(module_name.as_str(), filename.as_str());
let out = match result {
Err(err) => {
if err.kind() == ErrorKind::NotFound {
Expand Down Expand Up @@ -420,6 +433,26 @@ fn test_code_fetch() {
//println!("code_fetch_output {:?}", code_fetch_output);
}

#[test]
fn test_code_fetch_no_ext() {
let (_temp_dir, deno_dir) = test_setup();

let cwd = std::env::current_dir().unwrap();
let cwd_string = String::from(cwd.to_str().unwrap()) + "/";

// Assuming cwd is the deno repo root.
let module_specifier = "./js/main";
let containing_file = cwd_string.as_str();
let r = deno_dir.code_fetch(module_specifier, containing_file);
assert!(r.is_ok());

// Assuming cwd is the deno repo root.
let module_specifier = "./js/mock_builtin";
let containing_file = cwd_string.as_str();
let r = deno_dir.code_fetch(module_specifier, containing_file);
assert!(r.is_ok());
kevinkassimo marked this conversation as resolved.
Show resolved Hide resolved
}

#[test]
fn test_src_file_to_url_1() {
let (_temp_dir, deno_dir) = test_setup();
Expand Down
16 changes: 11 additions & 5 deletions src/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
use errors::DenoResult;
use tokio_util;

use futures::Future;
// use futures::Future;
use futures::Stream;
use hyper;
use hyper::client::Client;
kevinkassimo marked this conversation as resolved.
Show resolved Hide resolved
use hyper::client::HttpConnector;
use hyper::Uri;
use hyper_rustls;
use std::io;

type Connector = hyper_rustls::HttpsConnector<HttpConnector>;

Expand All @@ -31,10 +32,15 @@ pub fn get_client() -> Client<Connector, hyper::Body> {
pub fn fetch_sync_string(module_name: &str) -> DenoResult<String> {
let url = module_name.parse::<Uri>().unwrap();
let client = get_client();
let future = client
.get(url)
.and_then(|response| response.into_body().concat2());
let body = tokio_util::block_on(future)?;
let get_future = client.get(url);
let response = tokio_util::block_on(get_future)?;
if !response.status().is_success() {
Err(io::Error::new(
io::ErrorKind::NotFound,
format!("cannot load from '{}'", module_name),
))?;
}
let body = tokio_util::block_on(response.into_body().concat2())?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it doesn't matter but this could be done using only one block_on. It would be nice to have a test showing we get NotFound error when trying to hit 404 url.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I have somehow combined into a single block_on, but had to explicitly implement Future for a struct holding Concat2 and StatusCode (to implement using enum that either holds body or and error seems more problematic)
(I started to use Rust only very recently, so this might not be a very good solution from the viewpoint of elegance. Would like to learn about better alternatives!)

btw a NotFound test is added

Ok(String::from_utf8(body.to_vec()).unwrap())
}

Expand Down
7 changes: 7 additions & 0 deletions tests/015_import_no_ext.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Downloading http://localhost:4545/tests/subdir/mod2.ts
Downloading http://localhost:4545/tests/subdir/print_hello.ts
true
[Function: printHello]
[Function: printHello]
true
[Function: printHello]
10 changes: 10 additions & 0 deletions tests/015_import_no_ext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { isTSFile, printHello, phNoExt } from "./subdir/mod3";
console.log(isTSFile);
console.log(printHello);
console.log(phNoExt);

import { isMod4 } from "./subdir/mod4";
console.log(isMod4);

import { printHello as ph } from "http://localhost:4545/tests/subdir/mod2";
console.log(ph);
1 change: 1 addition & 0 deletions tests/subdir/mod3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const isTSFile = false;
3 changes: 3 additions & 0 deletions tests/subdir/mod3.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const isTSFile = true;
export { printHello } from "./print_hello.ts";
export { printHello as phNoExt } from "./print_hello";
1 change: 1 addition & 0 deletions tests/subdir/mod4.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const isMod4 = true;