metro-file-map: Fix regressions with symlinks and unnormalised roots

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

@@ -19,6 +19,7 @@ import type {
 import type FileMapT from '../index';
 
 import {AbstractWatcher} from '../watchers/AbstractWatcher';
+import fs from 'node:fs';
 import * as path from 'node:path';
 
 jest.useRealTimers();
@@ -106,7 +107,11 @@ const EXTERNAL_DIR = path.resolve('/', 'external');
 const EXTERNAL_FILE = path.join(EXTERNAL_DIR, 'lib', 'helper.js');
 
 function createFileMap(
-  opts?: Readonly<{enableSymlinks?: boolean, watch?: boolean}>,
+  opts?: Readonly<{
+    enableSymlinks?: boolean,
+    roots?: ReadonlyArray<string>,
+    watch?: boolean,
+  }>,
 ): FileMapT {
   return new FileMap({
     cacheManagerFactory: () => ({
@@ -130,7 +135,7 @@ function createFileMap(
     resetCache: false,
     retainAllFiles: true,
     rootDir: ROOT_DIR,
-    roots: [SRC_DIR],
+    roots: opts?.roots ?? [SRC_DIR],
     useWatchman: false,
     watch: opts?.watch ?? false,
   });
@@ -162,6 +167,7 @@ describe('FileMap.addRoot', () => {
   });
 
   afterEach(async () => {
+    jest.restoreAllMocks();
     await fileMap.end();
   });
 
@@ -356,6 +362,47 @@ describe('FileMap.addRoot', () => {
     expect(fileSystem.getAllFiles()).toEqual([path.join(SRC_DIR, 'index.js')]);
   });
 
+  test('keeps the files of a root given through a symlink in an instance built from the cache', async () => {
+    const linkedRoot = path.join(ROOT_DIR, 'node_modules', 'linked');
+    const realFile = path.join(EXTERNAL_DIR, 'lib', 'helper.js');
+    jest
+      .spyOn(fs.promises, 'realpath')
+      .mockImplementation(async (root: string) => {
+        if (root === linkedRoot) {
+          return path.join(EXTERNAL_DIR, 'lib');
+        }
+        throw new Error(`ENOENT: no such file or directory, '${root}'`);
+      });
+    // Like Watchman, list a root's files under its real path.
+    mockNodeCrawler.mockImplementationOnce(options =>
+      Promise.resolve(
+        options.previousState.fileSystem.getDifference(
+          new Map([
+            [
+              path.relative(ROOT_DIR, path.join(SRC_DIR, 'index.js')),
+              [1, 42, 0, null, 0],
+            ],
+            [path.relative(ROOT_DIR, realFile), [1, 42, 0, null, 0]],
+          ]),
+        ),
+      ),
+    );
+    const roots = [SRC_DIR, linkedRoot];
+    fileMap = createFileMap({roots});
+    await fileMap.build();
+    await fileMap.end();
+
+    // Like Watchman, report only changes since the cache.
+    mockNodeCrawler.mockImplementationOnce(() =>
+      Promise.resolve({changedFiles: new Map(), removedFiles: new Set()}),
+    );
+    fileMap = createFileMap({roots});
+    const {fileSystem} = await fileMap.build();
+
+    expect(fileSystem.exists(realFile)).toBe(true);
+    expect(fileSystem.exists(path.join(SRC_DIR, 'index.js'))).toBe(true);
+  });
+
   describe('in watch mode', () => {
     beforeEach(() => {
       fileMap = createFileMap({watch: true});

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

@@ -9,7 +9,7 @@
  * @oncall react_native
  */
 
-import type {FileMetadata} from '../flow-types';
+import type {CacheData, FileMetadata} from '../flow-types';
 import type {
   Crawler,
   CrawlerFactoryOptions,
@@ -21,6 +21,7 @@ import type {
 import typeof * as AbstractWatcherModule from '../watchers/AbstractWatcher';
 
 import FileMap, {HastePlugin, NoopCacheManager} from '../index';
+import fs from 'node:fs';
 import * as path from 'node:path';
 
 // Hoisted above the imports by jest, so the backend class has to be built
@@ -172,6 +173,29 @@ describe('crawlerFactory', () => {
     await fileMap.end();
   });
 
+  test('takes its roots as given on a cache hit, without reading the disk', async () => {
+    let cacheContent: ?CacheData = null;
+    const cacheManagerFactory = () => ({
+      end: async () => {},
+      read: async () => cacheContent,
+      write: async (getSnapshot: () => CacheData) => {
+        cacheContent = getSnapshot();
+      },
+    });
+    const crawlerFactory = () => stubCrawler([path.join('src', 'index.js')]);
+    await createFileMap({cacheManagerFactory, crawlerFactory}).build();
+    const realpath = jest.spyOn(fs.promises, 'realpath');
+
+    const {fileSystem} = await createFileMap({
+      cacheManagerFactory,
+      crawlerFactory,
+    }).build();
+
+    expect(realpath).not.toHaveBeenCalled();
+    expect(fileSystem.exists(p('src', 'index.js'))).toBe(true);
+    realpath.mockRestore();
+  });
+
   test('failures are fatal, with no fallback to the node crawler', async () => {
     // Unlike a Watchman failure, there is no sensible crawler to fall back to -
     // silently crawling the filesystem instead would defeat the point.

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

@@ -215,6 +215,8 @@ jest.mock('fs', () => ({
       }
       return entry.link;
     }),
+    // No root in the mock file system is a symlink.
+    realpath: jest.fn(async path => path),
   },
 }));
 jest.mock('node:fs', () => jest.requireMock('fs'));

packages/metro-file-map/src/crawlers/__tests__/node-test.js

@@ -373,6 +373,28 @@ describe.each([['win32'], ['posix']])('node crawler on %s', platform => {
     );
   });
 
+  test.each([
+    ['a trailing separator', '/project/fruits/'],
+    ["'..' segments", '/project/vegetables/../fruits'],
+  ])('crawls a root with %s by its normal path', async (_, root) => {
+    nodeCrawl = require('../node').default;
+    const ignore = jest.fn(pearMatcher);
+
+    const {changedFiles} = await nodeCrawl({
+      console: global.console,
+      previousState: {fileSystem: emptyFS},
+      extensions: ['js'],
+      ignore,
+      rootDir,
+      roots: [p(root)],
+    });
+
+    expect([...changedFiles.keys()].sort()).toEqual(
+      ['fruits/directory/strawberry.js', 'fruits/tomato.js'].map(p),
+    );
+    expect(ignore).toHaveBeenCalledWith(p('/project/fruits/directory'));
+  });
+
   test('crawls from a filesystem root without doubling separators', async () => {
     const fs = require('node:fs');
     nodeCrawl = require('../node').default;

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

@@ -101,7 +101,9 @@ function find(options: CrawlerOptions): Promise<FileData> {
       });
     }
 
-    for (const root of roots) {
+    for (const givenRoot of roots) {
+      // Paths are built by concatenation, so the root must be normalised.
+      const root = path.resolve(givenRoot);
       const rootNormal = pathUtils.absoluteToNormal(root);
       const isWithinRoot =
         rootNormal !== '..' && !rootNormal.startsWith('..' + path.sep);

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

@@ -536,11 +536,22 @@ export default class FileMap extends EventEmitter {
 
         // The cache key does not include roots added by `addRoot`, so the
         // cache may hold files from an earlier instance's added roots. Drop
-        // them, including from plugins.
+        // them, including from plugins. Watchman lists a symlinked root's
+        // files under its real path. A custom crawler's files may not be on
+        // disk.
         if (initialData != null) {
-          for (const canonicalPath of fileSystem.pathsOutsideRoots(
-            this.#options.roots,
-          )) {
+          const realRoots =
+            this.#crawler == null
+              ? await Promise.all(
+                  this.#options.roots.map(root =>
+                    fsPromises.realpath(root).catch(() => root),
+                  ),
+                )
+              : [];
+          for (const canonicalPath of fileSystem.pathsOutsideRoots([
+            ...this.#options.roots,
+            ...realRoots,
+          ])) {
             fileDelta.removedFiles.add(canonicalPath);
           }
         }

packages/metro-file-map/src/lib/TreeFS.js

@@ -39,6 +39,19 @@ function isRegularFile(node: FileNode): boolean {
   return node[H.SYMLINK] === 0;
 }
 
+// Null if not within the ancestor ('' is the root).
+function relativeToAncestor(
+  normalPath: string,
+  ancestorNormalPath: string,
+): ?string {
+  if (ancestorNormalPath === '' || normalPath === ancestorNormalPath) {
+    return ancestorNormalPath === '' ? normalPath : '';
+  }
+  return normalPath.startsWith(ancestorNormalPath + path.sep)
+    ? normalPath.slice(ancestorNormalPath.length + 1)
+    : null;
+}
+
 type DeserializedSnapshotInput = {
   rootDir: string,
   fileSystemData: DirectoryNode,
@@ -1075,8 +1088,26 @@ export default class TreeFS implements MutableFileSystem {
 
     // This will be '', '..', '../..', etc.
     let candidateNormalPath =
-      commonRootDepth > 0 ? normalPath.slice(0, 3 * commonRootDepth - 1) : '';
-    const remainingNormalPath = normalPath.slice(commonRootDepth * 3);
+      commonRootDepth > 0
+        ? '..' + (path.sep + '..').repeat(commonRootDepth - 1)
+        : '';
+    // By its real path, if the start path reaches the common root through a
+    // symlink.
+    const remainingNormalPath =
+      relativeToAncestor(normalPath, candidateNormalPath) ??
+      relativeToAncestor(
+        closestLookup.exists
+          ? closestLookup.canonicalPath
+          : closestLookup.remainingPath === ''
+            ? closestLookup.canonicalMissingPath
+            : closestLookup.canonicalMissingPath +
+              path.sep +
+              closestLookup.remainingPath,
+        candidateNormalPath,
+      );
+    if (remainingNormalPath == null) {
+      return null;
+    }
 
     let nextNode: ?MixedNode = commonRoot;
     let depthBelowCommonRoot = 0;

packages/metro-file-map/src/lib/__tests__/TreeFS-test.js

@@ -590,6 +590,7 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
               [p('a/b/c/d/link-to-C'), [0, 0, 0, null, '', null]],
               [p('a/b/c/d/link-to-B'), [0, 0, 0, null, '..', null]],
               [p('a/b/c/d/link-to-A'), [0, 0, 0, null, '../..', null]],
+              [p('link-to-B'), [0, 0, 0, null, '..', null]],
               [
                 p('n_m/workspace/link-to-pkg'),
                 [0, 0, 0, null, '../workspace-pkg', null],
@@ -760,6 +761,14 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
         ['/A/B/C/foo.js', '/A/B/C/package.json', '/A/B/package.json'],
         ['/A/B/C/a/b/c/d/link-to-A'],
       ],
+      // A symlink in the root to an ancestor of the root
+      [
+        '/A/B/C/link-to-B/foo.js',
+        '/A/package.json',
+        'B/foo.js',
+        ['/A/B/foo.js', '/A/B/package.json'],
+        ['/A/B/C/link-to-B'],
+      ],
       [
         '/A/B/C/a/1/foo.js',
         '/A/B/C/a/1/real-package.json',