diff --git a/packages/metro-file-map/src/Watcher.js b/packages/metro-file-map/src/Watcher.js index 5040c8d7de..c1bd3ab6be 100644 --- a/packages/metro-file-map/src/Watcher.js +++ b/packages/metro-file-map/src/Watcher.js @@ -17,11 +17,16 @@ import type { PerfLogger, WatcherBackend, WatcherBackendChangeEvent, + WatchmanClocks, + WatchmanClockSpec, } from './flow-types'; import type {WatcherOptions as WatcherBackendOptions} from './watchers/common'; import nodeCrawl from './crawlers/node'; import watchmanCrawl from './crawlers/watchman'; +import normalizePathSeparatorsToPosix from './lib/normalizePathSeparatorsToPosix'; +import normalizePathSeparatorsToSystem from './lib/normalizePathSeparatorsToSystem'; +import {RootPathUtils} from './lib/RootPathUtils'; import {TOUCH_EVENT} from './watchers/common'; import FallbackWatcher from './watchers/FallbackWatcher'; import NativeWatcher from './watchers/NativeWatcher'; @@ -82,26 +87,45 @@ export type HealthCheckResult = export class Watcher extends EventEmitter { #activeWatcher: ?string; #backends: Array = []; + // Events received by started backends before watch() is called, to be + // replayed once it is. + #bufferedEvents: Array = []; #closed: boolean = false; #createBackend: ?(root: Path) => Promise; readonly #instanceId: number; #nextHealthCheckId: number = 0; + #onChange: ?(change: WatcherBackendChangeEvent) => void; readonly #options: WatcherOptions; readonly #pendingHealthChecks: Map< /* basename */ string, /* resolve */ () => void, > = new Map(); + // Whether watch() uses a Watchman backend. False if the Watchman crawl fell + // back to the node crawler, which returns no clock to subscribe `since`. + #useWatchmanWatcher: boolean; constructor(options: WatcherOptions) { super(); this.#options = options; this.#instanceId = nextInstanceId++; + this.#useWatchmanWatcher = options.useWatchman; } async crawl(): Promise { this.#options.perfLogger?.point('crawl_start'); const options = this.#options; + // Non-Watchman backends have no clock to subscribe `since`, so they must + // already be watching before the crawl begins, and their events buffered, + // for changes made while crawling not to be missed. Watchman backends are + // started by watch(), which subscribes `since` the crawl's clock to cover + // 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. + if (!options.useWatchman) { + await this.#startBackendsBeforeCrawl(); + } + const result = await this.#crawl({ previousState: options.previousState, roots: options.roots, @@ -112,6 +136,17 @@ export class Watcher extends EventEmitter { return result; } + async #startBackendsBeforeCrawl(): Promise { + if (!this.#options.watch) { + return; + } + await this.#startBackends(); + // end() may have closed us before the backends were set - don't leak them. + if (this.#options.abortSignal.aborted) { + await this.close(); + } + } + async recrawl( subpath: string, currentFileSystem: CrawlerOptions['previousState']['fileSystem'], @@ -180,6 +215,10 @@ export class Watcher extends EventEmitter { ' ' + firstError.toString(), ); + // Watch with a non-Watchman backend, started before the node crawl so + // that changes made during it are not missed. + this.#useWatchmanWatcher = false; + await this.#startBackendsBeforeCrawl(); try { delta = await nodeCrawl(crawlerOptions); } catch (retryError) { @@ -201,8 +240,28 @@ export class Watcher extends EventEmitter { return delta; } - async watch(onChange: (change: WatcherBackendChangeEvent) => void) { - const {extensions, ignorePatternForWatch, useWatchman} = this.#options; + async watch( + onChange: (change: WatcherBackendChangeEvent) => void, + clocks?: WatchmanClocks, + ): Promise { + this.#onChange = onChange; + + // Replay anything a backend started before the crawl reported in the + // meantime - these are handled exactly like live events. + const bufferedEvents = this.#bufferedEvents; + this.#bufferedEvents = []; + for (const event of bufferedEvents) { + onChange(event); + } + + if (this.#backends.length === 0) { + await this.#startBackends(clocks); + } + } + + async #startBackends(clocks?: WatchmanClocks): Promise { + const {extensions, ignorePatternForWatch} = this.#options; + const useWatchman = this.#useWatchmanWatcher; // WatchmanWatcher > NativeWatcher > FallbackWatcher const WatcherImpl = useWatchman @@ -221,7 +280,26 @@ export class Watcher extends EventEmitter { this.#options.perfLogger?.annotate({string: {watcher}}); this.#activeWatcher = watcher; - const createWatcherBackend = (root: Path): Promise => { + // Crawl clocks are keyed by path relative to the project root, but a + // backend only learns its Watchman watch root after `watch-project`, so + // hand each the whole map keyed by absolute watch root to look up. + let watchmanClocks: ?Map; + if (useWatchman && clocks != null) { + const pathUtils = new RootPathUtils(this.#options.rootDir); + watchmanClocks = new Map( + [...clocks].map(([root, clockSpec]) => [ + normalizePathSeparatorsToPosix( + pathUtils.normalToAbsolute(normalizePathSeparatorsToSystem(root)), + ), + clockSpec, + ]), + ); + } + + const createWatcherBackend = ( + root: Path, + rootClocks: ?Map, + ): Promise => { const watcherOptions: WatcherBackendOptions = { included: { extensions: new Set(extensions), @@ -233,6 +311,7 @@ export class Watcher extends EventEmitter { }, ignored: ignorePatternForWatch, watchmanDeferStates: this.#options.watchmanDeferStates, + clocks: rootClocks, }; const watcher: WatcherBackend = new WatcherImpl(root, watcherOptions); @@ -264,7 +343,12 @@ export class Watcher extends EventEmitter { ); return; } - onChange(change); + const onChange = this.#onChange; + if (onChange != null) { + onChange(change); + } else { + this.#bufferedEvents.push(change); + } }); await watcher.startWatching(); clearTimeout(rejectTimeout); @@ -272,16 +356,20 @@ export class Watcher extends EventEmitter { }); }; - this.#createBackend = createWatcherBackend; + // A root added by watchRoot() is watched before it is crawled, so it + // needs no crawl clock. + this.#createBackend = root => createWatcherBackend(root, null); this.#backends = await Promise.all( - this.#options.roots.map(createWatcherBackend), + this.#options.roots.map(root => + createWatcherBackend(root, watchmanClocks), + ), ); } /** * Watch a root in addition to those given at construction, reporting its * changes to the `onChange` passed to `watch()`. Does nothing outside watch - * mode, i.e. before `watch()`. + * mode, i.e. before the backends for the initial roots have started. */ async watchRoot(root: Path): Promise { const backend = await this.#createBackend?.(root); diff --git a/packages/metro-file-map/src/__tests__/Watcher-test.js b/packages/metro-file-map/src/__tests__/Watcher-test.js index a49835b72c..bb76b56dc5 100644 --- a/packages/metro-file-map/src/__tests__/Watcher-test.js +++ b/packages/metro-file-map/src/__tests__/Watcher-test.js @@ -15,6 +15,7 @@ import type { CrawlResult, FileData, FileMetadata, + WatcherBackendChangeEvent, } from '../flow-types'; import TreeFS from '../lib/TreeFS'; @@ -32,6 +33,48 @@ jest.mock( () => (options: CrawlerOptions) => mockNodeCrawl(options), ); +type MockBackend = { + kind: 'native' | 'watchman', + root: string, + clocks: ?Map, + emit: (event: WatcherBackendChangeEvent) => void, +}; +const mockStartedBackends: Array = []; +function mockBackendClass(kind: MockBackend['kind']) { + return class { + #root: string; + #clocks: ?Map; + #listener: ?(event: WatcherBackendChangeEvent) => void; + static isSupported(): boolean { + return true; + } + constructor(root: string, opts: {clocks?: ?Map, ...}) { + this.#root = root; + this.#clocks = opts.clocks; + } + onFileEvent(listener: (event: WatcherBackendChangeEvent) => void) { + this.#listener = listener; + } + async startWatching() { + mockStartedBackends.push({ + kind, + root: this.#root, + clocks: this.#clocks, + emit: event => this.#listener?.(event), + }); + } + async stopWatching() {} + }; +} +jest.mock('../watchers/NativeWatcher', () => ({ + __esModule: true, + default: mockBackendClass('native'), +})); +jest.mock('../watchers/WatchmanWatcher', () => ({ + __esModule: true, + default: mockBackendClass('watchman'), +})); + const rootDir = path.join(path.sep, 'project'); function crawlResult(canonicalPaths: ReadonlyArray): CrawlResult { @@ -55,7 +98,11 @@ const rejectWith = throw new Error(message); }; -function createWatcher(useWatchman: boolean, crawl?: ?Crawler): Watcher { +function createWatcher( + useWatchman: boolean, + crawl?: ?Crawler, + watch: boolean = false, +): Watcher { return new Watcher({ abortSignal: new AbortController().signal, computeSha1: false, @@ -79,12 +126,13 @@ function createWatcher(useWatchman: boolean, crawl?: ?Crawler): Watcher { rootDir, roots: [rootDir], useWatchman, - watch: false, + watch, watchmanDeferStates: [], }); } beforeEach(() => { + mockStartedBackends.length = 0; mockWatchmanCrawl.mockReset(); mockNodeCrawl.mockReset(); jest.spyOn(console, 'warn').mockImplementation(() => {}); @@ -169,6 +217,62 @@ describe('Watcher crawl fallback', () => { }); }); +describe('Watcher backend choice in watch mode', () => { + const event: WatcherBackendChangeEvent = { + event: 'touch', + relativePath: 'index.js', + root: rootDir, + metadata: {modifiedTime: 1, size: 1, type: 'f'}, + }; + + test('a successful Watchman crawl starts a Watchman backend only on watch()', async () => { + mockWatchmanCrawl.mockImplementation(async () => crawlResult([])); + const watcher = createWatcher(true, null, true); + + await watcher.crawl(); + expect(mockStartedBackends).toEqual([]); + + await watcher.watch(() => {}); + expect(mockStartedBackends.map(b => b.kind)).toEqual(['watchman']); + }); + + test('only the initial Watchman backends subscribe since the crawl clocks', async () => { + mockWatchmanCrawl.mockImplementation(async () => crawlResult([])); + const watcher = createWatcher(true, null, true); + + await watcher.crawl(); + await watcher.watch(() => {}, new Map([['', 'c:crawl-clock:1']])); + const addedRoot = path.join(path.sep, 'other'); + await watcher.watchRoot(addedRoot); + + expect( + mockStartedBackends.map(b => [b.root, b.clocks && [...b.clocks]]), + ).toEqual([ + [rootDir, [[rootDir.replaceAll(path.sep, '/'), 'c:crawl-clock:1']]], + [addedRoot, null], + ]); + }); + + test('a failing Watchman crawl starts a native backend before the node crawl, and watch() replays its events', async () => { + mockWatchmanCrawl.mockImplementation(rejectWith('watchman unavailable')); + mockNodeCrawl.mockImplementation(async () => { + // The backend is already watching while the node crawler runs. + expect(mockStartedBackends.map(b => b.kind)).toEqual(['native']); + mockStartedBackends[0].emit(event); + return crawlResult([]); + }); + const watcher = createWatcher(true, null, true); + + await watcher.crawl(); + const onChange = jest.fn<[WatcherBackendChangeEvent], void>(); + await watcher.watch(onChange); + + expect(onChange.mock.calls).toEqual([[event]]); + // No Watchman backend is started - the node crawl has no clock for it. + expect(mockStartedBackends.map(b => b.kind)).toEqual(['native']); + }); +}); + describe('Watcher crawl option', () => { const stubCrawler = (canonicalPaths: ReadonlyArray): Crawler => diff --git a/packages/metro-file-map/src/__tests__/index-test.js b/packages/metro-file-map/src/__tests__/index-test.js index 1dd061c432..9d980c1b55 100644 --- a/packages/metro-file-map/src/__tests__/index-test.js +++ b/packages/metro-file-map/src/__tests__/index-test.js @@ -22,13 +22,13 @@ import type { HasteMap, MockMap, ReadonlyFileSystemChanges, - WatcherBackendOptions, WorkerSetupArgs, } from '../flow-types'; import type {DependencyPlugin} from '../index'; import type {default as FileMapT} from '../index'; import type {HasteMapOptions} from '../plugins/HastePlugin'; import type {MockMapOptions} from '../plugins/MockPlugin'; +import type {WatcherOptions} from '../watchers/common'; import typeof WorkerModule from '../worker'; import {AbstractWatcher} from '../watchers/AbstractWatcher'; @@ -52,6 +52,11 @@ const toPosixMessage = (message: string): string => ? message.replaceAll('C:\\', '/').replaceAll('\\', '/') : message; +// Format a posix path as a Watchman-native path on the current platform, i.e., +// on Windows, drive letters on absolute paths, but posix-style separators. +const wp = (filePath: string): string => + process.platform === 'win32' ? filePath.replace(/^\//, 'C:/') : filePath; + function mockHashContents(contents: string | Buffer) { return crypto.createHash('sha1').update(contents).digest('hex'); } @@ -140,8 +145,11 @@ jest.mock('../crawlers/watchman', () => ({ })); class MockWatcher extends AbstractWatcher { - constructor(root: string, opts: WatcherBackendOptions) { + readonly opts: WatcherOptions; + + constructor(root: string, opts: WatcherOptions) { super(root, opts); + this.opts = opts; mockEmitters[root] = this; } @@ -1819,6 +1827,56 @@ describe('FileMap', () => { size: 55, }; + fm_it( + 'passes the clocks from the crawl to the watcher, keyed by watch root', + async () => { + for (const root of [p('/project/fruits'), p('/project/vegetables')]) { + expect(mockEmitters[root].opts.clocks).toEqual( + new Map([ + [wp('/project/fruits'), 'c:fake-clock:1'], + [wp('/project/vegetables'), 'c:fake-clock:2'], + [wp('/project/video'), 'c:fake-clock:3'], + ]), + ); + } + }, + ); + + test('does not miss a change made while the file map is being built', async () => { + mockNodeCrawler.mockImplementationOnce(async () => { + // The watcher is already running at this point, so this reports a + // change that the crawl itself cannot see. + mockFs[p('/project/fruits/Tomato.js')] = '// Tomato!'; + mockEmitters[p('/project/fruits')]?.emitFileEvent({ + event: 'touch', + relativePath: 'Tomato.js', + metadata: MOCK_CHANGE_FILE, + }); + return {changedFiles: new Map(), removedFiles: new Set()}; + }); + + let fileMap: ?FileMapT = null; + let fileSystem: ?FileSystem = null; + try { + ({fileMap, fileSystem} = await buildNewFileMap({ + useWatchman: false, + watch: true, + })); + const changeEvent = await Promise.race([ + waitForItToChange(fileMap), + new Promise(resolve => setTimeout(() => resolve(null), 1000)), + ]); + expect(fileSystem?.exists(p('/project/fruits/Tomato.js'))).toBe(true); + expect( + [...(changeEvent?.changes.addedFiles ?? [])].map( + ([filePath]) => filePath, + ), + ).toContain(path.join('fruits', 'Tomato.js')); + } finally { + await fileMap?.end(); + } + }); + fm_it( 'handles several change events at once', async ({fileMap, hasteMap}) => { diff --git a/packages/metro-file-map/src/index.js b/packages/metro-file-map/src/index.js index 55de602566..f0970e7c6d 100644 --- a/packages/metro-file-map/src/index.js +++ b/packages/metro-file-map/src/index.js @@ -1216,7 +1216,7 @@ export default class FileMap extends EventEmitter { this.#watcher != null, 'Expected #watcher to have been initialised by build()', ); - await this.#watcher.watch(onChange); + await this.#watcher.watch(onChange, clocks); if (this.#options.healthCheck.enabled) { const performHealthCheck = () => { diff --git a/packages/metro-file-map/src/watchers/WatchmanWatcher.js b/packages/metro-file-map/src/watchers/WatchmanWatcher.js index c01914309e..6dbe5582e0 100644 --- a/packages/metro-file-map/src/watchers/WatchmanWatcher.js +++ b/packages/metro-file-map/src/watchers/WatchmanWatcher.js @@ -9,6 +9,7 @@ * @oncall react_native */ +import type {WatchmanClockSpec} from '../flow-types'; import type {WatcherOptions} from './common'; import type { Client, @@ -20,6 +21,7 @@ import type { WatchmanWatchResponse, } from 'fb-watchman'; +import normalizePathSeparatorsToPosix from '../lib/normalizePathSeparatorsToPosix'; import normalizePathSeparatorsToSystem from '../lib/normalizePathSeparatorsToSystem'; import {AbstractWatcher} from './AbstractWatcher'; import * as common from './common'; @@ -48,12 +50,17 @@ export default class WatchmanWatcher extends AbstractWatcher { }>; readonly #watchmanDeferStates: ReadonlyArray; #deferringStates: ?Set = null; + // Clocks captured by the crawl, keyed by absolute (posix-separated) Watchman + // watch root. Consumed by the initial subscription only - a re-subscribe + // after disconnection starts from a fresh clock as before. + #clocks: ?Map; constructor(dir: string, opts: WatcherOptions) { - const {watchmanDeferStates, ...baseOpts} = opts; + const {watchmanDeferStates, clocks, ...baseOpts} = opts; super(dir, baseOpts); this.#watchmanDeferStates = watchmanDeferStates; + this.#clocks = clocks ?? null; // Use a unique subscription name per process per watched directory const watchKey = createHash('md5').update(this.root).digest('hex'); @@ -117,7 +124,18 @@ export default class WatchmanWatcher extends AbstractWatcher { root: normalizePathSeparatorsToSystem(resp.watch), }; - self.#client.command(['clock', getWatchRoot()], onClock); + // If the crawl gave us a clock for this watch root, subscribe `since` + // it, so that changes made between the crawl and this subscription are + // reported rather than missed. Otherwise request a fresh clock. + const sinceClock = self.#clocks?.get( + normalizePathSeparatorsToPosix(resp.watch), + ); + self.#clocks = null; + if (sinceClock != null) { + subscribe(sinceClock); + } else { + self.#client.command(['clock', getWatchRoot()], onClock); + } } function onClock(error: ?Error, resp: WatchmanClockResponse) { @@ -127,18 +145,21 @@ export default class WatchmanWatcher extends AbstractWatcher { } debug('Received clock response: %s', resp.clock); + handleWarning(resp); + subscribe(resp.clock); + } + + function subscribe(since: WatchmanClockSpec) { const watchProjectInfo = self.#watchProjectInfo; invariant( watchProjectInfo != null, - 'watch-project response should have been set before clock response', + 'watch-project response should have been set before subscribing', ); - handleWarning(resp); - const options: WatchmanQuery = { fields: ['name', 'exists', 'new', 'type', 'size', 'mtime_ms'], - since: resp.clock, + since, defer: self.#watchmanDeferStates, relative_root: watchProjectInfo.relativePath, }; diff --git a/packages/metro-file-map/src/watchers/__tests__/WatchmanWatcher-test.js b/packages/metro-file-map/src/watchers/__tests__/WatchmanWatcher-test.js index 40e7fb1540..02e8aeb925 100644 --- a/packages/metro-file-map/src/watchers/__tests__/WatchmanWatcher-test.js +++ b/packages/metro-file-map/src/watchers/__tests__/WatchmanWatcher-test.js @@ -103,6 +103,90 @@ describe('WatchmanWatcher', () => { return startPromise; }); + test('subscribes since a supplied clock instead of fetching a fresh one', () => { + const watchmanWatcher = new WatchmanWatcher(p('/project/subdir/js'), { + ignored: null, + included: { + extensions: new Set(['js']), + basenames: new Set(), + basenamePrefixes: [], + }, + watchmanDeferStates: ['busy'], + clocks: new Map([[wp('/project'), 'c:crawl-clock:1']]), + }); + const startPromise = watchmanWatcher.startWatching(); + + cmdCallback(null, { + watch: wp('/project'), + relative_path: wp('subdir/js'), + }); + + // With a clock for the watch root, there's no need for a `clock` command - + // the subscription is made `since` the supplied clock directly. + expect(mockClient.command).toHaveBeenLastCalledWith( + [ + 'subscribe', + p('/project'), + watchmanWatcher.subscriptionName, + { + defer: ['busy'], + fields: ['name', 'exists', 'new', 'type', 'size', 'mtime_ms'], + relative_root: p('subdir/js'), + since: 'c:crawl-clock:1', + }, + ], + expect.any(Function), + ); + + cmdCallback(null, {}); + return startPromise; + }); + + test('fetches a fresh clock when no supplied clock matches the watch root', () => { + const watchmanWatcher = new WatchmanWatcher(p('/project/subdir/js'), { + ignored: null, + included: { + extensions: new Set(['js']), + basenames: new Set(), + basenamePrefixes: [], + }, + watchmanDeferStates: ['busy'], + clocks: new Map([[wp('/other'), 'c:crawl-clock:1']]), + }); + const startPromise = watchmanWatcher.startWatching(); + + cmdCallback(null, { + watch: wp('/project'), + relative_path: wp('subdir/js'), + }); + + expect(mockClient.command).toHaveBeenLastCalledWith( + ['clock', p('/project')], + expect.any(Function), + ); + cmdCallback(null, { + clock: 'c:fresh-clock:1', + }); + + expect(mockClient.command).toHaveBeenLastCalledWith( + [ + 'subscribe', + p('/project'), + watchmanWatcher.subscriptionName, + { + defer: ['busy'], + fields: ['name', 'exists', 'new', 'type', 'size', 'mtime_ms'], + relative_root: p('subdir/js'), + since: 'c:fresh-clock:1', + }, + ], + expect.any(Function), + ); + + cmdCallback(null, {}); + return startPromise; + }); + describe('change handling', () => { let watchmanWatcher: WatchmanWatcher; beforeEach(async () => { diff --git a/packages/metro-file-map/src/watchers/common.js b/packages/metro-file-map/src/watchers/common.js index c7df314da6..4d0e51137e 100644 --- a/packages/metro-file-map/src/watchers/common.js +++ b/packages/metro-file-map/src/watchers/common.js @@ -14,7 +14,11 @@ * https://github.com/amasad/sane/blob/64ff3a870c42e84f744086884bf55a4f9c22d376/src/common.js */ -import type {ChangeEventMetadata, WatcherIncludedFiles} from '../flow-types'; +import type { + ChangeEventMetadata, + WatcherIncludedFiles, + WatchmanClockSpec, +} from '../flow-types'; import type {Stats} from 'node:fs'; import path from 'node:path'; @@ -28,6 +32,11 @@ export const RECRAWL_EVENT = 'recrawl'; export const ALL_EVENT = 'all'; export type WatcherOptions = Readonly<{ + // Clocks captured by the crawl, keyed by absolute Watchman watch root. + // Only consumed by WatchmanWatcher, which subscribes `since` the clock for + // its root rather than a fresh one, so that changes made while the crawl + // result is applied and persisted are not missed. + clocks?: ?Map, included: ?WatcherIncludedFiles, ignored: ?RegExp, watchmanDeferStates: ReadonlyArray,