diff --git a/cli/file_watcher.rs b/cli/file_watcher.rs index c76e292191cf62..4aa93c581f547d 100644 --- a/cli/file_watcher.rs +++ b/cli/file_watcher.rs @@ -22,7 +22,7 @@ use tokio::time::{delay_for, Delay}; const DEBOUNCE_INTERVAL_MS: Duration = Duration::from_millis(200); -type FileWatcherFuture = Pin>>>; +type FileWatcherFuture = Pin>>; struct Debounce { delay: Delay, @@ -63,7 +63,7 @@ impl Stream for Debounce { } } -async fn error_handler(watch_future: FileWatcherFuture<()>) { +async fn error_handler(watch_future: FileWatcherFuture>) { let result = watch_future.await; if let Err(err) = result { let msg = format!("{}: {}", colors::red_bold("error"), err.to_string(),); @@ -94,7 +94,7 @@ pub async fn watch_func( ) -> Result<(), AnyError> where F: Fn() -> Result, AnyError>, - G: Fn(Vec) -> FileWatcherFuture<()>, + G: Fn(Vec) -> FileWatcherFuture>, { let mut debounce = Debounce::new(); @@ -129,6 +129,17 @@ where } } +pub enum ModuleResolutionResult { + Success { + paths_to_watch: Vec, + module_info: T, + }, + Fail { + source_path: PathBuf, + error: AnyError, + }, +} + /// This function adds watcher functionality to subcommands like `run` or `bundle`. /// The difference from [`watch_func`] is that this does depend on [`ModuleGraph`]. /// @@ -154,51 +165,63 @@ pub async fn watch_func_with_module_resolution( job_name: &str, ) -> Result<(), AnyError> where - F: Fn() -> FileWatcherFuture<(Vec, T)>, - G: Fn(T) -> FileWatcherFuture<()>, + F: Fn() -> FileWatcherFuture>, + G: Fn(T) -> FileWatcherFuture>, T: Clone, { let mut debounce = Debounce::new(); // Store previous data. If module resolution fails at some point, the watcher will try to // continue watching files using these data. - let mut paths = None; + let mut paths; let mut module = None; loop { match module_resolver().await { - Ok((next_paths, next_module)) => { - paths = Some(next_paths); - module = Some(next_module); + ModuleResolutionResult::Success { + paths_to_watch, + module_info, + } => { + paths = paths_to_watch; + module = Some(module_info); } - Err(e) => { - // If at least one of `paths` and `module` is `None`, the watcher cannot decide which files - // should be watched. So return the error immediately without watching anything. - if paths.is_none() || module.is_none() { - return Err(e); + ModuleResolutionResult::Fail { source_path, error } => { + paths = vec![source_path]; + if module.is_none() { + eprintln!("{}: {}", colors::red_bold("error"), error); } } } - // These `unwrap`s never cause panic since `None` is already checked above. - let cur_paths = paths.clone().unwrap(); - let cur_module = module.clone().unwrap(); + let _watcher = new_watcher(&paths, &debounce)?; - let _watcher = new_watcher(&cur_paths, &debounce)?; - let func = error_handler(operation(cur_module)); - let mut is_file_changed = false; - select! { - _ = debounce.next() => { - is_file_changed = true; + if let Some(module) = &module { + let func = error_handler(operation(module.clone())); + let mut is_file_changed = false; + select! { + _ = debounce.next() => { + is_file_changed = true; + info!( + "{} File change detected! Restarting!", + colors::intense_blue("Watcher"), + ); + }, + _ = func => {}, + }; + + if !is_file_changed { + info!( + "{} {} finished! Restarting on file change...", + colors::intense_blue("Watcher"), + job_name, + ); + debounce.next().await; info!( "{} File change detected! Restarting!", colors::intense_blue("Watcher"), ); - }, - _ = func => {}, - }; - - if !is_file_changed { + } + } else { info!( - "{} {} finished! Restarting on file change...", + "{} {} failed! Restarting on file change...", colors::intense_blue("Watcher"), job_name, ); diff --git a/cli/main.rs b/cli/main.rs index fdbfdad773a7e2..de2e1b40251ee8 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -50,6 +50,7 @@ mod worker; use crate::file_fetcher::File; use crate::file_fetcher::FileFetcher; +use crate::file_watcher::ModuleResolutionResult; use crate::media_type::MediaType; use crate::permissions::Permissions; use crate::program_state::ProgramState; @@ -307,10 +308,11 @@ async fn bundle_command( let module_resolver = || { let flags = flags.clone(); - let source_file = source_file.clone(); + let source_file1 = source_file.clone(); + let source_file2 = source_file.clone(); async move { let module_specifier = - ModuleSpecifier::resolve_url_or_path(&source_file)?; + ModuleSpecifier::resolve_url_or_path(&source_file1)?; debug!(">>>>> bundle START"); let program_state = ProgramState::new(flags.clone())?; @@ -373,6 +375,16 @@ async fn bundle_command( Ok((paths_to_watch, module_graph)) } + .map(move |result| match result { + Ok((paths_to_watch, module_graph)) => ModuleResolutionResult::Success { + paths_to_watch, + module_info: module_graph, + }, + Err(e) => ModuleResolutionResult::Fail { + source_path: PathBuf::from(source_file2), + error: e, + }, + }) .boxed_local() }; @@ -423,7 +435,10 @@ async fn bundle_command( ) .await?; } else { - let (_, module_graph) = module_resolver().await?; + let module_graph = match module_resolver().await { + ModuleResolutionResult::Fail { error, .. } => return Err(error), + ModuleResolutionResult::Success { module_info, .. } => module_info, + }; operation(module_graph).await?; } @@ -610,10 +625,11 @@ async fn run_from_stdin(flags: Flags) -> Result<(), AnyError> { async fn run_with_watch(flags: Flags, script: String) -> Result<(), AnyError> { let module_resolver = || { - let script = script.clone(); + let script1 = script.clone(); + let script2 = script.clone(); let flags = flags.clone(); async move { - let main_module = ModuleSpecifier::resolve_url_or_path(&script)?; + let main_module = ModuleSpecifier::resolve_url_or_path(&script1)?; let program_state = ProgramState::new(flags)?; let handler = Rc::new(RefCell::new(FetchHandler::new( &program_state, @@ -641,6 +657,16 @@ async fn run_with_watch(flags: Flags, script: String) -> Result<(), AnyError> { Ok((paths_to_watch, main_module)) } + .map(move |result| match result { + Ok((paths_to_watch, module_info)) => ModuleResolutionResult::Success { + paths_to_watch, + module_info, + }, + Err(e) => ModuleResolutionResult::Fail { + source_path: PathBuf::from(script2), + error: e, + }, + }) .boxed_local() }; diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 39147f0383b7d0..430bd53cd7b534 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -527,6 +527,9 @@ fn fmt_watch_test() { let actual = std::fs::read_to_string(badly_formatted).unwrap(); assert_eq!(expected, actual); + // the watcher process is still alive + assert!(child.try_wait().unwrap().is_none()); + child.kill().unwrap(); drop(t); } @@ -1231,7 +1234,7 @@ fn bundle_js_watch() { assert!(stderr_lines.next().unwrap().contains("mod6.bundle.js")); let file = PathBuf::from(&bundle); assert!(file.is_file()); - assert!(stderr_lines.next().unwrap().contains("Bundle finished!")); + wait_for_process_finished("Bundle", &mut stderr_lines); std::fs::write(&file_to_watch, "console.log('Hello world2');") .expect("error writing file"); @@ -1244,7 +1247,7 @@ fn bundle_js_watch() { assert!(stderr_lines.next().unwrap().contains("mod6.bundle.js")); let file = PathBuf::from(&bundle); assert!(file.is_file()); - assert!(stderr_lines.next().unwrap().contains("Bundle finished!")); + wait_for_process_finished("Bundle", &mut stderr_lines); // Confirm that the watcher keeps on working even if the file is updated and has invalid syntax std::fs::write(&file_to_watch, "syntax error ^^") @@ -1258,24 +1261,29 @@ fn bundle_js_watch() { assert!(stderr_lines.next().unwrap().contains("mod6.bundle.js")); let file = PathBuf::from(&bundle); assert!(file.is_file()); - assert!(stderr_lines.next().unwrap().contains("Bundle finished!")); + wait_for_process_finished("Bundle", &mut stderr_lines); + + // the watcher process is still alive + assert!(deno.try_wait().unwrap().is_none()); deno.kill().unwrap(); drop(t); } -/// Confirm that the watcher exits immediately if module resolution fails at the first attempt +/// Confirm that the watcher continues to work even if module resolution fails at the *first* attempt #[test] -fn bundle_watch_fail() { +fn bundle_watch_not_exit() { let t = TempDir::new().expect("tempdir fail"); let file_to_watch = t.path().join("file_to_watch.js"); std::fs::write(&file_to_watch, "syntax error ^^") .expect("error writing file"); + let target_file = t.path().join("target.js"); let mut deno = util::deno_cmd() .current_dir(util::root_path()) .arg("bundle") .arg(&file_to_watch) + .arg(&target_file) .arg("--watch") .arg("--unstable") .env("NO_COLOR", "1") @@ -1291,7 +1299,26 @@ fn bundle_watch_fail() { std::thread::sleep(std::time::Duration::from_secs(1)); assert!(stderr_lines.next().unwrap().contains("file_to_watch.js")); assert!(stderr_lines.next().unwrap().contains("error:")); - assert!(!deno.wait().unwrap().success()); + assert!(stderr_lines.next().unwrap().contains("Bundle failed!")); + // the target file hasn't been created yet + assert!(!target_file.is_file()); + + // Make sure the watcher actually restarts and works fine with the proper syntax + std::fs::write(&file_to_watch, "console.log(42);") + .expect("error writing file"); + std::thread::sleep(std::time::Duration::from_secs(1)); + assert!(stderr_lines + .next() + .unwrap() + .contains("File change detected!")); + assert!(stderr_lines.next().unwrap().contains("file_to_watch.js")); + assert!(stderr_lines.next().unwrap().contains("target.js")); + wait_for_process_finished("Bundle", &mut stderr_lines); + // bundled file is created + assert!(target_file.is_file()); + + // the watcher process is still alive + assert!(deno.try_wait().unwrap().is_none()); drop(t); } @@ -1328,12 +1355,16 @@ fn info_with_compiled_source() { assert_eq!(output.stderr, b""); } -// Helper function to skip watcher output that doesn't contain -// "Process finished" phrase. -fn wait_for_process_finished(stderr_lines: &mut impl Iterator) { +/// Helper function to skip watcher output that doesn't contain +/// "{job_name} finished" phrase. +fn wait_for_process_finished( + job_name: &str, + stderr_lines: &mut impl Iterator, +) { + let phrase = format!("{} finished", job_name); loop { let msg = stderr_lines.next().unwrap(); - if msg.contains("Process finished") { + if msg.contains(&phrase) { break; } } @@ -1366,7 +1397,7 @@ fn run_watch() { std::io::BufReader::new(stderr).lines().map(|r| r.unwrap()); assert!(stdout_lines.next().unwrap().contains("Hello world")); - wait_for_process_finished(&mut stderr_lines); + wait_for_process_finished("Process", &mut stderr_lines); // TODO(lucacasonato): remove this timeout. It seems to be needed on Linux. std::thread::sleep(std::time::Duration::from_secs(1)); @@ -1379,7 +1410,7 @@ fn run_watch() { assert!(stderr_lines.next().unwrap().contains("Restarting")); assert!(stdout_lines.next().unwrap().contains("Hello world2")); - wait_for_process_finished(&mut stderr_lines); + wait_for_process_finished("Process", &mut stderr_lines); // Add dependency let another_file = t.path().join("another_file.js"); @@ -1393,7 +1424,7 @@ fn run_watch() { std::thread::sleep(std::time::Duration::from_secs(1)); assert!(stderr_lines.next().unwrap().contains("Restarting")); assert!(stdout_lines.next().unwrap().contains('0')); - wait_for_process_finished(&mut stderr_lines); + wait_for_process_finished("Process", &mut stderr_lines); // Confirm that restarting occurs when a new file is updated std::fs::write(&another_file, "export const foo = 42;") @@ -1401,7 +1432,7 @@ fn run_watch() { std::thread::sleep(std::time::Duration::from_secs(1)); assert!(stderr_lines.next().unwrap().contains("Restarting")); assert!(stdout_lines.next().unwrap().contains("42")); - wait_for_process_finished(&mut stderr_lines); + wait_for_process_finished("Process", &mut stderr_lines); // Confirm that the watcher keeps on working even if the file is updated and has invalid syntax std::fs::write(&file_to_watch, "syntax error ^^") @@ -1409,7 +1440,7 @@ fn run_watch() { std::thread::sleep(std::time::Duration::from_secs(1)); assert!(stderr_lines.next().unwrap().contains("Restarting")); assert!(stderr_lines.next().unwrap().contains("error:")); - wait_for_process_finished(&mut stderr_lines); + wait_for_process_finished("Process", &mut stderr_lines); // Then restore the file std::fs::write( @@ -1420,15 +1451,18 @@ fn run_watch() { std::thread::sleep(std::time::Duration::from_secs(1)); assert!(stderr_lines.next().unwrap().contains("Restarting")); assert!(stdout_lines.next().unwrap().contains("42")); - wait_for_process_finished(&mut stderr_lines); + wait_for_process_finished("Process", &mut stderr_lines); + + // the watcher process is still alive + assert!(child.try_wait().unwrap().is_none()); child.kill().unwrap(); drop(t); } -/// Confirm that the watcher exits immediately if module resolution fails at the first attempt +/// Confirm that the watcher continues to work even if module resolution fails at the *first* attempt #[test] -fn run_watch_fail() { +fn run_watch_not_exit() { let t = TempDir::new().expect("tempdir fail"); let file_to_watch = t.path().join("file_to_watch.js"); std::fs::write(&file_to_watch, "syntax error ^^") @@ -1437,22 +1471,36 @@ fn run_watch_fail() { let mut child = util::deno_cmd() .current_dir(util::root_path()) .arg("run") - .arg(&file_to_watch) .arg("--watch") .arg("--unstable") + .arg(&file_to_watch) .env("NO_COLOR", "1") .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) .spawn() .expect("failed to spawn script"); + let stdout = child.stdout.as_mut().unwrap(); + let mut stdout_lines = + std::io::BufReader::new(stdout).lines().map(|r| r.unwrap()); let stderr = child.stderr.as_mut().unwrap(); let mut stderr_lines = std::io::BufReader::new(stderr).lines().map(|r| r.unwrap()); std::thread::sleep(std::time::Duration::from_secs(1)); assert!(stderr_lines.next().unwrap().contains("error:")); - assert!(!child.wait().unwrap().success()); + assert!(stderr_lines.next().unwrap().contains("Process failed!")); + + // Make sure the watcher actually restarts and works fine with the proper syntax + std::fs::write(&file_to_watch, "console.log(42);") + .expect("error writing file"); + std::thread::sleep(std::time::Duration::from_secs(1)); + assert!(stderr_lines.next().unwrap().contains("Restarting")); + assert!(stdout_lines.next().unwrap().contains("42")); + wait_for_process_finished("Process", &mut stderr_lines); + + // the watcher process is still alive + assert!(child.try_wait().unwrap().is_none()); drop(t); }