From f1d5cb3e1bc566614bcdad7489cd475f177d590e Mon Sep 17 00:00:00 2001
From: Rob Hogan <rob@rhogan.net>
Date: Sat, 7 Sep 2024 15:16:58 -0700
Subject: [PATCH] Fix symlinked packages not respecting package.json
 redirections (#1349)

Summary:
`TreeFS.hierarchicalLookup` is a recently introduced function for finding closest package.json for an input path.

It works by traversing to the input path while "collecting ancestors" - i.e. all of the directory nodes on route to the input path, and then checking each of them in reverse order for package.json.

This gets a bit complicated when traversal includes symlinks. For the test case in this commit:

- `n_m/workspace/link-to-pkg` is a symlink to '../workspace-pkg'.
- When resolving the package scope of `n_m/workspace/link-to-pkg/subpath`, we should check:
 - `n_m/workspace/link-to-pkg` (realpath: `../workspace-pkg`)
 - `n_m/workspace` (realpath: `n_m/workspace`)

In particular, when traversing to `../workspace-pkg`, we should *not* collect `..` but we *should* collect `../workspace-pkg`.

We attempt to do this by keeping track of an `unseenPathFromIdx`, which lets us skip ancestor collection when we're traversing from the root to the symlink target.

The logic here was effectively off-by-one - we were correctly skipping segments before the symlink target, and correctly collecting segments after, but we missed the symlink target itself.

By setting `unseenPathFromIdx` to the index of the start of the target's basename, we include it correctly.

Fixes https://github.com/facebook/metro/issues/1347

Changelog:
```
- **[Fix]**: Fix https://github.com/facebook/metro/issues/1347, symlinked packages not respecting package.json redirections.
```

Pull Request resolved: https://github.com/facebook/metro/pull/1349

Test Plan:
- New unit test
 - Verified e2e by patching metro-file-map in user's repro in https://github.com/facebook/metro/issues/1347

Reviewed By: cortinico

Differential Revision: D62343439

Pulled By: robhogan

fbshipit-source-id: a823601daeffdc8dfd5b5bc3987b75889d44588f
---
 packages/metro-file-map/src/lib/TreeFS.js          | 14 ++++++++++----
 .../src/lib/__tests__/TreeFS-test.js               | 11 +++++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/packages/metro-file-map/src/lib/TreeFS.js b/packages/metro-file-map/src/lib/TreeFS.js
index 28971ff32..7ddfd4583 100644
--- a/packages/metro-file-map/src/lib/TreeFS.js
+++ b/packages/metro-file-map/src/lib/TreeFS.js
@@ -35,7 +35,11 @@ function isRegularFile(node: FileNode): boolean {
   return node[H.SYMLINK] === 0;
 }
 
-type NormalizedSymlinkTarget = {ancestorOfRootIdx: ?number, normalPath: string};
+type NormalizedSymlinkTarget = {
+  ancestorOfRootIdx: ?number,
+  normalPath: string,
+  startOfBasenameIdx: number,
+};
 
 /**
  * OVERVIEW:
@@ -667,9 +671,10 @@ export default class TreeFS implements MutableFileSystem {
         }
 
         // For the purpose of collecting ancestors: Ignore the traversal to
-        // the symlink target, and start collecting ancestors only when we
-        // reach the remaining part of the path.
-        unseenPathFromIdx = normalSymlinkTarget.normalPath.length;
+        // the symlink target, and start collecting ancestors only
+        // from the target itself (ie, the basename of the normal target path)
+        // onwards.
+        unseenPathFromIdx = normalSymlinkTarget.startOfBasenameIdx;
 
         if (seen == null) {
           // Optimisation: set this lazily only when we've encountered a symlink
@@ -1122,6 +1127,7 @@ export default class TreeFS implements MutableFileSystem {
       ancestorOfRootIdx:
         this.#pathUtils.getAncestorOfRootIdx(normalSymlinkTarget),
       normalPath: normalSymlinkTarget,
+      startOfBasenameIdx: normalSymlinkTarget.lastIndexOf(path.sep) + 1,
     };
     this.#cachedNormalSymlinkTargets.set(symlinkNode, result);
     return result;
diff --git a/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js b/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js
index 708855a95..1964a14e8 100644
--- a/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js
+++ b/packages/metro-file-map/src/lib/__tests__/TreeFS-test.js
@@ -314,6 +314,10 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
               p('a/b/c/d/link-to-A'),
               ['', 0, 0, 0, '', '', p('../../../../../..')],
             ],
+            [
+              p('n_m/workspace/link-to-pkg'),
+              ['', 0, 0, 0, '', '', p('../../../workspace-pkg')],
+            ],
           ].concat(
             [
               'a/package.json',
@@ -331,6 +335,7 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
               'a/n_m/pkg/n_m/pkg2/package.json',
               '../../package.json',
               '../../../a/b/package.json',
+              '../workspace-pkg/package.json',
             ].map(posixPath => [p(posixPath), ['', 0, 0, 0, '', '', 0]]),
           ),
         ),
@@ -447,6 +452,12 @@ describe.each([['win32'], ['posix']])('TreeFS on %s', platform => {
       ['/A/B/C/a/n_m/pkg3/foo.js', null, null, ['/A/B/C/a/n_m/pkg3']],
       // Does not look beyond n_m, if n_m does not exist
       ['/A/B/C/a/b/n_m/pkg/foo', null, null, ['/A/B/C/a/b/n_m']],
+      [
+        '/A/B/C/n_m/workspace/link-to-pkg/subpath',
+        '/A/B/workspace-pkg/package.json',
+        'subpath',
+        ['/A/B/C/n_m/workspace/link-to-pkg', '/A/B/workspace-pkg/subpath'],
+      ],
     ])(
       '%s => %s (relative %s, invalidatedBy %s)',
       (