fix(graph-builder): omit skipped dependency links (#16455)
Return undefined for skipped optional dependencies and unresolved peers instead of an empty path. Keep those aliases out of the children map so frozen installs cannot link them to the current working directory. Closes pnpm/pnpm#16454.
This commit is contained in:
1 parent
d45bfb1174
commit
21cd33fc03
3 files changed
+56
-4
No files matched your search
@@ -0,0 +1,6 @@
|
||||
---
|
||||
"@pnpm/deps.graph-builder": patch
|
||||
"pnpm": patch
|
||||
---
|
||||
|
||||
Fixed frozen installs creating symlinks to the working directory for skipped optional dependencies and unresolved peer dependencies [#16454](https://github.com/pnpm/pnpm/issues/16454).
|
||||
@@ -204,7 +204,8 @@ function getChildrenPaths (
|
||||
): { [alias: string]: string } {
|
||||
const children: { [alias: string]: string } = {}
|
||||
for (const [alias, ref] of Object.entries(allDeps)) {
|
||||
children[alias] = resolveChildPath(ctx, alias, ref, peerDeps, parent)
|
||||
const childPath = resolveChildPath(ctx, alias, ref, peerDeps, parent)
|
||||
if (childPath != null) children[alias] = childPath
|
||||
}
|
||||
return children
|
||||
}
|
||||
@@ -215,7 +216,7 @@ function resolveChildPath (
|
||||
ref: string,
|
||||
peerDeps: Set<string> | null,
|
||||
parent: { importerId: string, pkgDir?: string }
|
||||
): string {
|
||||
): string | undefined {
|
||||
const packageRootLinkTarget = dp.packageRootLinkTarget(ref)
|
||||
if (packageRootLinkTarget != null && parent.pkgDir != null) {
|
||||
return path.join(parent.pkgDir, packageRootLinkTarget)
|
||||
@@ -233,7 +234,7 @@ function resolveDepPathLocation (
|
||||
ref: string,
|
||||
alias: string,
|
||||
peerDeps: Set<string> | null
|
||||
): string {
|
||||
): string | undefined {
|
||||
if (ctx.locationByDepPath[childRelDepPath]) {
|
||||
return ctx.locationByDepPath[childRelDepPath]
|
||||
}
|
||||
@@ -246,5 +247,5 @@ function resolveDepPathLocation (
|
||||
if (!ctx.skipped.has(childRelDepPath as DepPath) && (peerDeps == null || !peerDeps.has(alias))) {
|
||||
throw new Error(`${childRelDepPath} not found in ${WANTED_LOCKFILE}`)
|
||||
}
|
||||
return ''
|
||||
return undefined
|
||||
}
|
||||
@@ -6,6 +6,7 @@ import path from 'node:path'
|
||||
import { expect, test } from '@jest/globals'
|
||||
import { depPathToFilename } from '@pnpm/deps.path'
|
||||
import type { LockfileObject } from '@pnpm/lockfile.fs'
|
||||
import type { DepPath } from '@pnpm/types'
|
||||
|
||||
import { lockfileToDepGraph, type LockfileToDepGraphOptions } from '../src/lockfileToDepGraph.js'
|
||||
|
||||
@@ -124,3 +125,47 @@ test('lockfileToDepGraph rejects a global-virtual-store slot whose version escap
|
||||
lockfileToDepGraph(lockfile, null, opts)
|
||||
).rejects.toThrow(expect.objectContaining({ code: 'ERR_PNPM_INVALID_DEPENDENCY_NAME' }))
|
||||
})
|
||||
|
||||
test.each(['skipped optional dependency', 'unresolved peer dependency'])(
|
||||
'lockfileToDepGraph omits children with %s', async (scenario) => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'graph-builder-'))
|
||||
const isOptional = scenario === 'skipped optional dependency'
|
||||
const lockfile = {
|
||||
lockfileVersion: '9.0',
|
||||
importers: {
|
||||
'.': {
|
||||
specifiers: { parent: '1.0.0', ...(isOptional ? { missing: '1.0.0' } : {}) },
|
||||
dependencies: { parent: '1.0.0' },
|
||||
...(isOptional ? { optionalDependencies: { missing: '1.0.0' } } : {}),
|
||||
},
|
||||
},
|
||||
packages: {
|
||||
'parent@1.0.0': {
|
||||
resolution: { tarball: 'https://example.com/parent.tgz' },
|
||||
dependencies: { child: '1.0.0', ...(!isOptional ? { missing: '1.0.0' } : {}) },
|
||||
...(isOptional
|
||||
? { optionalDependencies: { missing: '1.0.0' } }
|
||||
: { peerDependencies: { missing: '*' } }),
|
||||
},
|
||||
'child@1.0.0': { resolution: { tarball: 'https://example.com/child.tgz' } },
|
||||
},
|
||||
} as unknown as LockfileObject
|
||||
const opts = {
|
||||
...graphOpts(dir),
|
||||
includeUnchangedDeps: true,
|
||||
skipped: new Set<DepPath>(isOptional ? ['missing@1.0.0' as DepPath] : []),
|
||||
}
|
||||
const parentDir = path.join(opts.virtualStoreDir, 'parent@1.0.0', 'node_modules', 'parent')
|
||||
const childDir = path.join(opts.virtualStoreDir, 'child@1.0.0', 'node_modules', 'child')
|
||||
fs.mkdirSync(parentDir, { recursive: true })
|
||||
fs.mkdirSync(childDir, { recursive: true })
|
||||
try {
|
||||
const { graph, directDependenciesByImporterId } = await lockfileToDepGraph(lockfile, lockfile, opts)
|
||||
|
||||
expect(graph[parentDir].children).toStrictEqual({ child: childDir })
|
||||
expect(directDependenciesByImporterId['.']).toStrictEqual({ parent: parentDir })
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true })
|
||||
}
|
||||
}
|
||||
)
|
||||
Reference in new issue
Block a user