fix(catalogs): re-resolve a project the catalog entry moved past (#13860)
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 `<name>@<version>` rather than a bare version, and a non-registry resolution records a protocol; neither compares to a version, so both are left alone.
This commit is contained in:
1 parent
2d8c3e23f4
commit
e5ed8100b0
11 files changed
+186
-15
No files matched your search
@@ -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.
|
||||
Generated
+3
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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('<ROOT>', '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('<ROOT>', 'package.json')}`)
|
||||
})
|
||||
|
||||
test(`frozen-lockfile+hoistPattern: installation fails if specs in package.json don't match the ones in ${WANTED_LOCKFILE}`, async () => {
|
||||
prepareEmpty()
|
||||
|
||||
|
||||
@@ -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:*",
|
||||
|
||||
@@ -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))
|
||||
})
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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'
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
@@ -12,6 +12,9 @@
|
||||
{
|
||||
"path": "../../__utils__/prepare"
|
||||
},
|
||||
{
|
||||
"path": "../../catalogs/protocol-parser"
|
||||
},
|
||||
{
|
||||
"path": "../../catalogs/types"
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user