From e5ed8100b07c41bf03057e5f0f8a07cdece4d5bf Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Fri, 14 Aug 2026 14:15:51 +0200 Subject: [PATCH] fix(catalogs): re-resolve a project the catalog entry moved past (#13860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A catalog entry resolves to one version for every project that references it, but only the projects in the install that moves the entry follow it. The rest kept the version they had — one the entry no longer allowed — and nothing corrected it afterwards: by then the entry's specifier matched what the lockfile recorded, so the up-to-date check read those projects as current and skipped resolution entirely. A frozen install accepted the result too, so a workspace could sit on two versions of one catalog entry indefinitely. The up-to-date check now also compares each `catalog:` dependency's recorded version against the version its entry resolved to, and the resolution the install forgets is chosen by the same predicate, so a project left behind re-resolves through the catalog on its next install.\n\nExplicit frozen installs now reject the inconsistency. An alias (`npm:` in the entry) records `@` rather than a bare version, and a non-registry resolution records a protocol; neither compares to a version, so both are left alone. --- .changeset/catalog-stale-sibling.md | 7 +++ pnpm-lock.yaml | 3 ++ .../deps-installer/src/install/index.ts | 45 +++++++++++------ .../deps-installer/test/catalogs.ts | 48 +++++++++++++++++++ .../test/install/frozenLockfile.ts | 31 ++++++++++++ pnpm11/lockfile/verification/package.json | 1 + .../src/allProjectsAreUpToDate.ts | 2 + .../src/catalogResolutionsAreUpToDate.ts | 36 ++++++++++++++ pnpm11/lockfile/verification/src/index.ts | 1 + .../catalogResolutionsAreUpToDate.test.ts | 24 ++++++++++ pnpm11/lockfile/verification/tsconfig.json | 3 ++ 11 files changed, 186 insertions(+), 15 deletions(-) create mode 100644 .changeset/catalog-stale-sibling.md create mode 100644 pnpm11/lockfile/verification/src/catalogResolutionsAreUpToDate.ts create mode 100644 pnpm11/lockfile/verification/test/catalogResolutionsAreUpToDate.test.ts diff --git a/.changeset/catalog-stale-sibling.md b/.changeset/catalog-stale-sibling.md new file mode 100644 index 0000000000..40a0ff13ee --- /dev/null +++ b/.changeset/catalog-stale-sibling.md @@ -0,0 +1,7 @@ +--- +"@pnpm/installing.deps-installer": patch +"@pnpm/lockfile.verification": minor +"pnpm": patch +--- + +A project that wasn't part of an install that moved a catalog entry now follows the entry the next time it is installed. It used to keep the version the entry resolved to before — a version the entry no longer allowed — and no later install corrected it, so one catalog entry ended up resolved to two versions. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7a52f3cba9..7cdf9184cd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -7159,6 +7159,9 @@ importers: pnpm11/lockfile/verification: dependencies: + '@pnpm/catalogs.protocol-parser': + specifier: workspace:* + version: link:../../catalogs/protocol-parser '@pnpm/catalogs.types': specifier: workspace:* version: link:../../catalogs/types diff --git a/pnpm11/installing/deps-installer/src/install/index.ts b/pnpm11/installing/deps-installer/src/install/index.ts index 8ba17c4676..1980759d83 100644 --- a/pnpm11/installing/deps-installer/src/install/index.ts +++ b/pnpm11/installing/deps-installer/src/install/index.ts @@ -71,7 +71,7 @@ import { resolvePatchedDependencies, } from '@pnpm/lockfile.settings-checker' import { PACKAGE_MAP_FILENAME, writePackageMap, writePnpFile } from '@pnpm/lockfile.to-pnp' -import { allProjectsAreUpToDate, satisfiesPackageManifest } from '@pnpm/lockfile.verification' +import { allProjectsAreUpToDate, catalogResolutionIsStale, catalogResolutionsAreUpToDate, satisfiesPackageManifest } from '@pnpm/lockfile.verification' import { globalInfo, logger, streamParser } from '@pnpm/logger' import { groupPatchedDependencies, type PatchGroupRecord } from '@pnpm/patching.config' import { createVersionSpecFromResolvedVersion, getAllDependenciesFromManifest, getAllUniqueSpecs } from '@pnpm/pkg-manifest.utils' @@ -773,7 +773,6 @@ export async function mutateModules ( } } let outdatedLockfileSettingName: ChangedField | null = changedLockfileSettings[0] ?? null - const _isWantedDepBareSpecifierSame = isWantedDepBareSpecifierSame.bind(null, ctx.wantedLockfile.catalogs, opts.catalogs) const upToDateLockfileMajorVersion = ctx.wantedLockfile.lockfileVersion.toString().startsWith(`${LOCKFILE_MAJOR_VERSION}.`) let didFastUpdateOverrides = false const contextProjects = Object.values(ctx.projects).map((project) => { @@ -1068,7 +1067,11 @@ export async function mutateModules ( } if (ctx.wantedLockfile?.importers) { - forgetResolutionsOfPrevWantedDeps(ctx.wantedLockfile.importers[project.id], wantedDependencies, _isWantedDepBareSpecifierSame) + forgetResolutionsOfPrevWantedDeps(wantedDependencies, { + importer: ctx.wantedLockfile.importers[project.id], + prevCatalogs: ctx.wantedLockfile.catalogs, + catalogsConfig: opts.catalogs, + }) } if (opts.ignoreScripts && project.manifest?.scripts && (project.manifest.scripts.preinstall != null || @@ -1404,8 +1407,9 @@ Note that in CI environments, this setting is enabled by default.`, ignoredOptionalDependencies: opts.ignoredOptionalDependencies, }) for (const { id, manifest, rootDir } of Object.values(ctx.projects)) { - const { satisfies, detailedReason } = _satisfiesPackageManifest(ctx.wantedLockfile.importers[id], manifest) - if (!satisfies) { + const importer = ctx.wantedLockfile.importers[id] + const { satisfies, detailedReason } = _satisfiesPackageManifest(importer, manifest) + if (!satisfies || (importer != null && !catalogResolutionsAreUpToDate(importer, ctx.wantedLockfile.catalogs))) { if (!ctx.existsWantedLockfile) { throw new PnpmError('NO_LOCKFILE', `Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is absent`, { @@ -1577,22 +1581,30 @@ function pkgHasDependencies (manifest: ProjectManifest): boolean { // If the specifier is new, the old resolution probably does not satisfy it anymore. // By removing these resolutions we ensure that they are resolved again using the new specs. function forgetResolutionsOfPrevWantedDeps ( - importer: ProjectSnapshot, wantedDeps: WantedDependency[], - isWantedDepBareSpecifierSame: (alias: string, prevBareSpecifier: string | undefined, nextBareSpecifier: string) => boolean + ctx: { + importer: ProjectSnapshot + prevCatalogs: CatalogSnapshots | undefined + catalogsConfig: Catalogs | undefined + } ): void { + const { importer, prevCatalogs, catalogsConfig } = ctx if (!importer.specifiers) return importer.dependencies = importer.dependencies ?? {} importer.devDependencies = importer.devDependencies ?? {} importer.optionalDependencies = importer.optionalDependencies ?? {} for (const { alias, bareSpecifier } of wantedDeps) { - if (alias && !isWantedDepBareSpecifierSame(alias, importer.specifiers[alias], bareSpecifier)) { - if (!importer.dependencies[alias]?.startsWith('link:')) { - delete importer.dependencies[alias] - } - delete importer.devDependencies[alias] - delete importer.optionalDependencies[alias] + if (!alias) continue + const prevBareSpecifier = importer.specifiers[alias] + if ( + isWantedDepBareSpecifierSame(prevCatalogs, catalogsConfig, alias, prevBareSpecifier, bareSpecifier) && + !catalogResolutionIsStale({ importer, catalogs: prevCatalogs, alias, specifier: prevBareSpecifier }) + ) continue + if (!importer.dependencies[alias]?.startsWith('link:')) { + delete importer.dependencies[alias] } + delete importer.devDependencies[alias] + delete importer.optionalDependencies[alias] } } @@ -2520,7 +2532,6 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => { updateWorkspaceDependencies: false, injectWorkspacePackages: opts.injectWorkspacePackages, } - const _isWantedDepBareSpecifierSame = isWantedDepBareSpecifierSame.bind(null, ctx.wantedLockfile.catalogs, opts.catalogs) for (const project of allProjectsLocatedInsideWorkspace) { if (!newProjects.some(({ rootDir }) => rootDir === project.rootDir)) { // This code block mirrors the installCase() function in @@ -2528,7 +2539,11 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => { // deduplicate code. const wantedDependencies = getWantedDependencies(project.manifest, getWantedDepsOpts) .map((wantedDependency) => ({ ...wantedDependency, updateSpec: true, preserveNonSemverVersionSpec: true })) - forgetResolutionsOfPrevWantedDeps(ctx.wantedLockfile.importers[project.id], wantedDependencies, _isWantedDepBareSpecifierSame) + forgetResolutionsOfPrevWantedDeps(wantedDependencies, { + importer: ctx.wantedLockfile.importers[project.id], + prevCatalogs: ctx.wantedLockfile.catalogs, + catalogsConfig: opts.catalogs, + }) newProjects.push({ mutation: 'install', ...project, diff --git a/pnpm11/installing/deps-installer/test/catalogs.ts b/pnpm11/installing/deps-installer/test/catalogs.ts index e6c7a2a12d..72d164e9d5 100644 --- a/pnpm11/installing/deps-installer/test/catalogs.ts +++ b/pnpm11/installing/deps-installer/test/catalogs.ts @@ -1792,6 +1792,54 @@ describe('update', () => { // is-positive since public packages can release new versions and break the // tests here. + test('a project left behind by a catalog update follows the entry on its next install', async () => { + const { options, projects, readLockfile } = preparePackagesAndReturnObjects([{ + name: 'project1', + dependencies: { + '@pnpm.e2e/foo': 'catalog:', + }, + }, { + name: 'project2', + dependencies: { + '@pnpm.e2e/foo': 'catalog:', + }, + }]) + + const catalogs = { + default: { '@pnpm.e2e/foo': '1.0.0' }, + } + const mutateOpts = { + ...options, + lockfileOnly: true, + catalogs, + } + + await mutateModules(installProjects(projects), mutateOpts) + catalogs.default['@pnpm.e2e/foo'] = '^1.0.0' + await mutateModules(installProjects(projects), mutateOpts) + + // Update one project. The other keeps the version the entry resolved to before, which + // the bumped entry no longer admits. + const { updatedCatalogs } = await addDependenciesToPackage( + projects['project1' as ProjectId], + ['@pnpm.e2e/foo'], + { + ...mutateOpts, + dir: path.join(options.lockfileDir, 'project1'), + update: true, + }) + expect(updatedCatalogs).toEqual({ default: { '@pnpm.e2e/foo': '^1.3.0' } }) + catalogs.default['@pnpm.e2e/foo'] = '^1.3.0' + + await mutateModules(installProjects(projects), mutateOpts) + + // A catalog entry resolves to one version for every project that references it. + expect(readLockfile().importers).toMatchObject({ + project1: { dependencies: { '@pnpm.e2e/foo': { specifier: 'catalog:', version: '1.3.0' } } }, + project2: { dependencies: { '@pnpm.e2e/foo': { specifier: 'catalog:', version: '1.3.0' } } }, + }) + }) + test('update works on cataloged dependency', async () => { const { options, projects, readLockfile } = preparePackagesAndReturnObjects([{ name: 'project1', diff --git a/pnpm11/installing/deps-installer/test/install/frozenLockfile.ts b/pnpm11/installing/deps-installer/test/install/frozenLockfile.ts index c6315630ee..9574f73bf6 100644 --- a/pnpm11/installing/deps-installer/test/install/frozenLockfile.ts +++ b/pnpm11/installing/deps-installer/test/install/frozenLockfile.ts @@ -8,6 +8,7 @@ import { type MutatedProject, mutateModules, } from '@pnpm/installing.deps-installer' +import { readWantedLockfile, writeWantedLockfile } from '@pnpm/lockfile.fs' import { prepareEmpty, preparePackages } from '@pnpm/prepare' import type { ProjectRootDir } from '@pnpm/types' @@ -34,6 +35,36 @@ test(`frozen-lockfile: installation fails if specs in package.json don't match t ).rejects.toThrow(`Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is not up to date with ${path.join('', 'package.json')}`) }) +test('frozen-lockfile: installation fails if a catalog resolution is stale', async () => { + prepareEmpty() + const manifest = { + dependencies: { + 'is-positive': 'catalog:', + }, + } + + await install(manifest, testDefaults({ + catalogs: { default: { 'is-positive': '1.0.0' } }, + lockfileOnly: true, + })) + + const wantedLockfile = (await readWantedLockfile('.', { ignoreIncompatible: false }))! + wantedLockfile.catalogs = { + ...wantedLockfile.catalogs, + default: { + ...wantedLockfile.catalogs?.default, + 'is-positive': { specifier: '^3.0.0', version: '3.1.0' }, + }, + } + await writeWantedLockfile('.', wantedLockfile) + + await expect(install(manifest, testDefaults({ + catalogs: { default: { 'is-positive': '^3.0.0' } }, + frozenLockfile: true, + lockfileOnly: true, + }))).rejects.toThrow(`Cannot install with "frozen-lockfile" because ${WANTED_LOCKFILE} is not up to date with ${path.join('', 'package.json')}`) +}) + test(`frozen-lockfile+hoistPattern: installation fails if specs in package.json don't match the ones in ${WANTED_LOCKFILE}`, async () => { prepareEmpty() diff --git a/pnpm11/lockfile/verification/package.json b/pnpm11/lockfile/verification/package.json index 0402b3fbe7..6ac133f248 100644 --- a/pnpm11/lockfile/verification/package.json +++ b/pnpm11/lockfile/verification/package.json @@ -32,6 +32,7 @@ ".test": "cross-env NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules --disable-warning=ExperimentalWarning --disable-warning=DEP0169\" jest" }, "dependencies": { + "@pnpm/catalogs.protocol-parser": "workspace:*", "@pnpm/catalogs.types": "workspace:*", "@pnpm/config.matcher": "workspace:*", "@pnpm/crypto.hash": "workspace:*", diff --git a/pnpm11/lockfile/verification/src/allProjectsAreUpToDate.ts b/pnpm11/lockfile/verification/src/allProjectsAreUpToDate.ts index a9466f05da..bd2ea8c1bc 100644 --- a/pnpm11/lockfile/verification/src/allProjectsAreUpToDate.ts +++ b/pnpm11/lockfile/verification/src/allProjectsAreUpToDate.ts @@ -9,6 +9,7 @@ import pEvery from 'p-every' import { isEmpty } from 'ramda' import { allCatalogsAreUpToDate } from './allCatalogsAreUpToDate.js' +import { catalogResolutionsAreUpToDate } from './catalogResolutionsAreUpToDate.js' import { getWorkspacePackagesByDirectory } from './getWorkspacePackagesByDirectory.js' import { linkedPackagesAreUpToDate } from './linkedPackagesAreUpToDate.js' import { localTarballDepsAreUpToDate } from './localTarballDepsAreUpToDate.js' @@ -66,6 +67,7 @@ export async function allProjectsAreUpToDate ( return importer != null && _satisfiesPackageManifest(importer, project.manifest).satisfies && + catalogResolutionsAreUpToDate(importer, opts.wantedLockfile.catalogs) && (await _localTarballDepsAreUpToDate(projectInfo)) && (_linkedPackagesAreUpToDate(projectInfo)) }) diff --git a/pnpm11/lockfile/verification/src/catalogResolutionsAreUpToDate.ts b/pnpm11/lockfile/verification/src/catalogResolutionsAreUpToDate.ts new file mode 100644 index 0000000000..e826a744cd --- /dev/null +++ b/pnpm11/lockfile/verification/src/catalogResolutionsAreUpToDate.ts @@ -0,0 +1,36 @@ +import { parseCatalogProtocol } from '@pnpm/catalogs.protocol-parser' +import * as dp from '@pnpm/deps.path' +import type { CatalogSnapshots, ProjectSnapshot } from '@pnpm/lockfile.types' +import semver from 'semver' + +export function catalogResolutionsAreUpToDate ( + importer: ProjectSnapshot, + catalogs: CatalogSnapshots | undefined +): boolean { + if (importer.specifiers == null) return true + for (const [alias, specifier] of Object.entries(importer.specifiers)) { + if (!catalogResolutionIsStale({ importer, catalogs, alias, specifier })) continue + return false + } + return true +} + +export function catalogResolutionIsStale ( + { importer, catalogs, alias, specifier }: { + importer: ProjectSnapshot + catalogs: CatalogSnapshots | undefined + alias: string + specifier: string | undefined + } +): boolean { + if (specifier == null) return false + const catalogName = parseCatalogProtocol(specifier) + if (catalogName === null) return false + const catalogVersion = catalogs?.[catalogName]?.[alias]?.version + if (catalogVersion == null) return false + const ref = importer.dependencies?.[alias] ?? importer.devDependencies?.[alias] ?? importer.optionalDependencies?.[alias] + if (ref == null) return false + const resolvedVersion = dp.removeSuffix(ref) + if (semver.valid(resolvedVersion) == null) return false + return resolvedVersion !== catalogVersion +} diff --git a/pnpm11/lockfile/verification/src/index.ts b/pnpm11/lockfile/verification/src/index.ts index ecffbeaa7c..4413c08bf0 100644 --- a/pnpm11/lockfile/verification/src/index.ts +++ b/pnpm11/lockfile/verification/src/index.ts @@ -1,5 +1,6 @@ export { allCatalogsAreUpToDate } from './allCatalogsAreUpToDate.js' export { allProjectsAreUpToDate } from './allProjectsAreUpToDate.js' +export { catalogResolutionIsStale, catalogResolutionsAreUpToDate } from './catalogResolutionsAreUpToDate.js' export { getWorkspacePackagesByDirectory } from './getWorkspacePackagesByDirectory.js' export { linkedPackagesAreUpToDate } from './linkedPackagesAreUpToDate.js' export { localTarballDepsAreUpToDate } from './localTarballDepsAreUpToDate.js' diff --git a/pnpm11/lockfile/verification/test/catalogResolutionsAreUpToDate.test.ts b/pnpm11/lockfile/verification/test/catalogResolutionsAreUpToDate.test.ts new file mode 100644 index 0000000000..03985f13fc --- /dev/null +++ b/pnpm11/lockfile/verification/test/catalogResolutionsAreUpToDate.test.ts @@ -0,0 +1,24 @@ +import { expect, test } from '@jest/globals' +import { catalogResolutionIsStale, catalogResolutionsAreUpToDate } from '@pnpm/lockfile.verification' + +test('peer-suffixed catalog resolutions are compared by version', () => { + const importer = { + dependencies: { + foo: '1.3.0(peer@1.0.0)', + }, + specifiers: { + foo: 'catalog:', + }, + } + const catalogs = { + default: { + foo: { + specifier: '^1.4.0', + version: '1.4.0', + }, + }, + } + + expect(catalogResolutionIsStale({ importer, catalogs, alias: 'foo', specifier: 'catalog:' })).toBe(true) + expect(catalogResolutionsAreUpToDate(importer, catalogs)).toBe(false) +}) diff --git a/pnpm11/lockfile/verification/tsconfig.json b/pnpm11/lockfile/verification/tsconfig.json index 799bf93f1c..afcf189e5d 100644 --- a/pnpm11/lockfile/verification/tsconfig.json +++ b/pnpm11/lockfile/verification/tsconfig.json @@ -12,6 +12,9 @@ { "path": "../../__utils__/prepare" }, + { + "path": "../../catalogs/protocol-parser" + }, { "path": "../../catalogs/types" },