From 363b968bfcef26c30f84e485beec6194e5b1dd98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 22 Nov 2019 18:14:34 +0100 Subject: [PATCH] minor clean ups in TS compiler (#3394) --- cli/compilers/js.rs | 2 +- cli/compilers/ts.rs | 155 ++++++++++++++++---------------------------- cli/worker.rs | 76 ++++++++++------------ 3 files changed, 92 insertions(+), 141 deletions(-) diff --git a/cli/compilers/js.rs b/cli/compilers/js.rs index b6277659f0..02a7cc5b17 100644 --- a/cli/compilers/js.rs +++ b/cli/compilers/js.rs @@ -2,7 +2,7 @@ use crate::compilers::CompiledModule; use crate::compilers::CompiledModuleFuture; use crate::file_fetcher::SourceFile; -use crate::futures::future::FutureExt; +use futures::future::FutureExt; use std::pin::Pin; use std::str; diff --git a/cli/compilers/ts.rs b/cli/compilers/ts.rs index 34bf74ab1d..c255e18beb 100644 --- a/cli/compilers/ts.rs +++ b/cli/compilers/ts.rs @@ -16,7 +16,6 @@ use deno::Buf; use deno::ErrBox; use deno::ModuleSpecifier; use futures::future::FutureExt; -use futures::future::TryFutureExt; use futures::Future; use regex::Regex; use std::collections::HashSet; @@ -272,33 +271,22 @@ impl TsCompiler { let worker = TsCompiler::setup_worker(global_state.clone()); let worker_ = worker.clone(); - let first_msg_fut = async move { - worker.post_message(req_msg).await.unwrap(); - let result = worker.await; - if let Err(err) = result { - // TODO(ry) Need to forward the error instead of exiting. - eprintln!("{}", err.to_string()); - std::process::exit(1); - } + + async move { + worker.post_message(req_msg).await?; + worker.await?; debug!("Sent message to worker"); - worker_.get_message().await - }; - - first_msg_fut.map_err(|_| panic!("not handled")).and_then( - move |maybe_msg: Option| { - debug!("Received message from worker"); - - if let Some(msg) = maybe_msg { - let json_str = std::str::from_utf8(&msg).unwrap(); - debug!("Message: {}", json_str); - if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) { - return futures::future::err(ErrBox::from(diagnostics)); - } + let maybe_msg = worker_.get_message().await?; + debug!("Received message from worker"); + if let Some(msg) = maybe_msg { + let json_str = std::str::from_utf8(&msg).unwrap(); + debug!("Message: {}", json_str); + if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) { + return Err(ErrBox::from(diagnostics)); } - - futures::future::ok(()) - }, - ) + } + Ok(()) + } } /// Mark given module URL as compiled to avoid multiple compilations of same module @@ -382,63 +370,27 @@ impl TsCompiler { .add("Compile", &module_url.to_string()); let global_state_ = global_state.clone(); - let first_msg_fut = async move { - worker.post_message(req_msg).await.unwrap(); - let result = worker.await; - if let Err(err) = result { - // TODO(ry) Need to forward the error instead of exiting. - eprintln!("{}", err.to_string()); - std::process::exit(1); - } + async move { + worker.post_message(req_msg).await?; + worker.await?; debug!("Sent message to worker"); - worker_.get_message().await - }; - - let fut = first_msg_fut - .map_err(|_| panic!("not handled")) - .and_then(move |maybe_msg: Option| { - debug!("Received message from worker"); - - if let Some(msg) = maybe_msg { - let json_str = std::str::from_utf8(&msg).unwrap(); - debug!("Message: {}", json_str); - if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) { - return futures::future::err(ErrBox::from(diagnostics)); - } + let maybe_msg = worker_.get_message().await?; + if let Some(msg) = maybe_msg { + let json_str = std::str::from_utf8(&msg).unwrap(); + debug!("Message: {}", json_str); + if let Some(diagnostics) = Diagnostic::from_emit_result(json_str) { + return Err(ErrBox::from(diagnostics)); } - - futures::future::ok(()) - }) - .and_then(move |_| { - // if we are this far it means compilation was successful and we can - // load compiled filed from disk - futures::future::ready( - global_state_ - .ts_compiler - .get_compiled_module(&source_file_.url) - .map_err(|e| { - // TODO: this situation shouldn't happen - panic!( - "Expected to find compiled file: {} {}", - e, source_file_.url - ) - }), - ) - }) - .and_then(move |compiled_module| { - // Explicit drop to keep reference alive until future completes. - drop(compiling_job); - - futures::future::ok(compiled_module) - }) - .then(move |r| { - debug!(">>>>> compile_sync END"); - // TODO(ry) do this in worker's destructor. - // resource.close(); - futures::future::ready(r) - }); - - fut.boxed() + } + let compiled_module = global_state_ + .ts_compiler + .get_compiled_module(&source_file_.url) + .expect("Expected to find compiled file"); + drop(compiling_job); + debug!(">>>>> compile_sync END"); + Ok(compiled_module) + } + .boxed() } /// Get associated `CompiledFileMetadata` for given module if it exists. @@ -684,20 +636,22 @@ mod tests { String::from("hello.js"), ]); - tokio_util::run( - mock_state + let fut = async move { + let result = mock_state .ts_compiler .compile_async(mock_state.clone(), &out) - .then(|result| { - assert!(result.is_ok()); - assert!(result - .unwrap() - .code - .as_bytes() - .starts_with("console.log(\"Hello World\");".as_bytes())); - futures::future::ok(()) - }), - ) + .await; + + assert!(result.is_ok()); + assert!(result + .unwrap() + .code + .as_bytes() + .starts_with("console.log(\"Hello World\");".as_bytes())); + Ok(()) + }; + + tokio_util::run(fut.boxed()) } #[test] @@ -718,19 +672,20 @@ mod tests { String::from("$deno$/bundle.js"), ]); - tokio_util::run( - state + let fut = async move { + let result = state .ts_compiler .bundle_async( state.clone(), module_name, Some(String::from("$deno$/bundle.js")), ) - .then(|result| { - assert!(result.is_ok()); - futures::future::ok(()) - }), - ) + .await; + + assert!(result.is_ok()); + Ok(()) + }; + tokio_util::run(fut.boxed()) } #[test] diff --git a/cli/worker.rs b/cli/worker.rs index 1b931a85de..08ac436593 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -128,17 +128,19 @@ impl Worker { maybe_code, loader, modules, - ) - .get_future(isolate); - recursive_load.and_then(move |id| { + ); + + async move { + let id = recursive_load.get_future(isolate).await?; worker.state.global_state.progress.done(); - if is_prefetch { - futures::future::ok(()) - } else { + + if !is_prefetch { let mut isolate = worker.isolate.lock().unwrap(); - futures::future::ready(isolate.mod_evaluate(id)) + return isolate.mod_evaluate(id); } - }) + + Ok(()) + } } /// Post message to worker as a host. @@ -235,15 +237,13 @@ mod tests { tokio_util::run(async move { let mut worker = Worker::new("TEST".to_string(), StartupData::None, state, ext); - worker + let result = worker .execute_mod_async(&module_specifier, None, false) - .then(|result| { - if let Err(err) = result { - eprintln!("execute_mod err {:?}", err); - } - tokio_util::panic_on_error(worker) - }) - .await + .await; + if let Err(err) = result { + eprintln!("execute_mod err {:?}", err); + } + tokio_util::panic_on_error(worker).await }); let metrics = &state_.metrics; @@ -277,15 +277,13 @@ mod tests { tokio_util::run(async move { let mut worker = Worker::new("TEST".to_string(), StartupData::None, state, ext); - worker + let result = worker .execute_mod_async(&module_specifier, None, false) - .then(|result| { - if let Err(err) = result { - eprintln!("execute_mod err {:?}", err); - } - tokio_util::panic_on_error(worker) - }) - .await + .await; + if let Err(err) = result { + eprintln!("execute_mod err {:?}", err); + } + tokio_util::panic_on_error(worker).await }); let metrics = &state_.metrics; @@ -328,15 +326,14 @@ mod tests { ext, ); worker.execute("denoMain()").unwrap(); - worker + let result = worker .execute_mod_async(&module_specifier, None, false) - .then(|result| { - if let Err(err) = result { - eprintln!("execute_mod err {:?}", err); - } - tokio_util::panic_on_error(worker) - }) - .await + .await; + + if let Err(err) = result { + eprintln!("execute_mod err {:?}", err); + } + tokio_util::panic_on_error(worker).await }); assert_eq!(state_.metrics.resolve_count.load(Ordering::SeqCst), 3); @@ -386,14 +383,13 @@ mod tests { let worker_ = worker.clone(); - tokio::spawn( - worker - .then(move |r| { - r.unwrap(); - futures::future::ok(()) - }) - .compat(), - ); + let fut = async move { + let r = worker.await; + r.unwrap(); + Ok(()) + }; + + tokio::spawn(fut.boxed().compat()); let msg = json!("hi").to_string().into_boxed_str().into_boxed_bytes();