fix: follow symlinks discovered inside watched real directories (#190)#291
Merged
alexander-akait merged 11 commits intomainfrom Apr 27, 2026
Merged
fix: follow symlinks discovered inside watched real directories (#190)#291alexander-akait merged 11 commits intomainfrom
alexander-akait merged 11 commits intomainfrom
Conversation
When followSymlinks was enabled, symlinks nested inside a watched real directory were never resolved, so changes to their targets were missed. The DirectoryWatcher now follows symlinks during scan when nestedWatching is on, recording symlinked subdirectories as nested watchers and attaching a file watcher on the resolved real path of symlinked files so target changes propagate to the symlink path.
|
Merging this PR will degrade performance by 77.14%
Performance Changes
Comparing 
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #291 +/- ##
==========================================
+ Coverage 90.76% 95.82% +5.06%
==========================================
Files 7 7
Lines 1115 1173 +58
Branches 312 342 +30
==========================================
+ Hits 1012 1124 +112
+ Misses 91 44 -47
+ Partials 12 5 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Inline the symlink target watcher setup directly into the scan loop, drop the separate watchSymlinkTarget public method, and rename the tracking map to the internal _symlinkTargetWatchers convention. Order the followSymlinks check so the option flag short-circuits before any syscall-level symlink test.
Replace fs.realpathSync with async fs.realpath, chained with fs.stat on the resolved path. Both the symlink-follow and plain branches funnel through a single apply() closure that handles directory vs file recording and optional file-watcher attachment for targets outside the scanned directory.
When close() runs, it calls close() on each symlink target watcher, which synchronously removes it from the target DirectoryWatcher's watchers map — no further change events can reach us, matching the pattern used by createNestedWatcher.
When the symlink target already has an entry in its parent DirectoryWatcher, attaching our extra watcher triggers a 'watch (outdated on attach)' emit with initial=true. We already captured the file's state in the scan's apply(), so replaying it only risks leaking timing-dependent phantom events up to the user when FS_ACCURACY pushes safeTime past their startTime. Skip initial events in the handler and only forward real changes.
Recording the target's mtime for a followed symlink caused change detection to miss cases where the symlink itself is replaced with a different target that shares the same mtime (e.g. two files created together in a test fixture). The symlink's own lstat mtime changes on unlink+relink, so use it for the file entry and keep the out-of-directory target watcher for content-level changes.
Creating the extra fixture files inside the test left them younger than FS_ACCURACY (up to 2000 ms) when watch() started. The initial scan's computed safeTime (mtime + FS_ACCURACY) could then exceed the user's startTime, making checkStartTime fire the replay up to the aggregated event before the test set active = true. Tick 2500 ms after creating the fixtures so they are safely older than FS_ACCURACY on slow filesystems too (seen on Node 10 polling CI).
Using startTime=1 made every watcher.checkStartTime(safeTime, true) succeed, so the initial scan of the target's parent DirectoryWatcher would fire a change event on the symlink target watcher. The 2-arg emit from setFileTime doesn't carry the initial flag, so the handler couldn't distinguish a genuine change from the initial scan replay and propagated it to the user. Using Date.now() lets the library's built-in checkStartTime gate suppress replays for files that pre-date the attachment time, matching how the user's own directory watcher suppresses them.
fs.realpath on Windows can return the resolved path with different casing than the watched directory path, which broke the 'target lives in the same directory' check and attached unnecessary cross-directory watchers. Use withoutCase (lowercase normalization) like the existing filesWithoutCase map does to make the check filesystem-case-aware. Also apply prettier formatting.
The dir-symlink handling created a nested DirectoryWatcher on the symlink path. When the symlink was replaced with a target of a different kind (dir -> file via unlink+symlinkDir in the existing 'should detect a symlink dir change in a watched symlinked directory' test) the directories map carried a stale entry that setFileTime didn't clean up, which was tolerated on Linux/macOS timing but hung on Windows polling. Issue #190 is about symlinks to files not being followed inside a watched real directory; dir-symlink content tracking is left on the original lstat-based path (the link's own mtime is tracked, content of the target dir is not propagated). Drop the now-redundant subdirectory test that relied on the removed behavior.
A single new Watchpack(opts).close() is ~100 µs, and CodSpeed simulation instruments exactly one iteration per task — which was too short to measure stably and produced cross-environment swings of up to 28% even when the construction path wasn't touched. Loop 100 constructions per bench body so each instrumented run covers ~10 ms, amortizing GC/JIT/timer-resolution noise across the batch.
alexander-akait
pushed a commit
that referenced
this pull request
Apr 27, 2026
Add explicit tests pinning the symlink-path semantics established by PRs #291/#296 against the original bug report: - a file modified through a symlinked directory is reported with the symlink path (e.g. `a/b/ext_dir_link/inner`), not the resolved target path (`ext_dir/inner`). - the user-supplied `ignored` callback is queried with the symlink path, so an allowlist scoped to the watched subtree continues to match files that physically live outside it. These assertions go beyond the existing `expect([...changes]).toEqual` checks (which only verify the aggregated directory) and lock down the per-file path that consumers like webpack actually rely on. Refs #231 https://claude.ai/code/session_016tnQoHN9qHz9PRZo312iHs
alexander-akait
pushed a commit
that referenced
this pull request
Apr 27, 2026
The #190 / #231 fixes (PRs #291, #296) made DirectoryWatcher descend into symlinked directories whose realpath lives outside the watched real directory. The existing short-circuit only catches symlinks that point to a sibling in the same parent (`dirname(realPath) === this.path`); it does not catch the case where the target is an ancestor of the symlink itself, e.g. `a/b/loop -> ..`. Without protection, `readdir` follows the symlink, finds the original tree again, and a new DirectoryWatcher is created at every recursion level (locally observed: ~1500 watchers within 2 s, ~2500 within 2.5 s) until paths hit PATH_MAX. Compute `path.relative(realPath, itemPath)` before descending: when the relative path doesn't go up (no `..`, not absolute), the symlink target is at-or-above the symlink itself and would create a cycle. In that case, treat the symlink as a plain entry instead of descending. A regression test (`should not recurse infinitely when a symlinked directory points to one of its ancestors`) creates `a/b/cycle -> ..` and asserts the WatcherManager's `directoryWatchers` size stays under 10 — without the guard the test fails with ~1580 watchers. Also adds a changeset entry documenting the fix. Refs #231 https://claude.ai/code/session_016tnQoHN9qHz9PRZo312iHs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When followSymlinks was enabled, symlinks nested inside a watched real
directory were never resolved, so changes to their targets were missed.
The DirectoryWatcher now follows symlinks during scan when
nestedWatching is on, recording symlinked subdirectories as nested
watchers and attaching a file watcher on the resolved real path of
symlinked files so target changes propagate to the symlink path.