From 707dcbf7600e5668ee03cdf208a0431c6e651f42 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Thu, 24 Sep 2026 18:30:03 +0100 Subject: [PATCH] metro-file-map: Recrawl a directory after an unnamed change in FallbackWatcher Summary: On Windows, libuv reports a change with no filename when changes to a watched directory overflow the 4KB buffer they're read into ([`fs-event.c`](https://github.com/libuv/libuv/blob/9be264a4efa515dcb6ced36267dac7a03f308be3/src/win/fs-event.c#L460-L592)). That's easy to hit when a package manager writes a new package into `node_modules`, and when it happens we have no idea which entries changed, or how many. `FallbackWatcher` handled these with `sane`'s `#detectChangedFile` heuristic: `lstat` every file registered in the directory and report either the first one that's missing or the most recently modified one. So at most one change was reported, any others were missed until restart, and for a directory with no registered files yet (like a newly created package directory) the event was dropped entirely. This diff replaces that heuristic. On an unnamed change, `FallbackWatcher` lists the directory to watch and register anything new, so later changes under it are reported, then emits a `recrawl` event for it. `FileMap` already handles `recrawl` (`NativeWatcher` emits it for directory renames) by crawling that subtree and applying the difference, so additions, modifications and deletions are all picked up. `recrawl` goes through the usual debounce, so a burst of overflows costs one crawl. Follows https://github.com/expo/expo/pull/49363, which lists a new directory on an unnamed change in Expo's fork of `metro-file-map`. That covers the new-directory case only. Changelog: ``` - **[Fix]**: Fix the fallback (Linux/Windows) watcher missing changes when Windows overflows its change buffer for a directory ``` Test plan: Two new tests in `FallbackWatcher-test.js` call the `fs.watch` listener for a directory with no filename, as libuv does on overflow, with that directory's (and the root's) real events withheld. On `main` neither the `recrawl` nor the watch on a new subdirectory happens; both pass with this diff. Run on macOS only - I haven't reproduced a real overflow on Windows. ``` yarn jest packages/metro-file-map yarn flow check yarn eslint packages/metro-file-map/src/watchers ``` --- .../src/watchers/FallbackWatcher.js | 80 +++++++------------ .../__tests__/FallbackWatcher-test.js | 67 +++++++++++++++- 2 files changed, 96 insertions(+), 51 deletions(-) diff --git a/packages/metro-file-map/src/watchers/FallbackWatcher.js b/packages/metro-file-map/src/watchers/FallbackWatcher.js index 602a8e65aa..19b5190482 100644 --- a/packages/metro-file-map/src/watchers/FallbackWatcher.js +++ b/packages/metro-file-map/src/watchers/FallbackWatcher.js @@ -31,6 +31,7 @@ const fsPromises = fs.promises; const TOUCH_EVENT = common.TOUCH_EVENT; const DELETE_EVENT = common.DELETE_EVENT; +const RECRAWL_EVENT = common.RECRAWL_EVENT; /** * This setting delays all events. It suppresses 'change' events that @@ -236,60 +237,12 @@ export default class FallbackWatcher extends AbstractWatcher { await Promise.all(promises); } - /** - * On some platforms, as pointed out on the fs docs (most likely just win32) - * the file argument might be missing from the fs event. Try to detect what - * change by detecting if something was deleted or the most recent file change. - */ - #detectChangedFile( - dir: string, - event: string, - callback: (file: string) => void, - ) { - if (!this.#dirRegistry[dir]) { - return; - } - - let found = false; - let closest: ?Readonly<{file: string, mtime: Stats['mtime']}> = null; - let c = 0; - Object.keys(this.#dirRegistry[dir]).forEach((file, i, arr) => { - fs.lstat(path.join(dir, file), (error, stat) => { - if (found) { - return; - } - - if (error) { - if (isIgnorableFileError(error)) { - found = true; - callback(file); - } else { - this.emitError(error); - } - } else { - if (closest == null || stat.mtime > closest.mtime) { - closest = {file, mtime: stat.mtime}; - } - if (arr.length === ++c) { - callback(closest.file); - } - } - }); - }); - } - /** * Normalize fs events and pass it on to be processed. */ #normalizeChange(dir: string, event: string, file: string) { if (!file) { - this.#detectChangedFile(dir, event, actualFile => { - if (actualFile) { - this.#processChange(dir, event, actualFile).catch(error => - this.emitError(error), - ); - } - }); + this.#processUnnamedChange(dir).catch(error => this.emitError(error)); } else { this.#processChange(dir, event, path.normalize(file)).catch(error => this.emitError(error), @@ -297,6 +250,35 @@ export default class FallbackWatcher extends AbstractWatcher { } } + /** + * Process an event that doesn't name the changed entry. Windows sends these + * when changes to a directory overflow the buffer they're reported through, + * so any number of entries under `dir` may have been added, changed or + * removed. + */ + async #processUnnamedChange(dir: string) { + // Watch and register anything new, so that later changes are reported. + await recReaddir( + dir, + subdir => { + this.#watchdir(subdir); + }, + filename => { + this.#register(filename, 'f'); + }, + symlink => { + this.#register(symlink, 'l'); + }, + this.#checkedEmitError, + this.ignored, + ); + // Then have the file map reconcile everything under `dir`. + this.#emitEvent({ + event: RECRAWL_EVENT, + relativePath: path.relative(this.root, dir), + }); + } + /** * Process changes. */ diff --git a/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js b/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js index 70e74e6382..d87a0b536e 100644 --- a/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js @@ -9,6 +9,8 @@ * @oncall react_native */ +import type {WatcherBackendChangeEvent} from '../../flow-types'; + import FallbackWatcher from '../FallbackWatcher'; import {createTempWatchRoot} from './helpers'; import EventEmitter from 'node:events'; @@ -32,6 +34,10 @@ describe('FallbackWatcher', () => { let calls: Array; let watchFailure: ?{code: string, path: string}; let watchOverride: ?{path: string, watcher: ErroredFSWatcher}; + // The listener passed to `fs.watch` for each directory, and the directories + // whose events are withheld from it. + let listeners: Map void>; + let mutedDirs: Set; const indexOfCall = (op: 'watch' | 'readdir', dir: string) => calls.indexOf(`${op}:${dir}`); @@ -46,9 +52,11 @@ describe('FallbackWatcher', () => { calls = []; watchFailure = null; watchOverride = null; + listeners = new Map(); + mutedDirs = new Set(); const {watch} = fs; - jest.spyOn(fs, 'watch').mockImplementation((dir, ...args) => { + jest.spyOn(fs, 'watch').mockImplementation((dir, options, listener) => { calls.push(`watch:${String(dir)}`); const override = watchOverride; if (override != null && dir === override.path) { @@ -63,7 +71,12 @@ describe('FallbackWatcher', () => { error.code = failure.code; throw error; } - return watch(dir, ...args); + listeners.set(String(dir), listener); + return watch(dir, options, (event, filename) => { + if (!mutedDirs.has(String(dir))) { + listener(event, filename); + } + }); }); const {readdir} = fs.promises; // $FlowFixMe[incompatible-call] - variadic passthrough @@ -175,6 +188,56 @@ describe('FallbackWatcher', () => { expectWatchedBeforeListed(join(watchRoot, 'a')); }); }); + + // Windows reports a change with no filename when changes to a directory + // overflow its buffer, so any number of entries under it may have changed. + describe('when an event does not name the changed entry', () => { + const emitUnnamedChange = (dir: string) => { + const listener = listeners.get(dir); + if (listener == null) { + throw new Error(`Not watching ${dir}`); + } + listener('change', null); + }; + + beforeEach(async () => { + await mkdir(join(watchRoot, 'a')); + await writeFile(join(watchRoot, 'a', 'existing.js'), ''); + await watcher?.startWatching(); + // Only the unnamed change reports anything, and on macOS the root's + // watcher also sees changes in subdirectories. + mutedDirs.add(watchRoot); + mutedDirs.add(join(watchRoot, 'a')); + calls = []; + }); + + test('requests a recrawl of the directory', async () => { + const events: Array = []; + watcher?.onFileEvent(event => { + events.push(event); + }); + await writeFile(join(watchRoot, 'a', 'new.js'), ''); + await rm(join(watchRoot, 'a', 'existing.js')); + + emitUnnamedChange(join(watchRoot, 'a')); + + await waitFor(() => events.some(event => event.event === 'recrawl')); + expect(events).toEqual([ + {event: 'recrawl', relativePath: 'a', root: watchRoot}, + ]); + }); + + test('watches a new directory under it', async () => { + await mkdir(join(watchRoot, 'a', 'b')); + + emitUnnamedChange(join(watchRoot, 'a')); + + await waitFor( + () => indexOfCall('readdir', join(watchRoot, 'a', 'b')) >= 0, + ); + expectWatchedBeforeListed(join(watchRoot, 'a', 'b')); + }); + }); }); function fsError(code: string, path: string): Error {