Fix detection of added/removed files when a directory is moved or cloned (#1660)

Summary:
Pull Request resolved: #1660

Previously, when running on macOS without Watchman, when a directory was moved or cloned from another location (inside or outside a watch), the contents would *not* be detected by Metro at their new locations.

That's because the OS only sends us a "rename" event for the directory, and no events for any of its contents (unsurprisingly, because they haven't changed).

This stack introduces a new 'recrawl' event which watcher backends may fire when they are unable to determine whether the contents of an extant directory are new, and makes use of it in `NativeWatcher`.

This does not apply to Watchman, which has its own (similar) recrawl mechanism, and it doesn't apply on Linux or Windows, which currently use a recursive watcher that crawl all "new" directories and compares contents to a tracked file map.

Changelog:
```
 - **[Fix]**: Detection of contents of directories moved or cloned into a watched location
```

Reviewed By: vzaidman

Differential Revision: D94362228

fbshipit-source-id: d652c60539338efa273e196166b75b2327d4b114

Author: robhogan

packages/metro-file-map/src/watchers/FallbackWatcher.js

@@ -124,12 +124,27 @@ export default class FallbackWatcher extends AbstractWatcher {
   }
 
   /**
-   * Removes a dir from the registry.
+   * Removes a dir from the registry, returning all files that were registered
+   * under it (recursively).
    */
