Skip to content

Commit 3b54e9c

Browse files
authored
fix: installLinks works with transitive external file dependencies (#8534)
fixes #8342 What This PR fixes an issue where npm fails to properly handle transitive file dependencies when using the --install-links flag. Previously, when a file dependency had its own file dependencies, npm would fail to resolve them correctly, resulting in `ERR_MODULE_NOT_FOUND` errors. Why When using `npm install --install-links` to install a local package that has its own file dependencies, npm would attempt to resolve the transitive dependencies relative to the installed location in `node_modules` rather than the original source location. This caused the installation to fail because the transitive dependencies couldn't be found at the incorrect path. For example, given this structure: Running `npm install --install-links ../b` from `mainpkg` would fail because npm tried to find a relative to `b` instead of relative to the original `b` source location. How The fix introduces logic to detect transitive file dependencies when `--install-links` is used and ensures they are resolved relative to their parent's original source location: Detect transitive file dependencies: When a parent package was installed (not linked) due to `--install-links` and has file dependencies of its own, those are identified as transitive file dependencies. Preserve original paths: The parent's resolved` field (e.g., file:../b) is used to determine the original source location. Correct path resolution: Transitive file dependencies are resolved relative to the parent's original location rather than its installed location in `node_modules`
1 parent 75ce64a commit 3b54e9c

File tree

2 files changed

+77
-4
lines changed

2 files changed

+77
-4
lines changed

workspaces/arborist/lib/arborist/build-ideal-tree.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,22 +1238,35 @@ This is a one-time fix-up, please be patient...
12381238
// Check if the target is within the project root
12391239
isProjectInternalFileSpec = targetPath.startsWith(resolvedProjectRoot + sep) || targetPath === resolvedProjectRoot
12401240
}
1241+
1242+
// When using --install-links, we need to handle transitive file dependencies specially
1243+
// If the parent was installed (not linked) due to --install-links, and this is a file: dep, we should also install it rather than link it
1244+
const parentWasInstalled = parent && !parent.isLink && parent.resolved?.startsWith('file:')
1245+
const isTransitiveFileDep = spec.type === 'directory' && parentWasInstalled && installLinks
1246+
12411247
// Decide whether to link or copy the dependency
1242-
const shouldLink = isWorkspace || isProjectInternalFileSpec || !installLinks
1248+
const shouldLink = (isWorkspace || isProjectInternalFileSpec || !installLinks) && !isTransitiveFileDep
12431249
if (spec.type === 'directory' && shouldLink) {
12441250
return this.#linkFromSpec(name, spec, parent, edge)
12451251
}
12461252

1247-
// if the spec matches a workspace name, then see if the workspace node will
1248-
// satisfy the edge. if it does, we return the workspace node to make sure it
1249-
// takes priority.
1253+
// if the spec matches a workspace name, then see if the workspace node will satisfy the edge. if it does, we return the workspace node to make sure it takes priority.
12501254
if (isWorkspace) {
12511255
const existingNode = this.idealTree.edgesOut.get(spec.name).to
12521256
if (existingNode && existingNode.isWorkspace && existingNode.satisfies(edge)) {
12531257
return existingNode
12541258
}
12551259
}
12561260

1261+
// For file: dependencies that we're installing (not linking), ensure proper resolution
1262+
if (isTransitiveFileDep && edge) {
1263+
// For transitive file deps, resolve relative to the parent's original source location
1264+
const parentOriginalPath = parent.resolved.slice(5) // Remove 'file:' prefix
1265+
const relativePath = edge.rawSpec.slice(5) // Remove 'file:' prefix
1266+
const absolutePath = resolve(parentOriginalPath, relativePath)
1267+
spec = npa.resolve(name, `file:${absolutePath}`)
1268+
}
1269+
12571270
// spec isn't a directory, and either isn't a workspace or the workspace we have
12581271
// doesn't satisfy the edge. try to fetch a manifest and build a node from that.
12591272
return this.#fetchManifest(spec)

workspaces/arborist/test/arborist/build-ideal-tree.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4389,4 +4389,64 @@ t.test('installLinks behavior with project-internal file dependencies', async t
43894389
t.ok(nestedDep, 'nested-dep should be found')
43904390
t.ok(nestedDep.isLink, 'nested-dep should be a link (project-internal)')
43914391
})
4392+
4393+
t.test('installLinks=true with transitive external file dependencies', async t => {
4394+
// mainpkg installs b (external file dep) with --install-links
4395+
// b depends on a (another external file dep via file:../a)
4396+
// Both should be installed (not linked) and dependencies should resolve correctly
4397+
const testRoot = t.testdir({
4398+
a: {
4399+
'package.json': JSON.stringify({
4400+
name: 'a',
4401+
main: 'index.js',
4402+
}),
4403+
'index.js': 'export const A = "A";',
4404+
},
4405+
b: {
4406+
'package.json': JSON.stringify({
4407+
name: 'b',
4408+
main: 'index.js',
4409+
dependencies: {
4410+
a: 'file:../a',
4411+
},
4412+
}),
4413+
'index.js': 'import {A} from "a";export const fn = () => console.log(A);',
4414+
},
4415+
mainpkg: {
4416+
'package.json': JSON.stringify({}),
4417+
},
4418+
})
4419+
4420+
const mainpkgPath = join(testRoot, 'mainpkg')
4421+
const bPath = join(testRoot, 'b')
4422+
createRegistry(t, false)
4423+
4424+
const arb = newArb(mainpkgPath, { installLinks: true })
4425+
4426+
// Add the external file dependency using the full path
4427+
await arb.buildIdealTree({ add: [`file:${bPath}`] })
4428+
4429+
const tree = arb.idealTree
4430+
4431+
// Both packages should be present in the tree
4432+
const packageB = tree.children.get('b')
4433+
const packageA = tree.children.get('a')
4434+
4435+
t.ok(packageB, 'package b should be found in tree')
4436+
t.ok(packageA, 'package a should be found in tree (transitive dependency)')
4437+
4438+
// Both should be installed (not linked) due to installLinks=true
4439+
t.notOk(packageB.isLink, 'package b should not be a link (installLinks=true)')
4440+
t.notOk(packageA.isLink, 'package a should not be a link (transitive with installLinks=true)')
4441+
4442+
// Verify that the resolved paths are correct
4443+
t.match(packageB.resolved, /file:.*[/\\]b$/, 'package b should have correct resolved path')
4444+
t.match(packageA.resolved, /file:.*[/\\]a$/, 'package a should have correct resolved path')
4445+
4446+
// Verify the dependency relationship
4447+
const edgeToA = packageB.edgesOut.get('a')
4448+
t.ok(edgeToA, 'package b should have an edge to a')
4449+
t.ok(edgeToA.valid, 'the edge from b to a should be valid')
4450+
t.equal(edgeToA.to, packageA, 'the edge from b should point to package a')
4451+
})
43924452
})

0 commit comments

Comments
 (0)