From 038ba6b7a711afa5c276627de101e1f1ca52e136 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 7 Sep 2026 16:08:43 +0100 Subject: [PATCH] metro-file-map: Watch directories before listing them Summary: `FallbackWatcher` starts an `fs.watch` on each directory from `recReaddir`'s `dirCallback`, and the crawl calls that after `readdir` has returned. Anything written into a directory *between the two* appears in neither the listing nor the watch, so it stays invisible until the next full crawl. There's no recovery on Linux or Windows. This is https://github.com/expo/expo/issues/48950 - `npx expo install` against a running dev server leaves the new modules unresolvable until restart, which is a lot more painful than it sounds now that agents routinely run Metro in a VM. @brentvatne diagnosed it in https://github.com/expo/expo/pull/49363, with a fix in Expo's fork that we can hopefully replace with this one. Now that we own the crawl (#1906), the fix is very simply to call `dirCallback` before `readdir` instead of after, which also moves it inside `recReaddir`'s existing `try`. Separate issues not fixed here, both pre-existing and both covered by https://github.com/expo/expo/pull/49363 : - An `fs.watch` that emits `error` is never removed from `#watched`. Node emits no `close` after `error`, so the path can never be re-watched, and `#stopWatching` waits on a `close` that will not arrive. - On win32, `fs.watch` can report an event with no filename. `#detectChangedFile` drops it when `#dirRegistry[dir]` is empty, which is exactly the state a newly watched directory is in. Changelog: ``` - **[Fix]**: `FallbackWatcher` no longer misses files written to a directory while it is being crawled ``` Test Plan: ``` yarn jest packages/metro-file-map yarn flow check yarn lint ``` New `watchers/__tests__/FallbackWatcher-test.js` asserts the ordering directly, that `fs.watch` precedes `readdir` for the same directory, on the initial crawl and on a directory created while watching, plus that a directory whose `fs.watch` throws is skipped without failing the crawl. All three fail on the parent commit and pass here. The race can't be asserted behaviourally on macOS, because FSEvents delivers with a latency window and a watch started immediately after a write still reports it: ``` $ node -e "fs.writeFileSync(d+'/raced.js', ''); fs.watch(d, (e, f) => console.log(e, f))" rename raced.js ``` That's why the issue is Linux/Windows only, and why the assertion is on the ordering rather than on a missed event. Metro has no Windows coverage for this backend, so the win32 path is unexercised either way. --- .../src/watchers/FallbackWatcher.js | 18 ++- .../__tests__/FallbackWatcher-test.js | 137 ++++++++++++++++++ 2 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js diff --git a/packages/metro-file-map/src/watchers/FallbackWatcher.js b/packages/metro-file-map/src/watchers/FallbackWatcher.js index 3f30e9be58..292dbda929 100644 --- a/packages/metro-file-map/src/watchers/FallbackWatcher.js +++ b/packages/metro-file-map/src/watchers/FallbackWatcher.js @@ -182,9 +182,17 @@ export default class FallbackWatcher extends AbstractWatcher { if (this.#watched[dir]) { return false; } - const watcher = fs.watch(dir, {persistent: true}, (event, filename) => - this.#normalizeChange(dir, event, filename), - ); + let watcher: FSWatcher; + try { + watcher = fs.watch(dir, {persistent: true}, (event, filename) => + this.#normalizeChange(dir, event, filename), + ); + } catch (error) { + // A directory we cannot watch is still worth crawling, so report the + // error and carry on rather than losing the subtree under it. + this.#checkedEmitError(error); + return false; + } this.#watched[dir] = watcher; watcher.on('error', this.#checkedEmitError); @@ -463,12 +471,14 @@ async function recReaddir( if (ignored != null && common.posixPathMatchesPattern(ignored, entry)) { return; } + // Report the directory before listing it. A consumer that starts watching + // here would otherwise miss anything written between the two. + dirCallback(entry, stats); names = await fsPromises.readdir(entry); } catch (error) { errorCallback(error); return; } - dirCallback(entry, stats); for (const name of names) { pending.push(path.join(entry, name)); } diff --git a/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js b/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js new file mode 100644 index 0000000000..c23224c3a6 --- /dev/null +++ b/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js @@ -0,0 +1,137 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + * @oncall react_native + */ + +import FallbackWatcher from '../FallbackWatcher'; +import {createTempWatchRoot} from './helpers'; +import fs from 'node:fs'; +import {join} from 'node:path'; + +jest.useRealTimers(); +jest.setTimeout(10 * 1000); + +const {mkdir, rm, writeFile} = fs.promises; + +describe('FallbackWatcher', () => { + let watchRoot: string; + let watcher: ?FallbackWatcher; + let calls: Array; + let watchFailure: ?{code: string, path: string}; + + const indexOfCall = (op: 'watch' | 'readdir', dir: string) => + calls.indexOf(`${op}:${dir}`); + + const expectWatchedBeforeListed = (dir: string) => { + expect(indexOfCall('watch', dir)).toBeGreaterThanOrEqual(0); + expect(indexOfCall('watch', dir)).toBeLessThan(indexOfCall('readdir', dir)); + }; + + beforeEach(async () => { + watchRoot = await createTempWatchRoot('Fallback', false); + calls = []; + watchFailure = null; + + const {watch} = fs; + jest.spyOn(fs, 'watch').mockImplementation((dir, ...args) => { + calls.push(`watch:${String(dir)}`); + const failure = watchFailure; + if (failure != null && dir === failure.path) { + const error = new Error(`Cannot watch path '${String(dir)}'.`); + // $FlowFixMe[prop-missing] code + error.code = failure.code; + throw error; + } + return watch(dir, ...args); + }); + const {readdir} = fs.promises; + // $FlowFixMe[incompatible-call] - variadic passthrough + jest.spyOn(fs.promises, 'readdir').mockImplementation((dir, ...args) => { + calls.push(`readdir:${String(dir)}`); + return readdir(dir, ...args); + }); + + watcher = new FallbackWatcher(watchRoot, { + dot: true, + globs: [], + ignored: null, + watchmanDeferStates: [], + }); + }); + + afterEach(async () => { + await watcher?.stopWatching(); + jest.restoreAllMocks(); + await rm(watchRoot, {recursive: true}); + }); + + // A file written into a directory after it has been listed but before it is + // watched is reported by neither the listing nor any subsequent event, and is + // missed until the next full crawl. This is how installing a package against + // a running server loses files: https://github.com/expo/expo/issues/48950 + describe('watches each directory before listing it', () => { + test('during the initial crawl', async () => { + await mkdir(join(watchRoot, 'a', 'b'), {recursive: true}); + + await watcher?.startWatching(); + + for (const dir of ['', 'a', join('a', 'b')]) { + expectWatchedBeforeListed(join(watchRoot, dir)); + } + }); + + test('for a directory created while watching', async () => { + await watcher?.startWatching(); + calls = []; + + const nested = join(watchRoot, 'new', 'nested'); + await mkdir(nested, {recursive: true}); + await writeFile(join(nested, 'file.js'), ''); + await waitFor(() => indexOfCall('readdir', nested)>= 0); + + for (const dir of [join(watchRoot, 'new'), nested]) { + expectWatchedBeforeListed(dir); + } + }); + }); + + // A watch we cannot establish - one directory over the inotify limit, say - + // must not cost us the files under it, which would silently truncate the + // file map. + test.each([ + ['ENOSPC', 1], + ['ENOENT', 0], + ])( + 'crawls past a directory it cannot watch (%s)', + async (code, expectedErrors) => { + await mkdir(join(watchRoot, 'a', 'b'), {recursive: true}); + watchFailure = {code, path: join(watchRoot, 'a')}; + const errors: Array = []; + watcher?.onError(error => { + errors.push(error); + }); + + await expect(watcher?.startWatching()).resolves.toBeUndefined(); + + expect(errors).toHaveLength(expectedErrors); + for (const dir of ['a', join('a', 'b')]) { + expect( + indexOfCall('readdir', join(watchRoot, dir)), + ).toBeGreaterThanOrEqual(0); + } + }, + ); +}); + +async function waitFor(predicate: () => boolean): Promise { + const deadline = Date.now() + 5000; + while (!predicate() && Date.now() < deadline) { + await new Promise(resolve => setTimeout(resolve, 20)); + } +}

AltStyle によって変換されたページ (->オリジナル) /