diff --git a/.changeset/fix-workspace-protocol-consistency.md b/.changeset/fix-workspace-protocol-consistency.md new file mode 100644 index 0000000000..033fe3fdfb --- /dev/null +++ b/.changeset/fix-workspace-protocol-consistency.md @@ -0,0 +1,13 @@ +--- +"@pnpm/resolve-dependencies": patch +"@pnpm/core": patch +"pnpm": patch +--- + +Fix workspace package protocol consistency when using `injectWorkspacePackages` + +Previously, workspace packages would inconsistently switch between `link:` and `file:` protocols after operations like `pnpm rm` when `injectWorkspacePackages` was enabled. The issue was that deduplication logic couldn't identify workspace packages in single-package operation contexts. + +This fix ensures workspace packages maintain consistent protocols by checking against all workspace packages from the lockfile, not just packages in the current operation context. + +Fixes #9518 diff --git a/config/deps-installer/src/resolveManifestDependencies.ts b/config/deps-installer/src/resolveManifestDependencies.ts index b0ac438eb7..3070ab2fa4 100644 --- a/config/deps-installer/src/resolveManifestDependencies.ts +++ b/config/deps-installer/src/resolveManifestDependencies.ts @@ -82,6 +82,7 @@ export async function resolveManifestDependencies ( wantedLockfile: emptyLockfile, workspacePackages: new Map(), peersSuffixMaxLength: 1000, + allProjectIds: ['.'], } ) await waitTillAllFetchingsFinish() diff --git a/pkg-manager/core/src/getPeerDependencyIssues.ts b/pkg-manager/core/src/getPeerDependencyIssues.ts index a30c374df1..398deef393 100644 --- a/pkg-manager/core/src/getPeerDependencyIssues.ts +++ b/pkg-manager/core/src/getPeerDependencyIssues.ts @@ -98,6 +98,7 @@ export async function getPeerDependencyIssues ( workspacePackages: ctx.workspacePackages ?? new Map(), supportedArchitectures: opts.supportedArchitectures, peersSuffixMaxLength: opts.peersSuffixMaxLength, + allProjectIds: Object.values(ctx.projects).map((p) => p.id), } ) diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index b7d573a17c..a273ec60a3 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -1283,6 +1283,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { trustPolicyExclude: opts.trustPolicyExclude, trustPolicyIgnoreAfter: opts.trustPolicyIgnoreAfter, blockExoticSubdeps: opts.blockExoticSubdeps, + allProjectIds: Object.values(ctx.projects).map((p) => p.id), } ) if (!opts.include.optionalDependencies || !opts.include.devDependencies || !opts.include.dependencies) { diff --git a/pkg-manager/core/test/install/injectWorkspacePackagesPersistence.test.ts b/pkg-manager/core/test/install/injectWorkspacePackagesPersistence.test.ts new file mode 100644 index 0000000000..5e7504155b --- /dev/null +++ b/pkg-manager/core/test/install/injectWorkspacePackagesPersistence.test.ts @@ -0,0 +1,108 @@ +import path from 'node:path' + +import { assertProject } from '@pnpm/assert-project' +import { mutateModules, mutateModulesInSingleProject, type ProjectOptions } from '@pnpm/core' +import { preparePackages } from '@pnpm/prepare' +import type { ProjectRootDir } from '@pnpm/types' + +import { testDefaults } from '../utils/index.js' + +test('workspace packages should maintain link: protocol after single-project pnpm rm with injectWorkspacePackages', async () => { + const projectAManifest: { name: string, version: string, dependencies: Record } = { + name: 'a', + version: '1.0.0', + dependencies: { + 'b': 'workspace:*', + 'is-positive': '1.0.0', + }, + } + const projectBManifest = { + name: 'b', + version: '1.0.0', + } + + preparePackages([ + { + location: 'a', + package: projectAManifest, + }, + { + location: 'b', + package: projectBManifest, + }, + ]) + + const allProjects: ProjectOptions[] = [ + { + buildIndex: 0, + manifest: projectAManifest, + rootDir: path.resolve('a') as ProjectRootDir, + }, + { + buildIndex: 0, + manifest: projectBManifest, + rootDir: path.resolve('b') as ProjectRootDir, + }, + ] + + // Initial full install with all projects + await mutateModules([ + { + mutation: 'install', + rootDir: path.resolve('a') as ProjectRootDir, + }, + { + mutation: 'install', + rootDir: path.resolve('b') as ProjectRootDir, + }, + ], testDefaults({ + allProjects, + injectWorkspacePackages: true, + })) + + const rootModules = assertProject(process.cwd()) + const lockfile = rootModules.readLockfile() + expect(lockfile.importers.a.dependencies!.b.version).toBe('link:../b') + + // Remove a dependency using mutateModulesInSingleProject. + // This is the code path used by `pnpm rm` when run from within a single + // workspace package directory. It passes allProjects with only the single + // project, so ctx.projects won't contain the other workspace packages. + // The workspacePackages map must still include all workspace packages + // for resolution to work. + delete projectAManifest.dependencies['is-positive'] + const workspacePackages = new Map([ + ['a', new Map([ + ['1.0.0', { + rootDir: path.resolve('a') as ProjectRootDir, + manifest: projectAManifest, + }], + ])], + ['b', new Map([ + ['1.0.0', { + rootDir: path.resolve('b') as ProjectRootDir, + manifest: projectBManifest, + }], + ])], + ]) + await mutateModulesInSingleProject( + { + binsDir: path.resolve('a', 'node_modules', '.bin'), + dependencyNames: ['is-positive'], + manifest: projectAManifest, + mutation: 'uninstallSome', + rootDir: path.resolve('a') as ProjectRootDir, + }, + testDefaults({ + workspacePackages, + injectWorkspacePackages: true, + }) + ) + + const lockfileAfterRm = rootModules.readLockfile() + + // Without the fix, workspace dep 'b' would switch from link: to file: protocol + // because dedupeInjectedDeps couldn't identify 'b' as a workspace package + // when only package 'a' was in the projects list. + expect(lockfileAfterRm.importers.a.dependencies!.b.version).toBe('link:../b') +}) diff --git a/pkg-manager/resolve-dependencies/src/dedupeInjectedDeps.ts b/pkg-manager/resolve-dependencies/src/dedupeInjectedDeps.ts index 7c518ddd41..96babd2259 100644 --- a/pkg-manager/resolve-dependencies/src/dedupeInjectedDeps.ts +++ b/pkg-manager/resolve-dependencies/src/dedupeInjectedDeps.ts @@ -20,6 +20,7 @@ export interface DedupeInjectedDepsOptions { pathsByNodeId: Map projects: ProjectToResolve[] resolvedImporters: ResolvedImporters + workspaceProjectIds: Set } export function dedupeInjectedDeps ( @@ -33,7 +34,7 @@ export function dedupeInjectedDeps ( type InjectedDepsByProjects = Map> function getInjectedDepsByProjects ( - opts: Pick, 'projects' | 'pathsByNodeId' | 'depGraph'> + opts: Pick, 'projects' | 'pathsByNodeId' | 'depGraph' | 'workspaceProjectIds'> ): InjectedDepsByProjects { const injectedDepsByProjects = new Map>() for (const project of opts.projects) { @@ -41,7 +42,7 @@ function getInjectedDepsByProjects ( const depPath = opts.pathsByNodeId.get(nodeId)! if (!opts.depGraph[depPath].id.startsWith('file:')) continue const id = opts.depGraph[depPath].id.substring(5) - if (opts.projects.some((project) => project.id === id)) { + if (opts.workspaceProjectIds.has(id)) { if (!injectedDepsByProjects.has(project.id)) injectedDepsByProjects.set(project.id, new Map()) injectedDepsByProjects.get(project.id)!.set(alias, { depPath, id }) } @@ -62,8 +63,16 @@ function getDedupeMap ( for (const [alias, dep] of deps.entries()) { // Check for subgroup not equal. // The injected project in the workspace may have dev deps - const isSubset = Object.entries(opts.depGraph[dep.depPath].children) - .every(([alias, depPath]) => opts.dependenciesByProjectId[dep.id].get(alias) === depPath) + const children = Object.entries(opts.depGraph[dep.depPath].children) + const targetProjectDeps = opts.dependenciesByProjectId[dep.id] + // When the target project wasn't part of the current resolution (e.g. single-project + // operation), its dependencies aren't available. We can only deduplicate safely when the + // injected dep has no children (the empty set is always a subset). + if (!targetProjectDeps) { + if (children.length > 0) continue + } + const isSubset = children + .every(([alias, depPath]) => targetProjectDeps?.get(alias) === depPath) if (isSubset) { dedupedInjectedDeps.set(alias, dep.id) } diff --git a/pkg-manager/resolve-dependencies/src/index.ts b/pkg-manager/resolve-dependencies/src/index.ts index 83fddcd0db..c932430745 100644 --- a/pkg-manager/resolve-dependencies/src/index.ts +++ b/pkg-manager/resolve-dependencies/src/index.ts @@ -124,6 +124,7 @@ export async function resolveDependencies ( lockfileIncludeTarballUrl?: boolean allowUnusedPatches?: boolean enableGlobalVirtualStore?: boolean + allProjectIds: string[] } ): Promise { const _toResolveImporter = toResolveImporter.bind(null, { @@ -216,6 +217,7 @@ export async function resolveDependencies ( resolvePeersFromWorkspaceRoot: Boolean(opts.resolvePeersFromWorkspaceRoot), resolvedImporters, peersSuffixMaxLength: opts.peersSuffixMaxLength, + workspaceProjectIds: new Set([...opts.allProjectIds, ...Object.keys(opts.wantedLockfile.importers)]), }) const linkedDependenciesByProjectId: Record = {} diff --git a/pkg-manager/resolve-dependencies/src/resolvePeers.ts b/pkg-manager/resolve-dependencies/src/resolvePeers.ts index ce9104cf18..4454497a48 100644 --- a/pkg-manager/resolve-dependencies/src/resolvePeers.ts +++ b/pkg-manager/resolve-dependencies/src/resolvePeers.ts @@ -91,6 +91,7 @@ export async function resolvePeers ( dedupeInjectedDeps?: boolean resolvedImporters: ResolvedImporters peersSuffixMaxLength: number + workspaceProjectIds: Set } ): Promise<{ dependenciesGraph: GenericDependenciesGraphWithResolvedChildren @@ -180,6 +181,7 @@ export async function resolvePeers ( pathsByNodeId, lockfileDir: opts.lockfileDir, resolvedImporters: opts.resolvedImporters, + workspaceProjectIds: opts.workspaceProjectIds, }) } if (opts.dedupePeerDependents) { diff --git a/pkg-manager/resolve-dependencies/test/dedupeDepPaths.test.ts b/pkg-manager/resolve-dependencies/test/dedupeDepPaths.test.ts index 402c89432f..ffdf64a282 100644 --- a/pkg-manager/resolve-dependencies/test/dedupeDepPaths.test.ts +++ b/pkg-manager/resolve-dependencies/test/dedupeDepPaths.test.ts @@ -103,6 +103,7 @@ test('packages are not deduplicated when versions do not match', async () => { virtualStoreDirMaxLength: 120, lockfileDir: '', peersSuffixMaxLength: 1000, + workspaceProjectIds: new Set(), }) expect(dependenciesByProjectId.project1.get('foo')).toEqual(dependenciesByProjectId.project2.get('foo')) diff --git a/pkg-manager/resolve-dependencies/test/resolvePeers.ts b/pkg-manager/resolve-dependencies/test/resolvePeers.ts index 0e374825f0..032ed8d86d 100644 --- a/pkg-manager/resolve-dependencies/test/resolvePeers.ts +++ b/pkg-manager/resolve-dependencies/test/resolvePeers.ts @@ -112,6 +112,7 @@ test('resolve peer dependencies of cyclic dependencies', async () => { lockfileDir: '', virtualStoreDirMaxLength: 120, peersSuffixMaxLength: 1000, + workspaceProjectIds: new Set(), }) expect(Object.keys(dependenciesGraph)).toStrictEqual([ 'foo/1.0.0', @@ -216,6 +217,7 @@ test('when a package is referenced twice in the dependencies graph and one of th virtualStoreDirMaxLength: 120, lockfileDir: '', peersSuffixMaxLength: 1000, + workspaceProjectIds: new Set(), }) expect(Object.keys(dependenciesGraph).sort()).toStrictEqual([ 'bar/1.0.0', @@ -398,6 +400,7 @@ describe('peer dependency issues', () => { virtualStoreDirMaxLength: 120, lockfileDir: '', peersSuffixMaxLength: 1000, + workspaceProjectIds: new Set(), })).peerDependencyIssuesByProjects }) it('should find peer dependency conflicts', () => { @@ -484,6 +487,7 @@ describe('unmet peer dependency issues', () => { virtualStoreDirMaxLength: 120, lockfileDir: '', peersSuffixMaxLength: 1000, + workspaceProjectIds: new Set(), })).peerDependencyIssuesByProjects }) it('should not warn when the found package has prerelease version and the wanted range is *', () => { @@ -557,6 +561,7 @@ describe('unmet peer dependency issue resolved from subdependency', () => { virtualStoreDirMaxLength: 120, lockfileDir: '', peersSuffixMaxLength: 1000, + workspaceProjectIds: new Set(), })).peerDependencyIssuesByProjects }) it('should return from where the bad peer dependency is resolved', () => { @@ -659,6 +664,7 @@ test('resolve peer dependencies with npm aliases', async () => { virtualStoreDirMaxLength: 120, lockfileDir: '', peersSuffixMaxLength: 1000, + workspaceProjectIds: new Set(), }) expect(Object.keys(dependenciesGraph).sort()).toStrictEqual([ 'bar/1.0.0',