metro-file-map: Do not miss files changed while the file map builds

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<WatcherBackend> = [];
+  // Events received by started backends before watch() is called, to be
+  // replayed once it is.
+  #bufferedEvents: Array<WatcherBackendChangeEvent> = [];
   #closed: boolean = false;
   #createBackend: ?(root: Path) => Promise<WatcherBackend>;
   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<CrawlResult> {
     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<void> {
+    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<void> {
+    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<void> {
+    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<WatcherBackend> => {
+    // 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<string, WatchmanClockSpec>;
+    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<string, WatchmanClockSpec>,
+    ): Promise<WatcherBackend> => {
       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,24 +343,33 @@ 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);
         resolve(watcher);
       });
     };
 
-    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<void> {
     const backend = await this.#createBackend?.(root);

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<string, string>,
+  emit: (event: WatcherBackendChangeEvent) => void,
+};
+const mockStartedBackends: Array<MockBackend> = [];
+function mockBackendClass(kind: MockBackend['kind']) {
+  return class {
+    #root: string;
+    #clocks: ?Map<string, string>;
+    #listener: ?(event: WatcherBackendChangeEvent) => void;
+    static isSupported(): boolean {
+      return true;
+    }
+    constructor(root: string, opts: {clocks?: ?Map<string, string>, ...}) {
+      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<string>): 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<string>): Crawler =>