Consistent reporting of symlink changes across watcher backends

Summary:
Currently, symlinks are reported (similarly to regular files) by all watcher backends when added or removed (if not ignored), *except* in some specific cases:
 - Symlinks discovered when adding a new subtree are not always reported by `NodeWatcher`.
 - Symlinks deleted are not reported by `FSEventsWatcher` or `NodeWatcher`.

That's simply because we use `walker` to scan files, and it distinguishes symlinks from regular files - we were not listening to the `symlink` event.
 - In the case of symlinks in very new directories, that meant we didn't emit an `add` event unless we already had a watch on the parent - the scan is expected to race against the recursive watch, so this was non-deterministic.
 - In the case of symlink deletions, these were not reported by `NodeWatcher` or `FSEventsWatcher` because those rely on an internal set of "tracked" files provided by `walker`, and don't emit events for untracked files that no longer exist.

This diff adds a listener for discovered symlinks and tests to verify pre-existing symlinks correctly have their deletion reported.

Changelog: [Internal]

*(symlink changes are not yet propagated out of `metro-file-map`, so this fix isn't technically observable)*

Reviewed By: jacdebug

Differential Revision: D41774723

fbshipit-source-id: 0e9e7d0ecd0bdfbb316af7657e7f2599b5dfb49d

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

@@ -78,6 +78,7 @@ export default class FSEventsWatcher extends EventEmitter {
     dir: string,
     dirCallback: (normalizedPath: string, stats: Stats) => void,
     fileCallback: (normalizedPath: string, stats: Stats) => void,
+    symlinkCallback: (normalizedPath: string, stats: Stats) => void,
     // $FlowFixMe[unclear-type] Add types for callback
     endCallback: Function,
     // $FlowFixMe[unclear-type] Add types for callback
@@ -90,6 +91,7 @@ export default class FSEventsWatcher extends EventEmitter {
       )
       .on('dir', FSEventsWatcher._normalizeProxy(dirCallback))
       .on('file', FSEventsWatcher._normalizeProxy(fileCallback))
+      .on('symlink', FSEventsWatcher._normalizeProxy(symlinkCallback))
       .on('error', errorCallback)
       .on('end', () => {
         endCallback();
@@ -133,14 +135,14 @@ export default class FSEventsWatcher extends EventEmitter {
     debug(`Watching ${this.root}`);
 
     this._tracked = new Set();
+    const trackPath = (filePath: string) => {
+      this._tracked.add(filePath);
+    };
     FSEventsWatcher._recReaddir(
       this.root,
-      (filepath: string) => {
-        this._tracked.add(filepath);
-      },
-      (filepath: string) => {
-        this._tracked.add(filepath);
-      },
+      trackPath,
+      trackPath,
+      trackPath,
       // $FlowFixMe[method-unbinding] - Refactor
       this.emit.bind(this, 'ready'),
       // $FlowFixMe[method-unbinding] - Refactor

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

@@ -70,7 +70,10 @@ module.exports = class NodeWatcher extends EventEmitter {
         this._watchdir(dir);
       },
       filename => {
-        this._register(filename);
+        this._register(filename, 'f');
+      },
+      symlink => {
+        this._register(symlink, 'l');
       },
       () => {
         this.emit('ready');
@@ -94,7 +97,7 @@ module.exports = class NodeWatcher extends EventEmitter {
    *
    *  Return false if ignored or already registered.
    */
-  _register(filepath: string): boolean {
+  _register(filepath: string, type: ChangeEventMetadata['type']): boolean {
     const dir = path.dirname(filepath);
     const filename = path.basename(filepath);
     if (this._dirRegistry[dir] && this._dirRegistry[dir][filename]) {
@@ -103,6 +106,7 @@ module.exports = class NodeWatcher extends EventEmitter {
 
     const relativePath = path.relative(this.root, filepath);
     if (
+      type === 'f' &&
       !common.isFileIncluded(this.globs, this.dot, this.doIgnore, relativePath)
     ) {
       return false;
@@ -173,7 +177,7 @@ module.exports = class NodeWatcher extends EventEmitter {
     watcher.on('error', this._checkedEmitError);
 
     if (this.root !== dir) {
-      this._register(dir);
+      this._register(dir, 'd');
     }
     return true;
   };
@@ -295,14 +299,27 @@ module.exports = class NodeWatcher extends EventEmitter {
               }
             },
             (file, stats) => {
-              if (this._register(file)) {
+              if (this._register(file, 'f')) {
                 this._emitEvent(ADD_EVENT, path.relative(this.root, file), {
                   modifiedTime: stats.mtime.getTime(),
                   size: stats.size,
                   type: 'f',
                 });
               }
             },
+            (symlink, stats) => {
+              if (this._register(symlink, 'l')) {
+                this._rawEmitEvent(
+                  ADD_EVENT,
+                  path.relative(this.root, symlink),
+                  {
+                    modifiedTime: stats.mtime.getTime(),
+                    size: stats.size,
+                    type: 'l',
+                  },
+                );
+              }
+            },
             function endCallback() {},
             this._checkedEmitError,
             this.ignored,
@@ -322,7 +339,7 @@ module.exports = class NodeWatcher extends EventEmitter {
         if (registered) {
           this._emitEvent(CHANGE_EVENT, relativePath, metadata);
         } else {
-          if (this._register(fullPath)) {
+          if (this._register(fullPath, type)) {
             this._emitEvent(ADD_EVENT, relativePath, metadata);
           }
         }

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

@@ -49,11 +49,12 @@ describe.each(Object.keys(WATCHERS))(
       await Promise.all([
         writeFile(join(watchRoot, 'existing', 'file-to-delete'), ''),
         writeFile(join(watchRoot, 'existing', 'file-to-modify'), ''),
+        symlink('target', join(watchRoot, 'existing', 'symlink-to-delete')),
       ]);
 
       // Short delay to ensure that 'add' events for the files above are not
       // reported by the OS to the watcher we haven't established yet.
-      await new Promise(resolve => setTimeout(resolve, 10));
+      await new Promise(resolve => setTimeout(resolve, 100));
 
       const opts: WatcherOptions = {
         dot: true,
@@ -74,8 +75,6 @@ describe.each(Object.keys(WATCHERS))(
     });
 
     beforeEach(async () => {
-      // Discard events before app add - sometimes pre-init events are reported
-      // after the watcher is ready.
       expect(await eventHelpers.nextEvent(() => mkdir(appRoot))).toStrictEqual({
         path: 'app',
         eventType: 'add',
@@ -181,6 +180,18 @@ describe.each(Object.keys(WATCHERS))(
       });
     });
 
+    maybeTest('detects deletion of a pre-existing symlink', async () => {
+      expect(
+        await eventHelpers.nextEvent(() =>
+          unlink(join(watchRoot, 'existing', 'symlink-to-delete')),
+        ),
+      ).toStrictEqual({
+        path: join('existing', 'symlink-to-delete'),
+        eventType: 'delete',
+        metadata: undefined,
+      });
+    });
+
     maybeTest('detects change to a pre-existing file as a change', async () => {
       expect(
         await eventHelpers.nextEvent(() =>

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

@@ -113,6 +113,7 @@ export function recReaddir(
   dir: string,
   dirCallback: (string, Stats) => void,
   fileCallback: (string, Stats) => void,
+  symlinkCallback: (string, Stats) => void,
   endCallback: () => void,
   errorCallback: Error => void,
   ignored: ?(boolean | RegExp),
@@ -121,6 +122,7 @@ export function recReaddir(
     .filterDir(currentDir => !anymatch(ignored, currentDir))
     .on('dir', normalizeProxy(dirCallback))
     .on('file', normalizeProxy(fileCallback))
+    .on('symlink', normalizeProxy(symlinkCallback))
     .on('error', errorCallback)
     .on('end', () => {
       if (platform === 'win32') {