Watcher - don't emit symlink events if enableSymlinks: false

Summary:
D42454225 (b1bef3d) introduced whole-graph invalidation on symlink changes, with the intention being that this would only occur if `resolver.unstable_enableSymlinks` was `true`. Unfortunately the event suppression added alongside it in D42454225 (b1bef3d) was broken, due to use of `type` (change type) vs `metadata.type` (file type).

Additionally, the previous suppression came too late in file processing - so that even if it had used `metadata.type`, a symlink would be added to the `FileSystem` instance even though no change would be emitted to Metro. This could've caused existence checks to succeed that should fail with `enableSymlinks: false` (and would fail after a recrawl, because crawlers exclude symlinks).

This diff moves symlink event detection up to the start of `onChange` processing, and returns early.

Changelog:
* **Fix**: Don't over-invalidate on symlink changes if resolution through symlinks is not enabled

Reviewed By: motiz88

Differential Revision: D43620608

fbshipit-source-id: 4e63258b246308766adf97beef3f7082caab6269

Author: robhogan

packages/metro-file-map/src/__tests__/index-test.js

@@ -1552,6 +1552,74 @@ describe('HasteMap', () => {
       expect(eventsQueue).toHaveLength(1);
     });
 
+    hm_it(
+      'suppresses backend symlink events if enableSymlinks: false',
+      async hm => {
+        const {fileSystem} = await hm.build();
+        const fruitsRoot = path.join('/', 'project', 'fruits');
+        const e = mockEmitters[fruitsRoot];
+        mockFs[path.join(fruitsRoot, 'Tomato.js')] = `
+        // Tomato!
+      `;
+        e.emit('all', 'change', 'Tomato.js', fruitsRoot, MOCK_CHANGE_FILE);
+        e.emit(
+          'all',
+          'change',
+          'LinkToStrawberry.js',
+          fruitsRoot,
+          MOCK_CHANGE_LINK,
+        );
+        const {eventsQueue} = await waitForItToChange(hm);
+        expect(eventsQueue).toEqual([
+          {
+            filePath: path.join(fruitsRoot, 'Tomato.js'),
+            metadata: MOCK_CHANGE_FILE,
+            type: 'change',
+          },
+        ]);
+        expect(
+          fileSystem.linkStats(path.join(fruitsRoot, 'LinkToStrawberry.js')),
+        ).toBeNull();
+      },
+    );
+
+    hm_it(
+      'emits symlink events if enableSymlinks: true',
+      async hm => {
+        const {fileSystem} = await hm.build();
+        const fruitsRoot = path.join('/', 'project', 'fruits');
+        const e = mockEmitters[fruitsRoot];
+        mockFs[path.join(fruitsRoot, 'Tomato.js')] = `
+        // Tomato!
+      `;
+        e.emit('all', 'change', 'Tomato.js', fruitsRoot, MOCK_CHANGE_FILE);
+        e.emit(
+          'all',
+          'change',
+          'LinkToStrawberry.js',
+          fruitsRoot,
+          MOCK_CHANGE_LINK,
+        );
+        const {eventsQueue} = await waitForItToChange(hm);
+        expect(eventsQueue).toEqual([
+          {
+            filePath: path.join(fruitsRoot, 'Tomato.js'),
+            metadata: MOCK_CHANGE_FILE,
+            type: 'change',
+          },
+          {
+            filePath: path.join(fruitsRoot, 'LinkToStrawberry.js'),
+            metadata: MOCK_CHANGE_LINK,
+            type: 'change',
+          },
+        ]);
+        expect(
+          fileSystem.linkStats(path.join(fruitsRoot, 'LinkToStrawberry.js')),
+        ).toEqual({fileType: 'l', modifiedTime: 46});
+      },
+      {config: {enableSymlinks: true}},
+    );
+
     hm_it(
       'emits a change even if a file in node_modules has changed',
       async hm => {

packages/metro-file-map/src/index.js

@@ -943,7 +943,9 @@ export default class HasteMap extends EventEmitter {
         // Ignore all directory events
         (metadata.type === 'd' ||
           // Ignore regular files with unwatched extensions
-          (metadata.type === 'f' && !hasWatchedExtension(filePath)))
+          (metadata.type === 'f' && !hasWatchedExtension(filePath)) ||
+          // Don't emit events relating to symlinks if enableSymlinks: false
+          (!this._options.enableSymlinks && metadata?.type === 'l'))
       ) {
         return;
       }
@@ -993,11 +995,6 @@ export default class HasteMap extends EventEmitter {
           const linkStats = fileSystem.linkStats(relativeFilePath);
 
           const enqueueEvent = (metadata: ChangeEventMetadata) => {
-            // Don't emit events relating to symlinks if enableSymlinks: false
-            if (!this._options.enableSymlinks && type === 'l') {
-              return null;
-            }
-
             eventsQueue.push({
               filePath: absoluteFilePath,
               metadata,