[0.80.11] breaks package exports with symlinks #1347
Labels
Comments
|
Huge thanks for the report @franksmule - confirmed this is a bug with the new Working on it... |
robhogan
added a commit
that referenced
this issue
Sep 7, 2024
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 #1347 ``` - **[Fix]**: Fix #1347, failure during resolution to read package.json of symlinked workspace packages for exports or redirects. ``` Test plan: - New unit test - Verified e2e by patching metro-file-map in user's repro
robhogan
added a commit
that referenced
this issue
Sep 7, 2024
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 #1347 ``` - **[Fix]**: Fix #1347, failure during resolution to read package.json of symlinked workspace packages for exports or redirects. ``` Test plan: - New unit test - Verified e2e by patching metro-file-map in user's repro
robhogan
added a commit
that referenced
this issue
Sep 8, 2024
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 #1347 Changelog: ``` - **[Fix]**: Fix #1347, symlinked packages not respecting package.json redirections. ``` Pull Request resolved: #1349 Test Plan: - New unit test - Verified e2e by patching metro-file-map in user's repro in #1347 Reviewed By: cortinico Differential Revision: D62343439 Pulled By: robhogan fbshipit-source-id: a823601daeffdc8dfd5b5bc3987b75889d44588f
robhogan
added a commit
that referenced
this issue
Sep 8, 2024
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 #1347 Changelog: ``` - **[Fix]**: Fix #1347, symlinked packages not respecting package.json redirections. ``` Pull Request resolved: #1349 Test Plan: - New unit test - Verified e2e by patching metro-file-map in user's repro in #1347 Reviewed By: cortinico Differential Revision: D62343439 Pulled By: robhogan fbshipit-source-id: a823601daeffdc8dfd5b5bc3987b75889d44588f
|
Should be fixed in v0.80.12 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hello, having an issue with
0.80.11andpnpm+expo+ symlinks + package exportsIn
0.80.10package exports are resolved correctly. In0.80.11package exports are not resolved.I have created a reproduction of this issue here: https://github.com/franksmule/expo-repo/tree/main
Symlinks are enabled, although I've enabled
shamefully-hoist=true- I believe local packages are still symlinked - so it seems to be an issue with the combination of symlinks and package exports?Metro Config: (https://github.com/franksmule/expo-repo/blob/main/expo-app/metro.config.js)
Reproduction: https://github.com/franksmule/expo-repo - change https://github.com/franksmule/expo-repo/blob/main/package.json#L5 - to
0.80.10and notice bundle will load.The text was updated successfully, but these errors were encountered: