From a9b512354d77abfa5825016fab0baf4bc74b94d4 Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Mon, 26 Oct 2026 11:58:14 +0000 Subject: [PATCH] metro-file-map: Watch directories as the node crawler lists them Without Watchman, `Watcher` starts its backends before the crawl, so that nothing changed during the crawl is missed: https://github.com/react/metro/blob/56efab302200568c919ac1b3c28dacf8d84fc692/packages/metro-file-map/src/Watcher.js#L118-L127 For `FallbackWatcher` that means walking the tree once to watch it and again to crawl it. This lets the crawl do both: the crawler hands each directory to the watcher just before listing it, the order the watcher's own walk uses. Directories the crawl skips as ignored are handed over too, for the watcher to walk unless it ignores them as well (eg `node_modules` without `retainAllFiles`). This is internal to the built-in node crawler. A `crawlerFactory` crawler, a recrawl and a root added with `addRoot` keep the watcher's own walk. ## Benchmark This takes another 28-30% off watch-mode startup on Linux and Windows. With the rest of this stack, watch-mode startup without Watchman goes from 1,896ms to 510ms (-73%) on Linux, and from 2,554ms to 522ms (-80%) on Windows. On macOS, which uses `NativeWatcher`, PRs 2-5 measure within noise of #2104. | | `main` | #2107 | This PR | vs `main` | vs #2107 | |---|---|---|---|---|---| | Linux: Watch-mode start, warm cache | 1,896ms | 707ms | 510ms | -73.1% (-73.4% to -72.8%) | -27.9% (-28.6% to -27.2%) | | Windows: Watch-mode start, warm cache | 2,554ms | 721ms | 522ms | -79.6% (-79.7% to -79.4%) | -27.7% (-28.0% to -27.2%) | | Linux: Watch-mode start, cold cache | 1,882ms | 699ms | 491ms | -73.9% (-74.5% to -73.5%) | -29.7% (-31.1% to -28.5%) | | Windows: Watch-mode start, cold cache | 2,547ms | 709ms | 507ms | -80.1% (-80.2% to -80.0%) | -28.5% (-28.9% to -27.9%) | | Linux: Heap after a watch-mode start | 31.8MB | 28.3MB | 28.3MB | -11.1% (-11.1% to -11.1%) | -0.1% (-0.1% to -0.1%) | | Windows: Heap after a watch-mode start | 31.9MB | 28.4MB | 28.4MB | -11.1% (-11.1% to -11.1%) | -0.1% (-0.1% to -0.1%) | | Linux: RSS after a watch-mode start | 197.6MB | 175.2MB | 174.3MB | -11.8% (-12.3% to -11.3%) | -0.5% (-0.9% to +0.0%) | | Windows: RSS after a watch-mode start | 206.1MB | 182.9MB | 183.9MB | -10.7% (-11.3% to -10.3%) | +0.6% (+0.4% to +0.8%) | ## Changelog ``` - **[Performance]**: metro-file-map: Set up file watching during the crawl instead of after it, on Linux and Windows without Watchman ``` ## Test plan New unit tests, and a `watch-integration-test.js` case for a directory the crawl skips but the watcher doesn't. Benchmark as in #2104. --- packages/metro-file-map/src/Watcher.js | 58 +++++++--- .../src/__tests__/Watcher-test.js | 101 ++++++++++++++++-- .../src/__tests__/addRoot-test.js | 9 ++ .../src/__tests__/index-test.js | 9 ++ .../src/__tests__/watch-integration-test.js | 13 +++ .../src/crawlers/__tests__/node-test.js | 31 ++++++ .../metro-file-map/src/crawlers/node/index.js | 49 +++++++-- .../src/watchers/FallbackWatcher.js | 62 ++++++++++- .../__tests__/FallbackWatcher-test.js | 42 ++++++++ .../metro-file-map/src/watchers/common.js | 3 + 10 files changed, 351 insertions(+), 26 deletions(-) diff --git a/packages/metro-file-map/src/Watcher.js b/packages/metro-file-map/src/Watcher.js index ddba02c2a9..18ab3bb3ed 100644 --- a/packages/metro-file-map/src/Watcher.js +++ b/packages/metro-file-map/src/Watcher.js @@ -8,6 +8,7 @@ * @format */ +import type {CrawlWatcher} from './crawlers/node'; import type { Console, Crawler, @@ -48,6 +49,8 @@ const LIVE_CHECK_TIMEOUT = 200; const MAX_LIVE_WAIT_TIME = 10000; type InternalCrawlOptions = Readonly<{ + // For a node crawl to report what it lists to, one for each root. + crawlWatchers?: ?ReadonlyArray, previousState: CrawlerOptions['previousState'], roots: ReadonlyArray, subpath?: string, @@ -130,11 +133,27 @@ export class Watcher extends EventEmitter { // the same window. If the Watchman crawl falls back to the node crawler, // there is no clock, so #crawl() starts non-Watchman backends before the // node crawl instead. + // + // The node crawler lists every directory under the roots, so a + // FallbackWatcher started for it can watch each just before it is listed, + // instead of walking the roots again. It watches the paths the crawl + // lists, so only for roots given normalized. + let crawlWatchers: ?ReadonlyArray = null; if (!options.useWatchman) { - await this.#startBackendsBeforeCrawl(); + const crawlFed = + options.crawl == null && + this.#getBackendClass() === FallbackWatcher && + options.roots.every(root => path.resolve(root) === root); + await this.#startBackendsBeforeCrawl({crawlFed}); + if (crawlFed) { + crawlWatchers = this.#backends.map(backend => + backend instanceof FallbackWatcher ? backend.getCrawlWatcher() : null, + ); + } } const result = await this.#crawl({ + crawlWatchers, previousState: options.previousState, roots: options.roots, useWatchman: options.useWatchman, @@ -144,11 +163,13 @@ export class Watcher extends EventEmitter { return result; } - async #startBackendsBeforeCrawl(): Promise { + async #startBackendsBeforeCrawl( + opts?: Readonly<{crawlFed?: boolean}>, + ): Promise { if (!this.#options.watch) { return; } - await this.#startBackends(); + await this.#startBackends(undefined, opts?.crawlFed === true); // end() may have closed us before the backends were set - don't leak them. if (this.#options.abortSignal.aborted) { await this.close(); @@ -172,7 +193,7 @@ export class Watcher extends EventEmitter { async #crawl(crawlOptions: InternalCrawlOptions): Promise { const options = this.#options; - const {useWatchman, subpath} = crawlOptions; + const {crawlWatchers, useWatchman, subpath} = crawlOptions; const ignoreForCrawl = (filePath: string) => options.ignoreForCrawl(filePath) || @@ -208,7 +229,11 @@ export class Watcher extends EventEmitter { let delta: CrawlResult; try { - delta = await crawl(crawlerOptions); + // Crawl watchers are only given for the node crawler. + delta = + crawlWatchers != null + ? await nodeCrawl(crawlerOptions, crawlWatchers) + : await crawl(crawlerOptions); } catch (firstError) { if (crawl !== watchmanCrawl) { throw firstError; @@ -267,16 +292,23 @@ export class Watcher extends EventEmitter { } } - async #startBackends(clocks?: WatchmanClocks): Promise { - const {extensions, ignorePatternForWatch} = this.#options; - const useWatchman = this.#useWatchmanWatcher; - + #getBackendClass(): + Class | Class | Class { // WatchmanWatcher > NativeWatcher > FallbackWatcher - const WatcherImpl = useWatchman + return this.#useWatchmanWatcher ? WatchmanWatcher : NativeWatcher.isSupported() ? NativeWatcher : FallbackWatcher; + } + + async #startBackends( + clocks?: WatchmanClocks, + crawlFed?: boolean = false, + ): Promise { + const {extensions, ignorePatternForWatch} = this.#options; + const useWatchman = this.#useWatchmanWatcher; + const WatcherImpl = this.#getBackendClass(); let watcher = 'fallback'; if (WatcherImpl === WatchmanWatcher) { @@ -307,8 +339,10 @@ export class Watcher extends EventEmitter { const createWatcherBackend = ( root: Path, rootClocks: ?Map, + crawlFed: boolean, ): Promise => { const watcherOptions: WatcherBackendOptions = { + crawlFed, included: { extensions: new Set(extensions), // Ensure we always include package.json files, which are crucial for @@ -366,10 +400,10 @@ export class Watcher extends EventEmitter { // A root added by watchRoot() is watched before it is crawled, so it // needs no crawl clock. - this.#createBackend = root => createWatcherBackend(root, null); + this.#createBackend = root => createWatcherBackend(root, null, false); this.#backends = await Promise.all( this.#options.roots.map(root => - createWatcherBackend(root, watchmanClocks), + createWatcherBackend(root, watchmanClocks, crawlFed), ), ); // Confirmed only once every backend has started, because starting one may diff --git a/packages/metro-file-map/src/__tests__/Watcher-test.js b/packages/metro-file-map/src/__tests__/Watcher-test.js index cda12f91eb..9a8a16ae38 100644 --- a/packages/metro-file-map/src/__tests__/Watcher-test.js +++ b/packages/metro-file-map/src/__tests__/Watcher-test.js @@ -9,6 +9,7 @@ * @oncall react_native */ +import type {CrawlWatcher} from '../crawlers/node'; import type { Crawler, CrawlerOptions, @@ -24,38 +25,65 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; const mockWatchmanCrawl = jest.fn<[CrawlerOptions], Promise>(); -const mockNodeCrawl = jest.fn<[CrawlerOptions], Promise>(); +const mockNodeCrawl = jest.fn< + [CrawlerOptions] | [CrawlerOptions, ReadonlyArray], + Promise, +>(); jest.mock( '../crawlers/watchman', () => (options: CrawlerOptions) => mockWatchmanCrawl(options), ); jest.mock( '../crawlers/node', - () => (options: CrawlerOptions) => mockNodeCrawl(options), + () => + (options: CrawlerOptions, crawlWatchers?: ?ReadonlyArray) => + crawlWatchers != null + ? mockNodeCrawl(options, crawlWatchers) + : mockNodeCrawl(options), ); type MockBackend = { - kind: 'native' | 'watchman', + kind: 'fallback' | 'native' | 'watchman', root: string, clocks: ?Map, + crawlFed: boolean, + crawledDirectories: Array, emit: (event: WatcherBackendChangeEvent) => void, }; const mockStartedBackends: Array = []; let mockNativeLiveOnStart = true; +let mockNativeSupported = true; function mockBackendClass(kind: MockBackend['kind']) { return class { + readonly root: string; #root: string; #clocks: ?Map; + #crawlFed: boolean; + #crawledDirectories: Array = []; #listener: ?(event: WatcherBackendChangeEvent) => void; static isSupported(): boolean { - return true; + return kind !== 'native' || mockNativeSupported; } static isLiveOnStart(): boolean { return kind !== 'native' || mockNativeLiveOnStart; } - constructor(root: string, opts: {clocks?: ?Map, ...}) { + constructor( + root: string, + opts: {clocks?: ?Map, crawlFed?: boolean, ...}, + ) { + this.root = root; this.#root = root; this.#clocks = opts.clocks; + this.#crawlFed = opts.crawlFed === true; + } + getCrawlWatcher(): CrawlWatcher { + return { + onDirectory: dir => { + this.#crawledDirectories.push(dir); + }, + onSkippedDirectory: async () => {}, + onSymlink: () => {}, + }; } onFileEvent(listener: (event: WatcherBackendChangeEvent) => void) { this.#listener = listener; @@ -65,6 +93,8 @@ function mockBackendClass(kind: MockBackend['kind']) { kind, root: this.#root, clocks: this.#clocks, + crawlFed: this.#crawlFed, + crawledDirectories: this.#crawledDirectories, emit: event => this.#listener?.(event), }); } @@ -82,8 +112,13 @@ jest.mock('../watchers/WatchmanWatcher', () => ({ __esModule: true, default: mockBackendClass('watchman'), })); +jest.mock('../watchers/FallbackWatcher', () => ({ + __esModule: true, + default: mockBackendClass('fallback'), +})); -const rootDir = path.join(path.sep, 'project'); +// Resolved, as roots are in practice (on Windows, with a drive letter) +const rootDir = path.resolve(path.sep, 'project'); function crawlResult(canonicalPaths: ReadonlyArray): CrawlResult { const changedFiles: FileData = new Map(); @@ -142,6 +177,7 @@ function createWatcher( beforeEach(() => { mockStartedBackends.length = 0; mockNativeLiveOnStart = true; + mockNativeSupported = true; mockWatchmanCrawl.mockReset(); mockNodeCrawl.mockReset(); jest.spyOn(console, 'warn').mockImplementation(() => {}); @@ -388,6 +424,59 @@ describe('Watcher with a backend that is not live on start', () => { }); }); +describe('Watcher with FallbackWatcher and the node crawler', () => { + beforeEach(() => { + mockNativeSupported = false; + }); + + test('watches what the crawl lists, instead of walking the roots', async () => { + const src = path.join(rootDir, 'src'); + mockNodeCrawl.mockImplementation(async (options, crawlWatchers) => { + // Started, and watching the root, before the crawl + expect(mockStartedBackends.map(b => [b.kind, b.crawlFed])).toEqual([ + ['fallback', true], + ]); + expect(crawlWatchers).toHaveLength(1); + crawlWatchers?.[0]?.onDirectory(src); + return crawlResult([]); + }); + const watcher = createWatcher(false, null, true); + + await watcher.crawl(); + + expect(mockStartedBackends[0].crawledDirectories).toEqual([src]); + }); + + test('a scoped recrawl is not reported to the watcher', async () => { + mockNodeCrawl.mockImplementation(async () => crawlResult([])); + const watcher = createWatcher(false, null, true); + await watcher.crawl(); + mockNodeCrawl.mockClear(); + + await watcher.recrawl( + 'src', + new TreeFS({ + processFile: () => { + throw new Error('not implemented'); + }, + rootDir, + }), + ); + + expect(mockNodeCrawl.mock.calls).toEqual([[expect.any(Object)]]); + }); + + test('with a custom crawler, the watcher walks the roots itself', async () => { + const watcher = createWatcher(false, async () => crawlResult([]), true); + + await watcher.crawl(); + + expect(mockStartedBackends.map(b => [b.kind, b.crawlFed])).toEqual([ + ['fallback', false], + ]); + }); +}); + describe('Watcher crawl option', () => { const stubCrawler = (canonicalPaths: ReadonlyArray): Crawler => diff --git a/packages/metro-file-map/src/__tests__/addRoot-test.js b/packages/metro-file-map/src/__tests__/addRoot-test.js index 1c9da13a62..93b3201edf 100644 --- a/packages/metro-file-map/src/__tests__/addRoot-test.js +++ b/packages/metro-file-map/src/__tests__/addRoot-test.js @@ -9,6 +9,7 @@ * @oncall react_native */ +import type {CrawlWatcher} from '../crawlers/node'; import type { CacheData, ChangeEvent, @@ -90,6 +91,14 @@ class MockWatcher extends AbstractWatcher { static isSupported(): boolean { return true; } + // Stands in for FallbackWatcher too, which a node crawl may report to + getCrawlWatcher(): CrawlWatcher { + return { + onDirectory: () => {}, + onSkippedDirectory: async () => {}, + onSymlink: () => {}, + }; + } } jest.mock('../watchers/FallbackWatcher', () => MockWatcher); diff --git a/packages/metro-file-map/src/__tests__/index-test.js b/packages/metro-file-map/src/__tests__/index-test.js index 22409b2a10..7300e7d87b 100644 --- a/packages/metro-file-map/src/__tests__/index-test.js +++ b/packages/metro-file-map/src/__tests__/index-test.js @@ -10,6 +10,7 @@ */ import type {InputOptions} from '..'; +import type {CrawlWatcher} from '../crawlers/node'; import type { BuildResult, CanonicalPath, @@ -156,6 +157,14 @@ class MockWatcher extends AbstractWatcher { static isSupported(): boolean { return true; } + // Stands in for FallbackWatcher too, which a node crawl may report to + getCrawlWatcher(): CrawlWatcher { + return { + onDirectory: () => {}, + onSkippedDirectory: async () => {}, + onSymlink: () => {}, + }; + } } jest.mock('../watchers/FallbackWatcher', () => MockWatcher); diff --git a/packages/metro-file-map/src/__tests__/watch-integration-test.js b/packages/metro-file-map/src/__tests__/watch-integration-test.js index fd030f0bdb..34f8144eb2 100644 --- a/packages/metro-file-map/src/__tests__/watch-integration-test.js +++ b/packages/metro-file-map/src/__tests__/watch-integration-test.js @@ -172,4 +172,17 @@ describe('FileMap in watch mode', () => { await until('marker.js added', () => exists('src/marker.js')); expect(exists('ignored-dir/y.js')).toBe(false); }); + + // The crawl skips node_modules without `retainAllFiles`, but changes in it + // are still reported in watch mode. + test('watches a directory the crawl skips', async () => { + write('node_modules/pkg/index.js'); + await fileMap.end(); + fileMap = createFileMap({retainAllFiles: false}); + ({fileSystem} = await fileMap.build()); + expect(exists('node_modules/pkg/index.js')).toBe(false); + + write('node_modules/pkg/new.js'); + await until('new.js added', () => exists('node_modules/pkg/new.js')); + }); }); diff --git a/packages/metro-file-map/src/crawlers/__tests__/node-test.js b/packages/metro-file-map/src/crawlers/__tests__/node-test.js index a8a0edebf5..6e5a31774a 100644 --- a/packages/metro-file-map/src/crawlers/__tests__/node-test.js +++ b/packages/metro-file-map/src/crawlers/__tests__/node-test.js @@ -401,6 +401,37 @@ describe.each([['win32'], ['posix']])('node crawler on %s', platform => { expect(fs.lstat).toHaveBeenCalledTimes(2); }); + test("reports each root's directories and symlinks to its watcher", async () => { + nodeCrawl = require('../node').default; + const reported: Array> = [[], []]; + const watcherFor = (i: number) => ({ + onDirectory: (dir: string) => reported[i].push(dir), + onSkippedDirectory: async () => {}, + onSymlink: (symlink: string) => reported[i].push(`link:${symlink}`), + }); + + await nodeCrawl( + { + console: global.console, + previousState: {fileSystem: emptyFS}, + extensions: ['js'], + ignore: pearMatcher, + rootDir, + roots: [p('/project/fruits'), p('/project/prototype')], + }, + [watcherFor(0), watcherFor(1)], + ); + + expect(reported).toEqual([ + [ + p('/project/fruits'), + p('/project/fruits/directory'), + `link:${p('/project/fruits/symlink')}`, + ], + [p('/project/prototype')], + ]); + }); + test('aborts the crawl on pre-aborted signal', async () => { nodeCrawl = require('../node').default; const err = new Error('aborted for test'); diff --git a/packages/metro-file-map/src/crawlers/node/index.js b/packages/metro-file-map/src/crawlers/node/index.js index 31b1f3c8d2..fda3ef38e9 100644 --- a/packages/metro-file-map/src/crawlers/node/index.js +++ b/packages/metro-file-map/src/crawlers/node/index.js @@ -16,7 +16,24 @@ import {RootPathUtils} from '../../lib/RootPathUtils'; import fs from 'node:fs'; import * as path from 'node:path'; -function find(options: CrawlerOptions): Promise { +/** + * Watches a root as the crawl lists it, instead of walking it itself. + */ +export type CrawlWatcher = Readonly<{ + // Called with each directory just before it is listed, so that it can be + // watched first. + onDirectory: (absolutePath: string) => void, + // Called with each directory the crawl skips as ignored, which the watcher + // may still need to watch. The crawl waits for the returned promise. + onSkippedDirectory: (absolutePath: string) => Promise, + // Called with each symlink found, whether or not the crawl includes it. + onSymlink: (absolutePath: string) => void, +}>; + +async function find( + options: CrawlerOptions, + watchers: ?ReadonlyArray, +): Promise { const { console, extensions, @@ -30,15 +47,16 @@ function find(options: CrawlerOptions): Promise { // consumers (e.g. a package.json cache) may hold contents read without a // stat, so it stats every file. const statRegularFiles = subpath != null; + const skippedDirectoryWalks: Array> = []; const result: FileData = new Map(); const pathUtils = new RootPathUtils(rootDir); const exts = new Set(extensions); - return new Promise(resolve => { + await new Promise(resolve => { let activeCalls = 0; const resolveIfDone = () => { if (activeCalls === 0) { - resolve(result); + resolve(); } }; @@ -49,8 +67,10 @@ function find(options: CrawlerOptions): Promise { dirPrefix: string, dirNormal: string, isWithinRoot: boolean, + watcher: ?CrawlWatcher, ): void { activeCalls++; + watcher?.onDirectory(directory); fs.readdir(directory, {withFileTypes: true}, (err, entries) => { activeCalls--; if (err) { @@ -63,7 +83,16 @@ function find(options: CrawlerOptions): Promise { const file = dirPrefix + name; const isSymbolicLink = entry.isSymbolicLink(); - if (ignore(file) || (!includeSymlinks && isSymbolicLink)) { + if (isSymbolicLink) { + watcher?.onSymlink(file); + } + if (ignore(file)) { + if (watcher != null && entry.isDirectory()) { + skippedDirectoryWalks.push(watcher.onSkippedDirectory(file)); + } + continue; + } + if (!includeSymlinks && isSymbolicLink) { continue; } @@ -83,6 +112,7 @@ function find(options: CrawlerOptions): Promise { file + path.sep, childNormal, isWithinRoot || childNormal === '', + watcher, ); continue; } @@ -121,7 +151,7 @@ function find(options: CrawlerOptions): Promise { }); } - for (const root of roots) { + roots.forEach((root, rootIndex) => { const rootNormal = pathUtils.absoluteToNormal(root); const isWithinRoot = rootNormal !== '..' && !rootNormal.startsWith('..' + path.sep); @@ -130,15 +160,20 @@ function find(options: CrawlerOptions): Promise { root.endsWith(path.sep) ? root : root + path.sep, rootNormal, isWithinRoot, + watchers?.[rootIndex], ); - } + }); // Resolve now if there were no roots to search. resolveIfDone(); }); + await Promise.all(skippedDirectoryWalks); + return result; } export default async function nodeCrawl( options: CrawlerOptions, + // If given, a watcher for each of `options.roots`, in order. + watchers?: ?ReadonlyArray, ): Promise { const {abortSignal, perfLogger, previousState, subpath} = options; @@ -146,7 +181,7 @@ export default async function nodeCrawl( perfLogger?.point('nodeCrawl_start'); - const fileData = await find(options); + const fileData = await find(options, watchers); abortSignal?.throwIfAborted(); diff --git a/packages/metro-file-map/src/watchers/FallbackWatcher.js b/packages/metro-file-map/src/watchers/FallbackWatcher.js index 18b3527e9c..861ce72af7 100644 --- a/packages/metro-file-map/src/watchers/FallbackWatcher.js +++ b/packages/metro-file-map/src/watchers/FallbackWatcher.js @@ -13,10 +13,12 @@ * Originally vendored from https://github.com/amasad/sane/blob/64ff3a870c42e84f744086884bf55a4f9c22d376/src/node_watcher.js */ +import type {CrawlWatcher} from '../crawlers/node'; import type { ChangeEventMetadata, WatcherBackendChangeEvent, } from '../flow-types'; +import type {WatcherOptions} from './common'; import type {Dirent, FSWatcher} from 'node:fs'; import {AbstractWatcher} from './AbstractWatcher'; @@ -58,13 +60,27 @@ export default class FallbackWatcher extends AbstractWatcher { readonly #watched: {[key: string]: FSWatcher, __proto__: null} = Object.create(null); + #closed: boolean = false; + readonly #crawlFed: boolean; + // Directories reported by the crawl that are not to be watched, because + // they or an ancestor are ignored. + readonly #crawlSkipped: Set = new Set(); + // Not live on darwin, though FallbackWatcher isn't currently used there. static isLiveOnStart(): boolean { return platform !== 'darwin'; } + constructor(dir: string, opts: WatcherOptions) { + super(dir, opts); + this.#crawlFed = opts.crawlFed === true; + } + async startWatching(): Promise { this.#watchdir(this.root); + if (this.#crawlFed) { + return; + } await this.#watchTree(this.root); } @@ -87,6 +103,48 @@ export default class FallbackWatcher extends AbstractWatcher { ); } + // Whether a directory the crawl reports isn't to be watched, because it or + // an ancestor is ignored. Records it if so, for its descendants. + #isCrawlSkipped(dir: string): boolean { + if (this.#crawlSkipped.has(path.dirname(dir))) { + this.#crawlSkipped.add(dir); + return true; + } + if ( + this.ignored != null && + common.posixPathMatchesPattern(this.ignored, dir) + ) { + this.#crawlSkipped.add(dir); + return true; + } + return false; + } + + /** + * In crawl-fed mode, what to watch as the crawl lists this watcher's root. + */ + getCrawlWatcher(): CrawlWatcher { + return { + onDirectory: dir => { + if (dir !== this.root && !this.#isCrawlSkipped(dir)) { + this.#watchdir(dir); + } + }, + // The crawl skipped it as ignored, so walk it here unless this watcher + // ignores it too. + onSkippedDirectory: async dir => { + if (!this.#isCrawlSkipped(dir)) { + await this.#watchTree(dir); + } + }, + onSymlink: symlink => { + if (!this.#crawlSkipped.has(path.dirname(symlink))) { + this.#register(symlink, 'l'); + } + }, + }; + } + /** * Whether events for a path are wanted, i.e. it is neither ignored nor (for * a regular file) excluded. A directory or symlink is also recorded as known, @@ -146,7 +204,7 @@ export default class FallbackWatcher extends AbstractWatcher { * Watch a directory. */ #watchdir: (dir: string) => boolean = (dir: string) => { - if (this.#watched[dir]) { + if (this.#closed || this.#watched[dir]) { return false; } if (this.root !== dir) { @@ -241,6 +299,8 @@ export default class FallbackWatcher extends AbstractWatcher { * End watching. */ async stopWatching(): Promise { + // A walk or crawl may still be listing directories. + this.#closed = true; await super.stopWatching(); const promises = Object.keys(this.#watched).map(dir => this.#stopWatching(dir), 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 4f11d35b12..406af08cae 100644 --- a/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/FallbackWatcher-test.js @@ -264,6 +264,48 @@ describe('FallbackWatcher', () => { }); }); + describe('fed by a crawl', () => { + beforeEach(() => { + watcher = new FallbackWatcher(watchRoot, { + crawlFed: true, + included: null, + ignored: /ignored/, + watchmanDeferStates: [], + }); + }); + + test('watches only what the crawl reports', async () => { + await mkdir(join(watchRoot, 'a', 'b'), {recursive: true}); + await mkdir(join(watchRoot, 'ignored', 'c'), {recursive: true}); + await watcher?.startWatching(); + expect(calls).toEqual([`watch:${watchRoot}`]); + + const crawlWatcher = watcher?.getCrawlWatcher(); + crawlWatcher?.onDirectory(join(watchRoot, 'a')); + crawlWatcher?.onDirectory(join(watchRoot, 'ignored')); + crawlWatcher?.onDirectory(join(watchRoot, 'ignored', 'c')); + + expect(calls).toEqual([ + `watch:${watchRoot}`, + `watch:${join(watchRoot, 'a')}`, + ]); + }); + + // end() may be called while the crawl is still listing directories + test('watches nothing once stopped', async () => { + await mkdir(join(watchRoot, 'a', 'b'), {recursive: true}); + await watcher?.startWatching(); + const crawlWatcher = watcher?.getCrawlWatcher(); + await watcher?.stopWatching(); + calls = []; + + crawlWatcher?.onDirectory(join(watchRoot, 'a')); + await crawlWatcher?.onSkippedDirectory(join(watchRoot, 'a')); + + expect(calls.filter(call => call.startsWith('watch:'))).toEqual([]); + }); + }); + describe('after a directory watcher errors', () => { let erroredWatcher: ErroredFSWatcher; diff --git a/packages/metro-file-map/src/watchers/common.js b/packages/metro-file-map/src/watchers/common.js index 4d0e51137e..4d65bd979e 100644 --- a/packages/metro-file-map/src/watchers/common.js +++ b/packages/metro-file-map/src/watchers/common.js @@ -37,6 +37,9 @@ export type WatcherOptions = Readonly<{ // its root rather than a fresh one, so that changes made while the crawl // result is applied and persisted are not missed. clocks?: ?Map, + // FallbackWatcher only: watch only the root on start, and the rest as the + // crawl reports it (see getCrawlWatcher), instead of walking. + crawlFed?: boolean, included: ?WatcherIncludedFiles, ignored: ?RegExp, watchmanDeferStates: ReadonlyArray,