feat(unstable): Support --watch flag for bundle and fmt subcommands (d…

…enoland#8276)

This commit adds support for "--watch" flag for "bundle" 
and "fmt" subcommands.

In addition to this, it refactors "run --watch" command so that
module resolution will occur every time the file watcher detects 
file addition/deletion, which allows the watcher to observe a file 
that is newly added to the dependency as well.

cli/file_watcher.rs

@@ -4,7 +4,7 @@ use crate::colors;
 use core::task::{Context, Poll};
 use deno_core::error::AnyError;
 use deno_core::futures::stream::{Stream, StreamExt};
-use deno_core::futures::Future;
+use deno_core::futures::{Future, FutureExt};
 use notify::event::Event as NotifyEvent;
 use notify::event::EventKind;
 use notify::Config;
@@ -18,22 +18,21 @@ use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 use std::time::Duration;
 use tokio::select;
-use tokio::time::{interval, Interval};
+use tokio::time::{delay_for, Delay};
 
 const DEBOUNCE_INTERVAL_MS: Duration = Duration::from_millis(200);
 
-// TODO(bartlomieju): rename
-type WatchFuture = Pin<Box<dyn Future<Output = Result<(), AnyError>>>>;
+type FileWatcherFuture<T> = Pin<Box<dyn Future<Output = Result<T, AnyError>>>>;
 
 struct Debounce {
- interval: Interval,
+ delay: Delay,
  event_detected: Arc<AtomicBool>,
 }
 
 impl Debounce {
  fn new() -> Self {
  Self {
- interval: interval(DEBOUNCE_INTERVAL_MS),
+ delay: delay_for(DEBOUNCE_INTERVAL_MS),
  event_detected: Arc::new(AtomicBool::new(false)),
  }
  }
@@ -53,33 +52,56 @@ impl Stream for Debounce {
  inner.event_detected.store(false, Ordering::Relaxed);
  Poll::Ready(Some(()))
  } else {
- let _ = inner.interval.poll_tick(cx);
- Poll::Pending
+ match inner.delay.poll_unpin(cx) {
+ Poll::Ready(_) => {
+ inner.delay = delay_for(DEBOUNCE_INTERVAL_MS);
+ Poll::Pending
+ }
+ Poll::Pending => Poll::Pending,
+ }
  }
  }
 }
 
-async fn error_handler(watch_future: WatchFuture) {
+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(),);
  eprintln!("{}", msg);
  }
 }
 
-pub async fn watch_func<F>(
- paths: &[PathBuf],
- closure: F,
+/// This function adds watcher functionality to subcommands like `fmt` or `lint`.
+/// The difference from [`watch_func_with_module_resolution`] is that this doesn't depend on
+/// [`ModuleGraph`].
+///
+/// - `target_resolver` is used for resolving file paths to be watched at every restarting of the watcher. The
+/// return value of this closure will then be passed to `operation` as an argument.
+///
+/// - `operation` is the actual operation we want to run every time the watcher detects file
+/// changes. For example, in the case where we would like to apply `fmt`, then `operation` would
+/// have the logic for it like calling `format_source_files`.
+///
+/// - `job_name` is just used for printing watcher status to terminal.
+///
+/// Note that the watcher will stop working if `target_resolver` fails at some point.
+///
+/// [`ModuleGraph`]: crate::module_graph::Graph
+pub async fn watch_func<F, G>(
+ target_resolver: F,
+ operation: G,
+ job_name: &str,
 ) -> Result<(), AnyError>
 where
- F: Fn() -> WatchFuture,
+ F: Fn() -> Result<Vec<PathBuf>, AnyError>,
+ G: Fn(Vec<PathBuf>) -> FileWatcherFuture<()>,
 {
  let mut debounce = Debounce::new();
- // This binding is required for the watcher to work properly without being dropped.
- let _watcher = new_watcher(paths, &debounce)?;
 
  loop {
- let func = error_handler(closure());
+ let paths = target_resolver()?;
+ let _watcher = new_watcher(&paths, &debounce)?;
+ let func = error_handler(operation(paths));
  let mut is_file_changed = false;
  select! {
  _ = debounce.next() => {
@@ -90,11 +112,95 @@ where
  );
  },
  _ = 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"),
+ );
+ }
+ }
+}
+
+/// This function adds watcher functionality to subcommands like `run` or `bundle`.
+/// The difference from [`watch_func`] is that this does depend on [`ModuleGraph`].
+///
+/// - `module_resolver` is used for both resolving file paths to be watched at every restarting
+/// of the watcher and building [`ModuleGraph`] or [`ModuleSpecifier`] which will then be passed
+/// to `operation`.
+///
+/// - `operation` is the actual operation we want to run every time the watcher detects file
+/// changes. For example, in the case where we would like to bundle, then `operation` would
+/// have the logic for it like doing bundle with the help of [`ModuleGraph`].
+///
+/// - `job_name` is just used for printing watcher status to terminal.
+///
+/// Note that the watcher will try to continue watching files using the previously resolved
+/// data if `module_resolver` fails at some point, which means the watcher won't work at all
+/// if `module_resolver` fails at the first attempt.
+///
+/// [`ModuleGraph`]: crate::module_graph::Graph
+/// [`ModuleSpecifier`]: deno_core::ModuleSpecifier
+pub async fn watch_func_with_module_resolution<F, G, T>(
+ module_resolver: F,
+ operation: G,
+ job_name: &str,
+) -> Result<(), AnyError>
+where
+ F: Fn() -> FileWatcherFuture<(Vec<PathBuf>, T)>,
+ 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 module = None;
+
+ loop {
+ match module_resolver().await {
+ Ok((next_paths, next_module)) => {
+ paths = Some(next_paths);
+ module = Some(next_module);
+ }
+ 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);
+ }
+ }
  }
