Skip to content

fix: resolve relative file: dependencies correctly with install-strategy=linked#9030

Merged
wraithgar merged 1 commit intonpm:latestfrom
manzoorwanijk:fix/linked-strategy-relative-file-deps
Mar 4, 2026
Merged

fix: resolve relative file: dependencies correctly with install-strategy=linked#9030
wraithgar merged 1 commit intonpm:latestfrom
manzoorwanijk:fix/linked-strategy-relative-file-deps

Conversation

@manzoorwanijk
Copy link
Contributor

@manzoorwanijk manzoorwanijk commented Feb 26, 2026

In continuation of our exploration of using install-strategy=linked in the Gutenberg monorepo, which powers the WordPress Block Editor, this is a follow up of the fixes from #8996.

When using install-strategy=linked, relative file: dependencies (e.g., file:./project2) fail with ENOENT because the path is resolved one level above the project root. For example, file:./project2 becomes file:../project2, causing npm to look for /Users/dan/Source/project2 instead of /Users/dan/Source/project1/project2.

In the isolated reifier, non-workspace file: dependencies are incorrectly classified as "external" dependencies. This routes them through the store extraction path (pacote.extract) instead of symlinking them directly like workspaces. A Link node's resolved value is relative to its node_modules/ location — that relative path becomes invalid when used from the store context.

This PR treats local file: dep targets found in fsChildren as local dependencies (alongside workspaces) so they get symlinked directly instead of extracted into the store:

  • _makeIdealGraph: Skip nodes from root.external if they (or their Link target) are in idealTree.fsChildren.
  • assignCommonProperties: Classify deps whose targets are in node.fsChildren as local dependencies instead of external.

References

Fixes #7549

@manzoorwanijk manzoorwanijk requested a review from a team as a code owner February 26, 2026 06:17
@manzoorwanijk manzoorwanijk force-pushed the fix/linked-strategy-relative-file-deps branch from 951f620 to 10c30a7 Compare March 3, 2026 07:14
@wraithgar
Copy link
Member

manually applied just the code changes to my refactor branch and tests all passed.

$ git diff
diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js
index 4fafe7736..dd0adb954 100644
--- a/workspaces/arborist/lib/arborist/isolated-reifier.js
+++ b/workspaces/arborist/lib/arborist/isolated-reifier.js
@@ -100,6 +100,9 @@ module.exports = cls => class IsolatedReifier extends cls {
 
     this.idealGraph.workspaces = await Promise.all(Array.from(idealTree.fsChildren.values(), w => this.workspaceProxy(w)))
     const processed = new Set()
+    // local `file:` deps are in fsChildren but are not workspaces.
+    // they are already handled as workspace-like proxies above and should not go through the external/store extraction path.
+    const localFsChildren = new Set(idealTree.fsChildren)
     const queue = [idealTree, ...idealTree.fsChildren]
     while (queue.length !== 0) {
       const next = queue.pop()
@@ -112,7 +115,7 @@ module.exports = cls => class IsolatedReifier extends cls {
           queue.push(edge.to)
         }
       })
-      if (!next.isProjectRoot && !next.isWorkspace && !next.inert) {
+      if (!next.isProjectRoot && !next.isWorkspace && !next.inert && !localFsChildren.has(next) && !localFsChildren.has(next.target)) {
         this.idealGraph.external.push(await this.externalProxy(next))
       }
     }
@@ -208,8 +211,11 @@ module.exports = cls => class IsolatedReifier extends cls {
 
     if (populateDeps) {
       const optionalDeps = edges.filter(edge => edge.optional).map(edge => edge.to.target)
-      result.localDependencies = await Promise.all(nonOptionalDeps.filter(n => n.isWorkspace).map(n => this.workspaceProxy(n)))
-      result.externalDependencies = await Promise.all(nonOptionalDeps.filter(n => !n.isWorkspace && !n.inert).map(n => this.externalProxy(n)))
+      // local file: deps (non-workspace fsChildren) should be treated as local dependencies, not external, so they get symlinked directly instead of being extracted into the store.
+      const fsChildSet = new Set(node.fsChildren)
+      const isLocal = (n) => n.isWorkspace || fsChildSet.has(n)
+      result.localDependencies = await Promise.all(nonOptionalDeps.filter(isLocal).map(n => this.workspaceProxy(n)))
+      result.externalDependencies = await Promise.all(nonOptionalDeps.filter(n => !isLocal(n) && !n.inert).map(n => this.externalProxy(n)))
       result.externalOptionalDependencies = await Promise.all(optionalDeps.filter(n => !n.inert).map(n => this.externalProxy(n)))
       result.dependencies = [
         ...result.externalDependencies,

@wraithgar wraithgar self-assigned this Mar 3, 2026
@manzoorwanijk manzoorwanijk force-pushed the fix/linked-strategy-relative-file-deps branch from a4d7f6e to c7311a1 Compare March 4, 2026 05:43
@wraithgar
Copy link
Member

We landed the refactor which means it's time to resolve conflicts. If you're comfortable doing that go ahead. If you're not just let us know and we'll help.

…tegy

Local file: dependencies were incorrectly classified as external in the
isolated reifier, routing them through store extraction instead of
symlinking directly. This caused the relative path to resolve from the
wrong directory, producing ENOENT errors.

Treat file dep targets found in fsChildren as local dependencies,
alongside workspaces, so they get symlinked directly.
@manzoorwanijk manzoorwanijk force-pushed the fix/linked-strategy-relative-file-deps branch from c7311a1 to 35c957a Compare March 4, 2026 17:41
@wraithgar wraithgar merged commit 16fbe13 into npm:latest Mar 4, 2026
16 checks passed
wraithgar pushed a commit that referenced this pull request Mar 4, 2026
This contains the changes from

- a2154cd (#8996)
- 880ecb7 (#9013)
- 26fa40e (#9041)
- 983742b (#9055)
- 10d5302 (#9051)
- a29aeee (#9028)
- 16fbe13 (#9030)
- 8614b2a (#9031)

Since Node 22 doesn't have npm 11 yet, it would be better to have this
change backported to v10


Also, we wish to use `install-strategy=linked` in the [Gutenberg
monorepo](WordPress/gutenberg#75814), which
powers the WordPress Block Editor. We are still on v10. So, these fixes
will help.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] incorrect resolution of relative paths with install-strategy=linked

2 participants