Improve benchmark telemetry and dev-server watching

crates/webui-dev-server/src/watch.rs

@@ -16,17 +16,65 @@ use std::sync::Arc;
 use std::time::Duration;
 
 use anyhow::{Context, Result};
-use notify::RecommendedWatcher;
-use notify::RecursiveMode;
-use notify_debouncer_mini::{new_debouncer, DebounceEventResult, Debouncer};
+use notify::{
+    event::{AccessKind, AccessMode},
+    Event, EventHandler, EventKind, RecommendedWatcher, RecursiveMode, Watcher, WatcherKind,
+};
+use notify_debouncer_mini::{
+    new_debouncer_opt, Config as DebouncerConfig, DebounceEventResult, Debouncer,
+};
 
 /// Owns the watcher background thread. Drop to stop watching.
 ///
 /// `notify-debouncer-mini` spawns its background thread inside the
 /// debouncer struct; dropping the struct kills the thread. Consumers
 /// should hold this handle for the lifetime of their server.
 pub struct WatcherHandle {
-    _debouncer: Debouncer<RecommendedWatcher>,
+    _debouncer: Debouncer<ContentChangeWatcher>,
+}
+
+struct ContentChangeWatcher(RecommendedWatcher);
+
+impl Watcher for ContentChangeWatcher {
+    fn new<F: EventHandler>(mut event_handler: F, config: notify::Config) -> notify::Result<Self> {
+        RecommendedWatcher::new(
+            move |event| {
+                if matches!(&event, Ok(event) if is_read_only_access_event(event)) {
+                    return;
+                }
+                event_handler.handle_event(event);
+            },
+            config,
+        )
+        .map(Self)
+    }
+
+    fn watch(&mut self, path: &Path, recursive_mode: RecursiveMode) -> notify::Result<()> {
+        self.0.watch(path, recursive_mode)
+    }
+
+    fn unwatch(&mut self, path: &Path) -> notify::Result<()> {
+        self.0.unwatch(path)
+    }
+
+    fn configure(&mut self, config: notify::Config) -> notify::Result<bool> {
+        self.0.configure(config)
+    }
+
+    fn kind() -> WatcherKind {
+        RecommendedWatcher::kind()
+    }
+}
+
+fn is_read_only_access_event(event: &Event) -> bool {
+    matches!(
+        event.kind,
+        EventKind::Access(
+            AccessKind::Read
+                | AccessKind::Open(AccessMode::Read)
+                | AccessKind::Close(AccessMode::Read)
+        )
+    )
 }
 
 /// Configuration for [`spawn_watcher`].
@@ -100,39 +148,52 @@ where
     let mut content_hashes: HashMap<PathBuf, u64> = HashMap::new();
     let retry_unchanged_when = cfg.retry_unchanged_when.clone();
     let explicit_filter = explicit_files.clone();
-    let mut debouncer = new_debouncer(cfg.debounce, move |res: DebounceEventResult| match res {
-        Ok(events) => {
-            // Filter out ignored paths and dedupe (notify can emit
-            // duplicate events for the same path within a window).
-            let mut paths: Vec<PathBuf> = Vec::with_capacity(events.len());
-            let mut seen: HashSet<PathBuf> = HashSet::with_capacity(events.len());
-            for e in events {
-                if should_ignore_event(&e.path, &ignore, &explicit_filter) {
-                    continue;
+    let notify_config = notify::Config::default().with_follow_symlinks(false);
+    let debouncer_config = DebouncerConfig::default()
+        .with_timeout(cfg.debounce)
+        .with_notify_config(notify_config);
+    let mut debouncer = new_debouncer_opt::<_, ContentChangeWatcher>(
+        debouncer_config,
+        move |res: DebounceEventResult| match res {
+            Ok(events) => {
+                // Filter out ignored paths and dedupe (notify can emit
+                // duplicate events for the same path within a window).
+                let mut paths: Vec<PathBuf> = Vec::with_capacity(events.len());
+                let mut seen: HashSet<PathBuf> = HashSet::with_capacity(events.len());
+                for e in events {
+                    if should_ignore_event(&e.path, &ignore, &explicit_filter) {
+                        continue;
+                    }
+                    if seen.insert(e.path.clone()) {
+                        paths.push(e.path);
+                    }
                 }
-                if seen.insert(e.path.clone()) {
-                    paths.push(e.path);
+                // Recursive backends emit directory metadata events alongside
+                // file writes. Existing directories are not source changes and
+                // would otherwise retrigger a rebuild after every build; keep
+                // missing paths so directory/file deletions still rebuild.
+                paths.retain(|path| !path.is_dir());
+                // Drop paths whose content is byte-identical to the last event.
+                // Editors fire a write on every Ctrl+S even when nothing changed;
+                // rebuilding then is pure wasted work, so a no-op save triggers no
+                // rebuild at all in the clean state. When the caller reports that a
+                // rebuild error is active, unchanged events are allowed through so a
+                // no-op save can retry transient failures. Deletions and oversized
+                // files always count as changed (see `content_changed`).
+                let retry_unchanged = retry_unchanged_when
+                    .as_ref()
+                    .is_some_and(|predicate| predicate());
+                paths
+                    .retain(|path| should_forward_path(&mut content_hashes, path, retry_unchanged));
+                if !paths.is_empty() {
+                    on_event(paths);
                 }
             }
-            // Drop paths whose content is byte-identical to the last event.
-            // Editors fire a write on every Ctrl+S even when nothing changed;
-            // rebuilding then is pure wasted work, so a no-op save triggers no
-            // rebuild at all in the clean state. When the caller reports that a
-            // rebuild error is active, unchanged events are allowed through so a
-            // no-op save can retry transient failures. Deletions and oversized
-            // files always count as changed (see `content_changed`).
-            let retry_unchanged = retry_unchanged_when
-                .as_ref()
-                .is_some_and(|predicate| predicate());
-            paths.retain(|path| should_forward_path(&mut content_hashes, path, retry_unchanged));
-            if !paths.is_empty() {
-                on_event(paths);
+            Err(e) => {
+                eprintln!("watcher error: {e:?}");
             }
-        }
-        Err(e) => {
-            eprintln!("watcher error: {e:?}");
-        }
-    })
+        },
+    )
     .context("Cannot start file watcher")?;
 
     let mut watched_roots = Vec::with_capacity(cfg.paths.len());
@@ -221,7 +282,12 @@ fn is_ignored(event_path: &Path, ignore: &[PathBuf]) -> bool {
         let only_one_component = components.next().is_none();
         if let (Some(first), true) = (first, only_one_component) {
             let name = first.as_os_str();
-            if candidate.components().any(|c| c.as_os_str() == name) {
+            // Check the lexical event path before its canonical form. pnpm
+            // dependencies are symlinks, so canonicalization removes the
+            // `node_modules` component that identifies the ignored subtree.
+            if event_path.components().any(|c| c.as_os_str() == name)
+                || candidate.components().any(|c| c.as_os_str() == name)
+            {
                 return true;
             }
         } else if candidate.starts_with(root) {
@@ -369,6 +435,54 @@ mod tests {
         assert!(!is_ignored(Path::new("/repo/src/index.ts"), &ignore));
     }
 
+    #[test]
+    fn read_only_access_events_are_not_content_changes() {
+        use notify::event::ModifyKind;
+
+        for kind in [
+            AccessKind::Read,
+            AccessKind::Open(AccessMode::Read),
+            AccessKind::Close(AccessMode::Read),
+        ] {
+            assert!(is_read_only_access_event(&Event::new(EventKind::Access(
+                kind
+            ))));
+        }
+        for kind in [
+            AccessKind::Any,
+            AccessKind::Open(AccessMode::Write),
+            AccessKind::Close(AccessMode::Write),
+        ] {
+            assert!(!is_read_only_access_event(&Event::new(EventKind::Access(
+                kind
+            ))));
+        }
+        assert!(!is_read_only_access_event(&Event::new(EventKind::Modify(
+            ModifyKind::Any
+        ))));
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn is_ignored_matches_node_modules_symlink_path() {
+        use std::os::unix::fs::symlink;
+
+        let dir = tempfile::tempdir().unwrap();
+        let package = dir.path().join("packages/example");
+        let node_modules = dir.path().join("node_modules");
+        std::fs::create_dir_all(&package).unwrap();
+        std::fs::create_dir(&node_modules).unwrap();
+        std::fs::write(package.join("index.js"), "export {};").unwrap();
+        symlink(&package, node_modules.join("example")).unwrap();
+
+        let linked_file = node_modules.join("example/index.js");
+        let canonical = std::fs::canonicalize(&linked_file).unwrap();
+        assert!(!canonical
+            .components()
+            .any(|component| component.as_os_str() == "node_modules"));
+        assert!(is_ignored(&linked_file, &[PathBuf::from("node_modules")]));
+    }
+
     #[test]
     fn content_changed_skips_identical_saves() {
         let dir = tempfile::tempdir().unwrap();