fast_path: Fix handling of inputs with indirection fragments after ot…

…her path segments

Summary:
Fixes an internal correctness issue where `fast_path.relative` did not return normalised paths when inputs contained indirection fragments (`./`, `../`) in any case other than a contiguous sequence after the project root.

This preserves the optimised behaviour for paths like `/project/root/../../foo` (common when concatenating onto the project root), while falling back to Node for more exotic paths - `/project/root/./other/../etc`.

The performance difference is negligible, and `fastPath.relative` is still about 2x faster than Node's implementation for Metro's use case (and path.relative is hot during Metro startup as well as resolution, so I do think it's worth keeping `fast_path` around).

Changelog: Internal

Reviewed By: huntie

Differential Revision: D52365868

fbshipit-source-id: f7a1cea62113c6742612474a40ee70e5d25353cb

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

@@ -23,23 +23,49 @@ describe.each([['win32'], ['posix']])('fast_path on %s', platform => {
       : filePath;
 
   let fastPath: FastPath;
+  let pathRelative: JestMockFn<[string, string], string>;
 
   beforeEach(() => {
     jest.resetModules();
     mockPathModule = jest.requireActual<{}>('path')[platform];
+    pathRelative = jest.spyOn(mockPathModule, 'relative');
     fastPath = require('../fast_path');
   });
 
+  test.each([
+    p('/project/root/baz/foobar'),
+    p('/project/root/../root2/foobar'),
+    p('/project/root/../../project2/foo'),
+  ])(`relative('/project/root', '%s') is correct and optimised`, normalPath => {
+    const rootDir = p('/project/root');
+    const expected = mockPathModule.relative(rootDir, normalPath);
+    pathRelative.mockClear();
+    expect(fastPath.relative(rootDir, normalPath)).toEqual(expected);
+    expect(pathRelative).not.toHaveBeenCalled();
+  });
+
   describe.each([p('/project/root'), p('/')])('root: %s', rootDir => {
+    beforeEach(() => {
+      pathRelative.mockClear();
+    });
+
     test.each([
-      p('/project/root/baz/foobar'),
+      p('/project/root/../root2/../root3/foo'),
       p('/project/baz/foobar'),
       p('/project/rootfoo/baz'),
-    ])(`relative('${rootDir}', '%s') matches path.relative`, normalPath => {
-      expect(fastPath.relative(rootDir, normalPath)).toEqual(
-        mockPathModule.relative(rootDir, normalPath),
-      );
-    });
+      p('/project/root/./baz/foo/bar'),
+      p('/project/root/a./../foo'),
+      p('/project/root/../a./foo'),
+      p('/project/root/.././foo'),
+    ])(
+      `relative('${rootDir}', '%s') falls back to path.relative`,
+      normalPath => {
+        const expected = mockPathModule.relative(rootDir, normalPath);
+        pathRelative.mockClear();
+        expect(fastPath.relative(rootDir, normalPath)).toEqual(expected);
+        expect(pathRelative).toHaveBeenCalled();
+      },
+    );
 
     test.each([
       p('normal/path'),

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

@@ -13,22 +13,39 @@ import * as path from 'path';
 // rootDir must be normalized and absolute, filename may be any absolute path.
 // (but will optimally start with rootDir)
 export function relative(rootDir: string, filename: string): string {
-  return filename.indexOf(rootDir + path.sep) === 0
-    ? filename.substr(rootDir.length + 1)
-    : path.relative(rootDir, filename);
+  if (filename.indexOf(rootDir + path.sep) === 0) {
+    const relativePath = filename.substr(rootDir.length + 1);
+    // Allow any sequence of indirection fragments at the start of the path,
+    // e.g ../../foo, but bail out to Node's path.relative if we find a
+    // possible indirection after any other segment, or a leading "./".
+    for (let i = 0; ; i += UP_FRAGMENT_LENGTH) {
+      const nextIndirection = relativePath.indexOf(CURRENT_FRAGMENT, i);
+      if (nextIndirection === -1) {
+        return relativePath;
+      }
+      if (
+        nextIndirection !== i + 1 || // Fallback when ./ later in the path, or leading
+        relativePath[i] !== '.' // and for anything other than a leading ../
+      ) {
+        return path.relative(rootDir, filename);
+      }
+    }
+  }
+  return path.relative(rootDir, filename);
 }
 
-const INDIRECTION_FRAGMENT = '..' + path.sep;
-const INDIRECTION_FRAGMENT_LENGTH = INDIRECTION_FRAGMENT.length;
+const UP_FRAGMENT = '..' + path.sep;
+const UP_FRAGMENT_LENGTH = UP_FRAGMENT.length;
+const CURRENT_FRAGMENT = '.' + path.sep;
 
 // rootDir must be an absolute path and normalPath must be a normal relative
 // path (e.g.: foo/bar or ../foo/bar, but never ./foo or foo/../bar)
 // As of Node 18 this is several times faster than path.resolve, over
 // thousands of real calls with 1-3 levels of indirection.
 export function resolve(rootDir: string, normalPath: string): string {
-  if (normalPath.startsWith(INDIRECTION_FRAGMENT)) {
+  if (normalPath.startsWith(UP_FRAGMENT)) {
     const dirname = rootDir === '' ? '' : path.dirname(rootDir);
-    return resolve(dirname, normalPath.slice(INDIRECTION_FRAGMENT_LENGTH));
+    return resolve(dirname, normalPath.slice(UP_FRAGMENT_LENGTH));
   } else {
     return (
       rootDir +