+ // 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(&cur_paths, &debounce)?;
+ let func = error_handler(operation(cur_module));
+ 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!(
- "{} Process terminated! Restarting on file change...",
+ "{} {} finished! Restarting on file change...",
  colors::intense_blue("Watcher"),
+ job_name,
  );
  debounce.next().await;
  info!(
@@ -125,7 +231,8 @@ fn new_watcher(
  watcher.configure(Config::PreciseEvents(true)).unwrap();
 
  for path in paths {
- watcher.watch(path, RecursiveMode::NonRecursive)?;
+ // Ignore any error e.g. `PathNotFound`
+ let _ = watcher.watch(path, RecursiveMode::NonRecursive);
  }
 
  Ok(watcher)

cli/flags.rs

@@ -361,6 +361,7 @@ fn types_parse(flags: &mut Flags, _matches: &clap::ArgMatches) {
 }
 
 fn fmt_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
+ flags.watch = matches.is_present("watch");
  let files = match matches.values_of("files") {
  Some(f) => f.map(PathBuf::from).collect(),
  None => vec![],
@@ -418,6 +419,8 @@ fn bundle_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
  None
  };
 
+ flags.watch = matches.is_present("watch");
+
  flags.subcommand = DenoSubcommand::Bundle {
  source_file,
  out_file,
@@ -723,6 +726,7 @@ Ignore formatting a file by adding an ignore comment at the top of the file:
  .multiple(true)
  .required(false),
  )
+ .arg(watch_arg())
 }
 
 fn repl_subcommand<'a, 'b>() -> App<'a, 'b> {
@@ -793,6 +797,7 @@ fn bundle_subcommand<'a, 'b>() -> App<'a, 'b> {
  .required(true),
  )
  .arg(Arg::with_name("out_file").takes_value(true).required(false))
+ .arg(watch_arg())
  .about("Bundle module and dependencies into single file")
  .long_about(
  "Output a single JavaScript file with all dependencies.
@@ -1855,6 +1860,44 @@ mod tests {
  ..Flags::default()
  }
  );
+
+ let r = flags_from_vec_safe(svec!["deno", "fmt", "--watch", "--unstable"]);
+ assert_eq!(
+ r.unwrap(),
+ Flags {
+ subcommand: DenoSubcommand::Fmt {
+ ignore: vec![],
+ check: false,
+ files: vec![],
+ },
+ watch: true,
+ unstable: true,
+ ..Flags::default()
+ }
+ );
+
+ let r = flags_from_vec_safe(svec![
+ "deno",
+ "fmt",
+ "--check",
+ "--watch",
+ "--unstable",
+ "foo.ts",
+ "--ignore=bar.js"
+ ]);
+ assert_eq!(
+ r.unwrap(),
+ Flags {
+ subcommand: DenoSubcommand::Fmt {
+ ignore: vec![PathBuf::from("bar.js")],
+ check: true,
+ files: vec![PathBuf::from("foo.ts")],
+ },
+ watch: true,
+ unstable: true,
+ ..Flags::default()
+ }
+ );
  }
 
  #[test]
@@ -2405,6 +2448,29 @@ mod tests {
  );
  }
 
+ #[test]
+ fn bundle_watch() {
+ let r = flags_from_vec_safe(svec![
+ "deno",
+ "bundle",
+ "--watch",
+ "--unstable",
+ "source.ts"
+ ]);
+ assert_eq!(
+ r.unwrap(),
+ Flags {
+ subcommand: DenoSubcommand::Bundle {
+ source_file: "source.ts".to_string(),
+ out_file: None,
+ },
+ watch: true,
+ unstable: true,
+ ..Flags::default()
+ }
+ )
+ }
+
  #[test]
  fn run_import_map() {
  let r = flags_from_vec_safe(svec![

cli/fs_util.rs

@@ -88,8 +88,8 @@ pub fn is_supported_ext(path: &Path) -> bool {
 /// Collects file paths that satisfy the given predicate, by recursively walking `files`.
 /// If the walker visits a path that is listed in `ignore`, it skips descending into the directory.
 pub fn collect_files<P>(
- files: Vec<PathBuf>,
- ignore: Vec<PathBuf>,
+ files: &[PathBuf],
+ ignore: &[PathBuf],
  predicate: P,
 ) -> Result<Vec<PathBuf>, AnyError>
 where
@@ -99,15 +99,12 @@ where
 
  // retain only the paths which exist and ignore the rest
  let canonicalized_ignore: Vec<PathBuf> = ignore
- .into_iter()
+ .iter()
  .filter_map(|i| i.canonicalize().ok())
  .collect();
 
- let files = if files.is_empty() {
- vec![std::env::current_dir()?]
- } else {
- files
- };
+ let cur_dir = [std::env::current_dir()?];
+ let files = if files.is_empty() { &cur_dir } else { files };
 
  for file in files {
  for entry in WalkDir::new(file)
@@ -232,15 +229,14 @@ mod tests {
  let ignore_dir_files = ["g.d.ts", ".gitignore"];
  create_files(&ignore_dir_path, &ignore_dir_files);
 
- let result =
- collect_files(vec![root_dir_path], vec![ignore_dir_path], |path| {
- // exclude dotfiles
- path
- .file_name()
- .and_then(|f| f.to_str())
- .map_or(false, |f| !f.starts_with('.'))
- })
- .unwrap();
+ let result = collect_files(&[root_dir_path], &[ignore_dir_path], |path| {
+ // exclude dotfiles
+ path
+ .file_name()
+ .and_then(|f| f.to_str())
+ .map_or(false, |f| !f.starts_with('.'))
+ })
+ .unwrap();
  let expected = [
  "a.ts",
  "b.js",