-  #unregisterDir(dirpath: string): void {
-    if (this.#dirRegistry[dirpath]) {
-      delete this.#dirRegistry[dirpath];
+  #unregisterDir(dirpath: string): Array<string> {
+    const removedFiles: Array<string> = [];
+
+    // Find and remove all entries under this directory
+    for (const registeredDir of Object.keys(this.#dirRegistry)) {
+      if (
+        registeredDir === dirpath ||
+        registeredDir.startsWith(dirpath + path.sep)
+      ) {
+        // Collect all files in this directory
+        for (const filename of Object.keys(this.#dirRegistry[registeredDir])) {
+          removedFiles.push(path.join(registeredDir, filename));
+        }
+        delete this.#dirRegistry[registeredDir];
+      }
     }
+
+    return removedFiles;
   }
 
   /**
@@ -350,7 +365,15 @@ export default class FallbackWatcher extends AbstractWatcher {
         return;
       }
       this.#unregister(fullPath);
-      this.#unregisterDir(fullPath);
+      // When a directory is deleted, emit delete events for all files we
+      // knew about under that directory
+      const removedFiles = this.#unregisterDir(fullPath);
+      for (const removedFile of removedFiles) {
+        this.#emitEvent({
+          event: DELETE_EVENT,
+          relativePath: path.relative(this.root, removedFile),
+        });
+      }
       if (registered) {
         this.#emitEvent({event: DELETE_EVENT, relativePath});
       }

packages/metro-file-map/src/watchers/NativeWatcher.js

@@ -21,6 +21,7 @@ const debug = require('debug')('Metro:NativeWatcher');
 
 const TOUCH_EVENT = 'touch';
 const DELETE_EVENT = 'delete';
+const RECRAWL_EVENT = 'recrawl';
 
 /**
  * NativeWatcher uses Node's native fs.watch API with recursive: true.
@@ -74,9 +75,8 @@ export default class NativeWatcher extends AbstractWatcher {
         // ~instant on macOS or Windows.
         recursive: true,
       },
-      (_event, relativePath) => {
-        // _event is always 'rename' on macOS, so we don't use it.
-        this._handleEvent(relativePath).catch(error => {
+      (event, relativePath) => {
+        this._handleEvent(event, relativePath).catch(error => {
           this.emitError(error);
         });
       },
@@ -95,13 +95,23 @@ export default class NativeWatcher extends AbstractWatcher {
     }
   }
 
-  async _handleEvent(relativePath: string) {
+  async _handleEvent(event: string, relativePath: string) {
     const absolutePath = path.resolve(this.root, relativePath);
     if (this.doIgnore(relativePath)) {
-      debug('Ignoring event on %s (root: %s)', relativePath, this.root);
+      debug(
+        'Ignoring event "%s" on %s (root: %s)',
+        event,
+        relativePath,
+        this.root,
+      );
       return;
     }
-    debug('Handling event on %s (root: %s)', relativePath, this.root);
+    debug(
+      'Handling event "%s" on %s (root: %s)',
+      event,
+      relativePath,
+      this.root,
+    );
 
     try {
       const stat = await fsPromises.lstat(absolutePath);
@@ -116,6 +126,23 @@ export default class NativeWatcher extends AbstractWatcher {
         return;
       }
 
+      // For directory "rename" events, notify that we need a recrawl since we
+      // wont' receive events for unmodified files underneath a moved (or
+      // cloned) directory. Renames are fired by the OS on moves, clones, and
+      // creations. We ignore "change" events because they indiciate a change
+      // to directory metadata, rather than its path or existence.
+      if (type === 'd' && event === 'rename') {
+        debug(
+          'Directory rename detected on %s, requesting recrawl',
+          relativePath,
+        );
+        this.emitFileEvent({
+          event: RECRAWL_EVENT,
+          relativePath,
+        });
+        return;
+      }
+
       this.emitFileEvent({
         event: TOUCH_EVENT,
         relativePath,

packages/metro-file-map/src/watchers/__tests__/helpers.js

@@ -42,7 +42,7 @@ const isWatchmanOnPath = () => {
 
 // `null` Watchers will be marked as skipped tests.
 export const WATCHERS: Readonly<{
-  [key: string]:
+  [key: 'Watchman' | 'Native' | 'Fallback']:
     | Class<FallbackWatcher>
     | Class<NativeWatcher>
     | Class<WatchmanWatcher>
@@ -53,6 +53,8 @@ export const WATCHERS: Readonly<{
   Fallback: FallbackWatcher,
 };
 
+export type WatcherName = keyof typeof WATCHERS;
+
 export type EventHelpers = {
   nextEvent: (afterFn: () => Promise<void>) => Promise<{
     eventType: string,
@@ -62,17 +64,17 @@ export type EventHelpers = {
   untilEvent: (
     afterFn: () => Promise<void>,
     expectedPath: string,
-    expectedEvent: 'touch' | 'delete',
+    expectedEvent: 'touch' | 'delete' | 'recrawl',
   ) => Promise<void>,
   allEvents: (
     afterFn: () => Promise<void>,
-    events: ReadonlyArray<[string, 'touch' | 'delete']>,
+    events: ReadonlyArray<[string, 'touch' | 'delete' | 'recrawl']>,
     opts?: {rejectUnexpected: boolean},
   ) => Promise<void>,
 };
 
 export const createTempWatchRoot = async (
-  watcherName: string,
+  watcherName: WatcherName,
   watchmanConfig: {[key: string]: unknown} | false = {},
 ): Promise<string> => {
   const tmpDir = await mkdtemp(
@@ -94,7 +96,7 @@ export const createTempWatchRoot = async (
 };
 
 export const startWatching = async (
-  watcherName: string,
+  watcherName: WatcherName,
   watchRoot: string,
   opts: WatcherOptions,
 ): (Promise<{

packages/metro-file-map/src/watchers/__tests__/integration-test.js

@@ -10,7 +10,7 @@
  */
 
 import type {WatcherOptions} from '../common';
-import type {EventHelpers} from './helpers';
+import type {EventHelpers, WatcherName} from './helpers';
 
 import NativeWatcher from '../NativeWatcher';
 import {WATCHERS, createTempWatchRoot, startWatching} from './helpers';
@@ -28,7 +28,7 @@ test('NativeWatcher is supported if and only if darwin', () => {
 
 describe.each(Object.keys(WATCHERS))(
   'Watcher integration tests: %s',
-  watcherName => {
+  (watcherName: WatcherName) => {
     let appRoot;
     let cookieCount = 1;
     let watchRoot;
@@ -42,6 +42,9 @@ describe.each(Object.keys(WATCHERS))(
         ? test
         : test.skip;
 
+    // NativeWatcher emits 'recrawl' for directories, others emit 'touch'
+    const expectedDirEventType = watcherName === 'Native' ? 'recrawl' : 'touch';
+
     beforeAll(async () => {
       watchRoot = await createTempWatchRoot(watcherName);
 
@@ -53,9 +56,11 @@ describe.each(Object.keys(WATCHERS))(
       // the watcher was started. Tests should touch only distinct subsets of
       // these files to ensure that tests remain isolated.
       await mkdir(join(watchRoot, 'existing'));
+      await mkdir(join(watchRoot, 'existing', 'to-move-out'));
       await Promise.all([
         writeFile(join(watchRoot, 'existing', 'file-to-delete.js'), ''),
         writeFile(join(watchRoot, 'existing', 'file-to-modify.js'), ''),
+        writeFile(join(watchRoot, 'existing', 'to-move-out', 'file.js'), ''),
         symlink('target', join(watchRoot, 'existing', 'symlink-to-delete')),
       ]);
 
@@ -82,11 +87,16 @@ describe.each(Object.keys(WATCHERS))(
     });
 
     beforeEach(async () => {
-      expect(await eventHelpers.nextEvent(() => mkdir(appRoot))).toStrictEqual({
+      // NativeWatcher emits 'recrawl' for directories, others emit 'touch'
+      const event = await eventHelpers.nextEvent(() => mkdir(appRoot));
+      expect(event).toMatchObject({
         path: 'app',
-        eventType: 'touch',
-        metadata: expect.any(Object),
+        eventType: expectedDirEventType,
       });
+      // For non-recrawl events, also check metadata
+      if (event.eventType === 'touch') {
+        expect(event.metadata).toEqual(expect.any(Object));
+      }
     });
 
     afterEach(async () => {
@@ -186,6 +196,66 @@ describe.each(Object.keys(WATCHERS))(
       });
     });
 
+    maybeTest(
+      'detects all files when a preexisting directory is moved in from outside a watched root',
+      async () => {
+        // Create a directory with a file in it outside the watch root, then move it in and check that both the directory and the file are reported as new.
+        const outsideDir = await fsPromises.mkdtemp(
+          join(os.tmpdir(), 'metro-file-map-unwatched-'),
+        );
+        const outsideFile = join(outsideDir, 'file.js');
+        await writeFile(outsideFile, '');
+
+        // NativeWatcher emits 'recrawl' for the directory, which triggers a
+        // full crawl that finds the file. Other watchers emit individual 'touch'
+        // events for both directory and file.
+        if (watcherName === 'Native') {
+          // NativeWatcher: expect recrawl event for the directory only
+          await eventHelpers.allEvents(
+            () => fsPromises.rename(outsideDir, join(appRoot, 'moved-in')),
+            [[join('app', 'moved-in'), 'recrawl']],
+            {rejectUnexpected: true},
+          );
+        } else {
+          // Other watchers: expect touch events for both directory and file
+          await eventHelpers.allEvents(
+            () => fsPromises.rename(outsideDir, join(appRoot, 'moved-in')),
+            [
+              [join('app', 'moved-in'), 'touch'],
+              [join('app', 'moved-in', 'file.js'), 'touch'],
+            ],
+            {rejectUnexpected: true},
+          );
+        }
+      },
+    );
+
+    maybeTest(
+      'reports directory as deleted when it is moved from a watched root to outside',
+      async () => {
+        // Create a directory with a file in it inside the watch root, then move it out and check that both the directory and the file are reported as deleted.
+        const outsideDir = await fsPromises.mkdtemp(
+          join(os.tmpdir(), 'metro-file-map-unwatched-'),
+        );
+
+        await eventHelpers.allEvents(
+          () =>
+            fsPromises.rename(
+              join(watchRoot, 'existing', 'to-move-out'),
+              join(outsideDir, 'moved-out'),
+            ),
+          watcherName === 'Native'
+            ? // NativeWatcher only emits an event for the directory, not contents
+              [[join('existing', 'to-move-out'), 'delete']]
+            : [
+                [join('existing', 'to-move-out'), 'delete'],
+                [join('existing', 'to-move-out', 'file.js'), 'delete'],
+              ],
+          {rejectUnexpected: true},
+        );
+      },
+    );
+
     maybeTest('detects deletion of a pre-existing symlink', async () => {
       expect(
         await eventHelpers.nextEvent(() =>
@@ -214,17 +284,21 @@ describe.each(Object.keys(WATCHERS))(
     });
 
     maybeTest('detects changes to files in a new directory', async () => {
-      expect(
-        await eventHelpers.nextEvent(() => mkdir(join(watchRoot, 'newdir'))),
-      ).toStrictEqual({
+      const dirEvent = await eventHelpers.nextEvent(() =>
+        mkdir(join(watchRoot, 'newdir')),
+      );
+      expect(dirEvent).toMatchObject({
         path: join('newdir'),
-        eventType: 'touch',
-        metadata: {
+        eventType: expectedDirEventType,
+      });
+      // For non-recrawl events, also check metadata
+      if (dirEvent.eventType === 'touch') {
+        expect(dirEvent.metadata).toStrictEqual({
           modifiedTime: expect.any(Number),
           size: expect.any(Number),
           type: 'd',
-        },
-      });
+        });
+      }
       expect(
         await eventHelpers.nextEvent(() =>
           writeFile(join(watchRoot, 'newdir', 'file-in-new-dir.js'), 'code'),
@@ -245,6 +319,9 @@ describe.each(Object.keys(WATCHERS))(
     maybeTestOn('darwin')(
       'emits deletion for all files when a directory is deleted',
       async () => {
+        // For NativeWatcher, the directory events will be 'recrawl', not 'touch'
+        const dirEventType = expectedDirEventType;
+
         await eventHelpers.allEvents(
           async () => {
             await mkdir(join(appRoot, 'subdir', 'subdir2'), {recursive: true});
@@ -255,8 +332,8 @@ describe.each(Object.keys(WATCHERS))(
             ]);
           },
           [
-            [join('app', 'subdir'), 'touch'],
-            [join('app', 'subdir', 'subdir2'), 'touch'],
+            [join('app', 'subdir'), dirEventType],
+            [join('app', 'subdir', 'subdir2'), dirEventType],
             [join('app', 'subdir', 'deep.js'), 'touch'],
             [join('app', 'subdir', 'subdir2', 'deeper.js'), 'touch'],
           ],

packages/metro-file-map/types/watchers/NativeWatcher.d.ts

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  *
  * @noformat
- * @generated SignedSource<<8239479487abff6df70ee54f9e05f352>>
+ * @generated SignedSource<<b68c5620efd3f5bec83279059d0d1b4e>>
  *
  * This file was translated from Flow by scripts/generateTypeScriptDefinitions.js
  * Original file: packages/metro-file-map/src/watchers/NativeWatcher.js
@@ -50,6 +50,6 @@ declare class NativeWatcher extends AbstractWatcher {
    * End watching.
    */
   stopWatching(): Promise<void>;
-  _handleEvent(relativePath: string): void;
+  _handleEvent(event: string, relativePath: string): void;
 }
 export default NativeWatcher;