fix(metro-file-map): Fix Windows cross-drive path handling (#1711)

kitten · meta-codesync[bot] · commit 6e6b52c4bd31 · 2026-05-12T12:38:21.000-07:00
Summary: Related PR for `expo/expo` fork: expo/expo#45648 Supersedes #1696 Cross-device path normals are intended to be relative paths from the rootDir. If the rootDir is at `C:\project` then a cross-device normal path at `D:\temp` is represented as `..\..\D:\temp`. This assumption is broken in several places, like `normalToAbsolute` printing, leading to `C:\D:\` paths that are invalid. Changelog: [Fix] Fix Windows path handling for cross-drive paths Pull Request resolved: #1711 Test Plan: - Unit tests were adjusted to demonstrate all failing cases - **Note:** Invalid unit tests have been removed for paths that rise above the filesystem root (they're assumed to be impossible, and their output doesn't have to be deterministic/correct) Reviewed By: robhogan Differential Revision: D104700077 Pulled By: CalixTang fbshipit-source-id: e77baa4e66495b50ef41b260cec60bcc832d4a6a
diff --git a/packages/metro-file-map/src/lib/RootPathUtils.js b/packages/metro-file-map/src/lib/RootPathUtils.js
@@ -46,6 +46,9 @@ const SEP_UP_FRAGMENT = path.sep + '..';
 const UP_FRAGMENT_SEP_LENGTH = UP_FRAGMENT_SEP.length;
 const CURRENT_FRAGMENT = '.' + path.sep;
 
+const IS_WIN32 = path.sep === '\\';
+const ROOT_BASE_IDX = IS_WIN32 ? 0 : 1;
+
 export class RootPathUtils {
   #rootDir: string;
   #rootDirnames: ReadonlyArray<string>;
@@ -149,6 +152,12 @@ export class RootPathUtils {
     const right = pos === 0 ? normalPath : normalPath.slice(pos);
     if (right.length === 0) {
       return left;
+    } else if (IS_WIN32 && pos > this.#rootDepth * UP_FRAGMENT_SEP_LENGTH) {
+      // On a real file system, navigating to `..` at the top level (posix `/`
+      // or Windows drive) is a no-op, but we can't respect that on Windows
+      // because Metro uses e.g. `..\..\D:\foo` to represent cross-drive
+      // relative paths.
+      return right;
     }
     // left may already end in a path separator only if it is a filesystem root,
     // '/' or 'X:\'.
@@ -198,7 +207,9 @@ export class RootPathUtils {
     if (relativePath === '') {
       return {collapsedSegments: 0, normalPath};
     }
-    const left = normalPath + path.sep;
+    const left = normalPath.endsWith(path.sep)
+      ? normalPath
+      : normalPath + path.sep;
     const rawPath = left + relativePath;
     if (normalPath === '..' || normalPath.endsWith(SEP_UP_FRAGMENT)) {
       const collapsed = this.#tryCollapseIndirectionsInSuffix(rawPath, 0, 0);
@@ -299,9 +310,10 @@ export class RootPathUtils {
         };
       }
 
-      // Cap the number of indirections at the total number of root segments.
-      // File systems treat '..' at the root as '.'.
-      if (totalUpIndirections < this.#rootParts.length - 1) {
+      // Cap the number of indirections at the total number of root parts.
+      // File systems treat '..' at the root as '.'. For Windows, cross-device
+      // paths need to survive this.
+      if (totalUpIndirections < this.#rootParts.length - ROOT_BASE_IDX) {
         totalUpIndirections++;
       }
 
diff --git a/packages/metro-file-map/src/lib/TreeFS.js b/packages/metro-file-map/src/lib/TreeFS.js
@@ -493,6 +493,13 @@ export default class TreeFS implements MutableFileSystem {
 
   remove(mixedPath: Path, changeListener?: FileSystemListener): void {
     const normalPath = this.#normalizePath(mixedPath);
+    this.#removeNormalPath(normalPath, changeListener);
+  }
+
+  #removeNormalPath(
+    normalPath: string,
+    changeListener?: FileSystemListener,
+  ): void {
     const result = this.#lookupByNormalPath(normalPath, {followLeaf: false});
     if (!result.exists) {
       return;
@@ -501,7 +508,10 @@ export default class TreeFS implements MutableFileSystem {
 
     if (isDirectory(node) && node.size > 0) {
       for (const basename of node.keys()) {
-        this.remove(canonicalPath + path.sep + basename, changeListener);
+        this.#removeNormalPath(
+          canonicalPath + path.sep + basename,
+          changeListener,
+        );
       }
       // Removing the last file will delete this directory
       return;
@@ -521,7 +531,7 @@ export default class TreeFS implements MutableFileSystem {
         // that's not expected to be a case common enough to justify
         // implementation complexity, or slowing down more common uses of
         // _lookupByNormalPath.
-        this.remove(path.dirname(canonicalPath), changeListener);
+        this.#removeNormalPath(path.dirname(canonicalPath), changeListener);
       }
     }
   }
@@ -1224,16 +1234,19 @@ export default class TreeFS implements MutableFileSystem {
       typeof literalSymlinkTarget === 'string',
       'Expected symlink target to be populated.',
     );
-    const absoluteSymlinkTarget = path.resolve(
+    let absoluteSymlinkTarget = path.resolve(
       this.#rootDir,
       canonicalPathOfSymlink,
       '..', // Symlink target is relative to its containing directory.
       literalSymlinkTarget, // May be absolute, in which case the above are ignored
     );
-    const normalSymlinkTarget = path.relative(
-      this.#rootDir,
+    if (absoluteSymlinkTarget.endsWith(path.sep)) {
+      absoluteSymlinkTarget = absoluteSymlinkTarget.slice(0, -1);
+    }
+    const normalSymlinkTarget = this.#pathUtils.absoluteToNormal(
       absoluteSymlinkTarget,
     );
+
     const result = {
       ancestorOfRootIdx:
         this.#pathUtils.getAncestorOfRootIdx(normalSymlinkTarget),
diff --git a/packages/metro-file-map/src/lib/__tests__/RootPathUtils-test.js b/packages/metro-file-map/src/lib/__tests__/RootPathUtils-test.js
@@ -91,37 +91,59 @@ describe.each([['win32'], ['posix']])('RootPathUtils on %s', platform => {
       expect(pathRelative).toHaveBeenCalled();
     });
 
-    test.each([
-      p('..'),
-      p('../..'),
-      p('../../'),
-      p('normal/path'),
-      p('normal/path/'),
-      p('../normal/path'),
-      p('../normal/path/'),
-      p('../../normal/path'),
-      p('../../../normal/path'),
-    ])(`normalToAbsolute('%s') matches path.resolve`, normalPath => {
-      let expected = mockPathModule.resolve(rootDir, normalPath);
-      // Unlike path.resolve, we expect to preserve trailing separators.
-      if (normalPath.endsWith(sep) && !expected.endsWith(sep)) {
-        expected += sep;
-      }
-      expect(pathUtils.normalToAbsolute(normalPath)).toEqual(expected);
-    });
+    const normalToAbsoluteInputs =
+      rootDir === p('/project/root')
+        ? [
+            p('..'),
+            p('../..'),
+            p('../../'),
+            p('normal/path'),
+            p('normal/path/'),
+            p('../normal/path'),
+            p('../normal/path/'),
+            p('../../normal/path'),
+            // On POSIX, `..` at the root re-enters the root
+            ...(platform === 'posix' ? [p('../../../normal/path')] : []),
+          ]
+        : [
+            p('..'),
+            p('../..'),
+            p('../../'),
+            p('normal/path'),
+            p('normal/path/'),
+          ];
 
-    test.each([
-      p('..'),
-      p('../root'),
-      p('../root/path'),
-      p('../project'),
-      p('../project/'),
-      p('../../project/root'),
-      p('../../project/root/'),
-      p('../../../normal/path'),
-      p('../../../normal/path/'),
-      p('../../..'),
-    ])(
+    test.each(normalToAbsoluteInputs)(
+      `normalToAbsolute('%s') matches path.resolve`,
+      normalPath => {
+        let expected = mockPathModule.resolve(rootDir, normalPath);
+        // Unlike path.resolve, we expect to preserve trailing separators.
+        if (normalPath.endsWith(sep) && !expected.endsWith(sep)) {
+          expected += sep;
+        }
+        expect(pathUtils.normalToAbsolute(normalPath)).toEqual(expected);
+      },
+    );
+
+    const relativeToNormalInputs =
+      rootDir === p('/project/root')
+        ? [
+            p('..'),
+            p('../root'),
+            p('../root/path'),
+            p('../project'),
+            p('../project/'),
+            p('../../project/root'),
+            p('../../project/root/'),
+            p('../../..'),
+            // On POSIX, `..` at the root re-enters the root
+            ...(platform === 'posix'
+              ? [p('../../../normal/path'), p('../../../normal/path/')]
+              : []),
+          ]
+        : [p('..')];
+
+    test.each(relativeToNormalInputs)(
       `relativeToNormal('%s') matches path.resolve + path.relative`,
       relativePath => {
         let expected = mockPathModule.relative(
@@ -153,4 +175,75 @@ describe.each([['win32'], ['posix']])('RootPathUtils on %s', platform => {
   ])('getAncestorOfRootIdx (%s => %s)', (input, expected) => {
     expect(pathUtils.getAncestorOfRootIdx(input)).toEqual(expected);
   });
+
+  if (platform === 'win32') {
+    describe('cross-drive absolute paths (Windows)', () => {
+      test.each([['C:\\project\\root'], ['C:\\']])(
+        'path.relative returns cross-drive target as-is from rootDir=%s',
+        rootDir => {
+          expect(mockPathModule.relative(rootDir, 'D:\\some\\file.js')).toEqual(
+            'D:\\some\\file.js',
+          );
+        },
+      );
+
+      test.each([
+        [
+          'C:\\project\\root',
+          'D:\\some\\file.js',
+          '..\\..\\..\\D:\\some\\file.js',
+        ],
+        ['C:\\project\\root', 'D:\\some\\', '..\\..\\..\\D:\\some\\'],
+        ['C:\\project\\root', 'D:\\', '..\\..\\..\\D:\\'],
+        ['C:\\', 'D:\\some\\file.js', '..\\D:\\some\\file.js'],
+        ['C:\\', 'D:\\', '..\\D:\\'],
+        ['D:\\project\\root', 'C:\\file.js', '..\\..\\..\\C:\\file.js'],
+      ])(
+        'absoluteToNormal emits a ..-chain (rootDir=%s, X=%s -> %s)',
+        (rootDir, absolutePath, expectedNormal) => {
+          pathUtils = new RootPathUtils(rootDir);
+          expect(pathUtils.absoluteToNormal(absolutePath)).toEqual(
+            expectedNormal,
+          );
+        },
+      );
+
+      test.each([
+        ['C:\\project\\root', 'D:\\some\\file.js'],
+        ['C:\\project\\root', 'D:\\some\\'],
+        ['C:\\project\\root', 'D:\\'],
+        ['C:\\', 'D:\\some\\file.js'],
+        ['C:\\', 'D:\\some\\'],
+        ['C:\\', 'D:\\'],
+        ['D:\\project\\root', 'C:\\file.js'],
+        ['D:\\project\\root', 'C:\\'],
+      ])(
+        'normalToAbsolute(absoluteToNormal(X)) === X for rootDir=%s, X=%s',
+        (rootDir, absolutePath) => {
+          pathUtils = new RootPathUtils(rootDir);
+          const normal = pathUtils.absoluteToNormal(absolutePath);
+          expect(pathUtils.normalToAbsolute(normal)).toEqual(absolutePath);
+        },
+      );
+
+      test.each([
+        ['C:\\project\\root', 'D:\\dir\\sub', 'extra\\file.js'],
+        ['C:\\project\\root', 'D:\\', 'foo.js'],
+        ['C:\\', 'D:\\dir', 'sub\\file.js'],
+      ])(
+        'joinNormalToRelative round-trips cross-drive (rootDir=%s, base=%s, rel=%s)',
+        (rootDir, baseAbsolute, relativePath) => {
+          pathUtils = new RootPathUtils(rootDir);
+          const baseNormal = pathUtils.absoluteToNormal(baseAbsolute);
+          const {normalPath} = pathUtils.joinNormalToRelative(
+            baseNormal,
+            relativePath,
+          );
+          expect(pathUtils.normalToAbsolute(normalPath)).toEqual(
+            mockPathModule.join(baseAbsolute, relativePath),
+          );
+        },
+      );
+    });
+  }
 });
diff --git a/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js b/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js
@@ -1235,4 +1235,85 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
       });
     });
   });
+
+  if (platform === 'win32') {
+    describe('cross-drive paths (Windows)', () => {
+      let tfsCD: TreeFSType;
+      const externalMeta: FileMetadata = [123, 4, 0, null, 0, 'external'];
+
+      beforeEach(() => {
+        tfsCD = new TreeFS({
+          rootDir: 'C:\\project',
+          files: new Map<CanonicalPath, FileMetadata>([
+            ['bar.js', [234, 3, 0, null, 0, 'bar']],
+            ['..\\..\\D:\\external\\file.js', externalMeta],
+          ]),
+          processFile: () => {
+            throw new Error('Not implemented');
+          },
+        });
+      });
+
+      test('exists() finds a seeded cross-drive file', () => {
+        expect(tfsCD.exists('D:\\external\\file.js')).toBe(true);
+      });
+
+      test('lookup() returns the absolute drive-prefixed path as realPath', () => {
+        expect(tfsCD.lookup('D:\\external\\file.js')).toMatchObject({
+          exists: true,
+          type: 'f',
+          realPath: 'D:\\external\\file.js',
+        });
+      });
+
+      test('getAllFiles() enumerates cross-drive and in-tree files side by side', () => {
+        expect(tfsCD.getAllFiles().sort()).toEqual([
+          'C:\\project\\bar.js',
+          'D:\\external\\file.js',
+        ]);
+      });
+
+      test('addOrModify() accepts a new cross-drive absolute path', () => {
+        tfsCD.addOrModify('D:\\added\\later.js', [1, 1, 0, null, 0, 'later']);
+        expect(tfsCD.exists('D:\\added\\later.js')).toBe(true);
+        expect(tfsCD.lookup('D:\\added\\later.js')).toMatchObject({
+          exists: true,
+          type: 'f',
+          realPath: 'D:\\added\\later.js',
+        });
+      });
+
+      test('remove() deletes a cross-drive entry and prunes empty ancestor dirs', () => {
+        tfsCD.remove('D:\\external\\file.js');
+        expect(tfsCD.exists('D:\\external\\file.js')).toBe(false);
+        expect(tfsCD.lookup('D:\\external').exists).toBe(false);
+        expect(tfsCD.exists('C:\\project\\bar.js')).toBe(true);
+      });
+
+      test('lookup() reports missing for non-existent cross-drive path', () => {
+        expect(tfsCD.lookup('D:\\external\\missing.js')).toMatchObject({
+          exists: false,
+        });
+        expect(tfsCD.exists('E:\\anywhere.js')).toBe(false);
+      });
+
+      test('lookup() follows a symlink whose target is a cross-drive path', () => {
+        const tfsLink = new TreeFS({
+          rootDir: 'C:\\project',
+          files: new Map<CanonicalPath, FileMetadata>([
+            ['..\\..\\D:\\external\\file.js', externalMeta],
+            ['link', [0, 0, 0, null, 'D:\\external\\file.js', null]],
+          ]),
+          processFile: () => {
+            throw new Error('Not implemented');
+          },
+        });
+        expect(tfsLink.lookup('C:\\project\\link')).toMatchObject({
+          exists: true,
+          type: 'f',
+          realPath: 'D:\\external\\file.js',
+        });
+      });
+    });
+  }
 });
