feat: skip lockfile writes for legacy packageManager field (#11284)

* feat: skip lockfile writes for legacy packageManager field

When pnpm is pinned via the `packageManager` field in `package.json`, the
resolved pnpm integrity info is no longer written to `pnpm-lock.yaml`
unless the pinned version is pnpm v12 or newer. `devEngines.packageManager`
still populates and reuses `packageManagerDependencies` as before. This
keeps the v10 -> v11 transition quiet by avoiding unrelated lockfile
churn for projects that pin pnpm the legacy way.

* fix: address Copilot review and CI failure

- Update `configurationalDependencies.test.ts` to assert the new behavior:
  the `packageManager` field no longer writes pnpm resolution info to the
  env lockfile while config dependencies still are.
- Fast-path in `switchCliVersion`: when the lockfile is not persisted and
  the running CLI already matches `pm.version`, skip store access and
  integrity resolution entirely.
- Clarify the `resolvePackageManagerIntegrities` docstring to describe
  the conditional `save` behavior.

* test: add unit tests for shouldPersistLockfile

Extract the decision logic for persisting pnpm resolution info to the env
lockfile into a dedicated helper so the branches — devEngines source,
legacy `packageManager` field with v11 or older, v12+, and invalid/missing
version — can all be covered without needing an actual pnpm v12 tarball
on the registry.
This commit is contained in:
Zoltan Kochan
2026-04-17 14:45:51 +02:00
committed by GitHub
parent 2dd8712c87
commit ea2a7fb244
9 changed files with 128 additions and 16 deletions

View File

@@ -0,0 +1,7 @@
---
"@pnpm/config.reader": minor
"@pnpm/installing.env-installer": minor
"pnpm": minor
---
When pnpm is declared via the `packageManager` field in `package.json`, its resolution info is no longer written to `pnpm-lock.yaml` — unless the pinned pnpm version is v12 or newer. The `packageManagerDependencies` section is still populated (and reused across runs) when pnpm is declared via `devEngines.packageManager`. This makes the transition from pnpm v10 to v11 quieter by avoiding unnecessary lockfile churn for projects that pin an older pnpm in the legacy `packageManager` field.

View File

@@ -43,7 +43,18 @@ export interface ConfigContext {
name: string name: string
version: string version: string
} }
wantedPackageManager?: EngineDependency wantedPackageManager?: WantedPackageManager
}
/**
* The package manager requested by the root project's manifest.
* Extends {@link EngineDependency} with the source of the declaration so that
* callers can treat the legacy `packageManager` field and
* `devEngines.packageManager` differently (e.g. only the latter persists
* resolved pnpm integrity info to `pnpm-lock.yaml`).
*/
export interface WantedPackageManager extends EngineDependency {
fromDevEngines?: boolean
} }
/** /**

View File

@@ -32,6 +32,7 @@ import type {
ProjectConfig, ProjectConfig,
UniversalOptions, UniversalOptions,
VerifyDepsBeforeRun, VerifyDepsBeforeRun,
WantedPackageManager,
} from './Config.js' } from './Config.js'
import { isConfigFileKey } from './configFileKey.js' import { isConfigFileKey } from './configFileKey.js'
import { extractAndRemoveDependencyBuildOptions, hasDependencyBuildOptions } from './dependencyBuildOptions.js' import { extractAndRemoveDependencyBuildOptions, hasDependencyBuildOptions } from './dependencyBuildOptions.js'
@@ -65,7 +66,7 @@ export {
ProjectConfigsMatchItemIsNotAStringError, ProjectConfigsMatchItemIsNotAStringError,
ProjectConfigUnsupportedFieldError, ProjectConfigUnsupportedFieldError,
} from './projectConfig.js' } from './projectConfig.js'
export type { Config, ConfigContext, ProjectConfig, UniversalOptions, VerifyDepsBeforeRun } export type { Config, ConfigContext, ProjectConfig, UniversalOptions, VerifyDepsBeforeRun, WantedPackageManager }
export { type ConfigFileKey, isConfigFileKey } from './configFileKey.js' export { type ConfigFileKey, isConfigFileKey } from './configFileKey.js'
export { isIniConfigKey, isNpmrcReadableKey } from './localConfig.js' export { isIniConfigKey, isNpmrcReadableKey } from './localConfig.js'
@@ -658,7 +659,7 @@ function getProcessEnv (env: string): string | undefined {
process.env[env.toLowerCase()] process.env[env.toLowerCase()]
} }
function getWantedPackageManager (manifest: ProjectManifest): { pm?: EngineDependency, warnings: string[] } { function getWantedPackageManager (manifest: ProjectManifest): { pm?: WantedPackageManager, warnings: string[] } {
const warnings: string[] = [] const warnings: string[] = []
const pmFromDevEngines = parseDevEnginesPackageManager(manifest.devEngines) const pmFromDevEngines = parseDevEnginesPackageManager(manifest.devEngines)
if (pmFromDevEngines) { if (pmFromDevEngines) {
@@ -669,7 +670,7 @@ function getWantedPackageManager (manifest: ProjectManifest): { pm?: EngineDepen
if (manifest.packageManager) { if (manifest.packageManager) {
warnings.push('Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored') warnings.push('Cannot use both "packageManager" and "devEngines.packageManager" in package.json. "packageManager" will be ignored')
} }
return { pm: pmFromDevEngines, warnings } return { pm: { ...pmFromDevEngines, fromDevEngines: true }, warnings }
} }
if (manifest.packageManager) { if (manifest.packageManager) {
const pm = parsePackageManager(manifest.packageManager) const pm = parsePackageManager(manifest.packageManager)

View File

@@ -13,6 +13,13 @@ export interface ResolvePackageManagerIntegritiesOpts {
rootDir: string rootDir: string
storeController: StoreController storeController: StoreController
storeDir: string storeDir: string
/**
* Whether to read from and write to the env lockfile file on disk.
* When false, resolution happens purely in memory; callers can still use
* the returned `EnvLockfile` to perform installs without persisting the
* resolved pnpm integrity info. Defaults to true.
*/
save?: boolean
} }
/** /**
@@ -32,14 +39,17 @@ export function isPackageManagerResolved (
/** /**
* Resolves integrity checksums for `pnpm`, `@pnpm/exe`, and their dependencies * Resolves integrity checksums for `pnpm`, `@pnpm/exe`, and their dependencies
* by calling resolveManifestDependencies. * by calling resolveManifestDependencies. When `opts.save` is true (the
* Writes the results to the `packageManagerDependencies` section of pnpm-lock.yaml. * default) the results are written to the `packageManagerDependencies`
* section of `pnpm-lock.yaml`; when false, resolution happens purely in
* memory and the returned `EnvLockfile` is never persisted to disk.
*/ */
export async function resolvePackageManagerIntegrities ( export async function resolvePackageManagerIntegrities (
pnpmVersion: string, pnpmVersion: string,
opts: ResolvePackageManagerIntegritiesOpts opts: ResolvePackageManagerIntegritiesOpts
): Promise<EnvLockfile> { ): Promise<EnvLockfile> {
const envLockfile = opts.envLockfile ?? (await readEnvLockfile(opts.rootDir)) ?? createEnvLockfile() const save = opts.save ?? true
const envLockfile = opts.envLockfile ?? (save ? await readEnvLockfile(opts.rootDir) : undefined) ?? createEnvLockfile()
if (isPackageManagerResolved(envLockfile, pnpmVersion)) { if (isPackageManagerResolved(envLockfile, pnpmVersion)) {
return envLockfile return envLockfile
@@ -82,7 +92,9 @@ export async function resolvePackageManagerIntegrities (
envLockfile.packages = prunedFile.packages ?? {} envLockfile.packages = prunedFile.packages ?? {}
envLockfile.snapshots = prunedFile.snapshots ?? {} envLockfile.snapshots = prunedFile.snapshots ?? {}
await writeEnvLockfile(opts.rootDir, envLockfile) if (save) {
await writeEnvLockfile(opts.rootDir, envLockfile)
}
} }
return envLockfile return envLockfile
} }

View File

@@ -0,0 +1,32 @@
import { shouldPersistLockfile } from './shouldPersistLockfile.js'
describe('shouldPersistLockfile', () => {
test('devEngines.packageManager always persists, regardless of version', () => {
expect(shouldPersistLockfile({ version: '9.3.0', fromDevEngines: true })).toBe(true)
expect(shouldPersistLockfile({ version: '11.0.0', fromDevEngines: true })).toBe(true)
expect(shouldPersistLockfile({ version: '12.0.0', fromDevEngines: true })).toBe(true)
expect(shouldPersistLockfile({ version: '>=9.0.0', fromDevEngines: true })).toBe(true)
})
test('packageManager field with pnpm v11 or older does not persist', () => {
expect(shouldPersistLockfile({ version: '9.3.0' })).toBe(false)
expect(shouldPersistLockfile({ version: '10.0.0' })).toBe(false)
expect(shouldPersistLockfile({ version: '11.0.0' })).toBe(false)
expect(shouldPersistLockfile({ version: '11.0.0-rc.1' })).toBe(false)
})
test('packageManager field with pnpm v12 or newer persists', () => {
expect(shouldPersistLockfile({ version: '12.0.0' })).toBe(true)
expect(shouldPersistLockfile({ version: '12.5.3' })).toBe(true)
expect(shouldPersistLockfile({ version: '13.0.0' })).toBe(true)
expect(shouldPersistLockfile({ version: '100.0.0' })).toBe(true)
})
test('missing or invalid version does not persist', () => {
expect(shouldPersistLockfile({ version: undefined })).toBe(false)
expect(shouldPersistLockfile({ version: 'not-a-version' })).toBe(false)
// Ranges are not valid for the legacy packageManager field — its parser
// rejects them, but we still guard defensively here.
expect(shouldPersistLockfile({ version: '^12.0.0' })).toBe(false)
})
})

View File

@@ -0,0 +1,19 @@
import type { WantedPackageManager } from '@pnpm/config.reader'
import semver from 'semver'
/**
* Decides whether the resolved pnpm integrity info should be written to
* `pnpm-lock.yaml` under the project's `packageManagerDependencies` section.
*
* - `devEngines.packageManager` always persists (supports ranges / dist-tags
* that need pinning to be reproducible).
* - The legacy `packageManager` field only persists when the pinned version
* is pnpm v12 or newer. Older pins already contain an exact version in the
* manifest itself, so the lockfile entry would only add churn — and the
* quiet behavior keeps the v10 → v11 transition painless.
*/
export function shouldPersistLockfile (pm: Pick<WantedPackageManager, 'version' | 'fromDevEngines'>): boolean {
if (pm.fromDevEngines === true) return true
if (pm.version == null || semver.valid(pm.version) == null) return false
return semver.major(pm.version) >= 12
}

View File

@@ -12,11 +12,23 @@ import { createStoreController } from '@pnpm/store.connection-manager'
import spawn from 'cross-spawn' import spawn from 'cross-spawn'
import semver from 'semver' import semver from 'semver'
import { shouldPersistLockfile } from './shouldPersistLockfile.js'
export async function switchCliVersion (config: Config, context: ConfigContext): Promise<void> { export async function switchCliVersion (config: Config, context: ConfigContext): Promise<void> {
const pm = context.wantedPackageManager const pm = context.wantedPackageManager
if (pm == null || pm.name !== 'pnpm' || pm.version == null) return if (pm == null || pm.name !== 'pnpm' || pm.version == null) return
let envLockfile = await readEnvLockfile(context.rootProjectManifestDir) ?? undefined const persistLockfile = shouldPersistLockfile(pm)
// In non-persist mode the env lockfile is intentionally not read, so there
// is no cached resolution to compare against. Since the legacy
// `packageManager` field always carries an exact version, we can skip both
// resolution and store access when the running CLI already matches.
if (!persistLockfile && pm.version === packageManager.version) return
let envLockfile = persistLockfile
? (await readEnvLockfile(context.rootProjectManifestDir) ?? undefined)
: undefined
let storeToUse: Awaited<ReturnType<typeof createStoreController>> | undefined let storeToUse: Awaited<ReturnType<typeof createStoreController>> | undefined
// Check if the env lockfile already has a resolved version that satisfies the wanted version/range. // Check if the env lockfile already has a resolved version that satisfies the wanted version/range.
@@ -30,6 +42,7 @@ export async function switchCliVersion (config: Config, context: ConfigContext):
rootDir: context.rootProjectManifestDir, rootDir: context.rootProjectManifestDir,
storeController: storeToUse.ctrl, storeController: storeToUse.ctrl,
storeDir: storeToUse.dir, storeDir: storeToUse.dir,
save: persistLockfile,
}) })
pmVersion = envLockfile.importers['.'].packageManagerDependencies?.['pnpm']?.version pmVersion = envLockfile.importers['.'].packageManagerDependencies?.['pnpm']?.version
if (!pmVersion) { if (!pmVersion) {
@@ -45,6 +58,7 @@ export async function switchCliVersion (config: Config, context: ConfigContext):
rootDir: context.rootProjectManifestDir, rootDir: context.rootProjectManifestDir,
storeController: storeToUse.ctrl, storeController: storeToUse.ctrl,
storeDir: storeToUse.dir, storeDir: storeToUse.dir,
save: persistLockfile,
}) })
} }

View File

@@ -122,7 +122,7 @@ test('config deps are installed after switching to a pnpm version that supports
expect(fs.existsSync('node_modules/.pnpm-config/@pnpm.e2e/has-patch-for-foo')).toBeTruthy() expect(fs.existsSync('node_modules/.pnpm-config/@pnpm.e2e/has-patch-for-foo')).toBeTruthy()
}) })
test('package manager is saved into the lockfile even if it matches the current version', async () => { test('package manager from the packageManager field is not saved into the lockfile', async () => {
const pnpmVersion = JSON.parse(fs.readFileSync(path.join(path.dirname(pnpmBinLocation), '..', 'package.json'), 'utf8')).version as string const pnpmVersion = JSON.parse(fs.readFileSync(path.join(path.dirname(pnpmBinLocation), '..', 'package.json'), 'utf8')).version as string
prepare({ prepare({
packageManager: `pnpm@${pnpmVersion}`, packageManager: `pnpm@${pnpmVersion}`,
@@ -135,18 +135,17 @@ test('package manager is saved into the lockfile even if it matches the current
expect(fs.existsSync('node_modules/.pnpm-config/@pnpm.e2e/has-patch-for-foo')).toBeTruthy() expect(fs.existsSync('node_modules/.pnpm-config/@pnpm.e2e/has-patch-for-foo')).toBeTruthy()
// The env lockfile should have both config dep and package manager entries // The legacy packageManager field already pins an exact version in the
// manifest itself, so pnpm resolution info must not leak into the lockfile.
// Config dependencies are still persisted because they are managed by an
// independent code path.
const envLockfile = await readEnvLockfile(process.cwd()) const envLockfile = await readEnvLockfile(process.cwd())
expect(envLockfile).not.toBeNull() expect(envLockfile).not.toBeNull()
expect(envLockfile!.importers['.'].configDependencies['@pnpm.e2e/has-patch-for-foo']).toStrictEqual({ expect(envLockfile!.importers['.'].configDependencies['@pnpm.e2e/has-patch-for-foo']).toStrictEqual({
specifier: '1.0.0', specifier: '1.0.0',
version: '1.0.0', version: '1.0.0',
}) })
expect(envLockfile!.importers['.'].packageManagerDependencies).toBeDefined() expect(envLockfile!.importers['.'].packageManagerDependencies).toBeUndefined()
expect(envLockfile!.importers['.'].packageManagerDependencies!['pnpm']).toStrictEqual({
specifier: pnpmVersion,
version: pnpmVersion,
})
}) })
test('installing a new configurational dependency', async () => { test('installing a new configurational dependency', async () => {

View File

@@ -21,6 +21,23 @@ test('switch to the pnpm version specified in the packageManager field of packag
expect(stdout.toString()).toContain('Version 9.3.0') expect(stdout.toString()).toContain('Version 9.3.0')
}) })
test('packageManager field does not write pnpm resolution info to pnpm-lock.yaml', async () => {
prepare()
const pnpmHome = path.resolve('pnpm')
const env = { PNPM_HOME: pnpmHome }
writeJsonFileSync('package.json', {
packageManager: 'pnpm@9.3.0',
})
const { stdout } = execPnpmSync(['help'], { env })
expect(stdout.toString()).toContain('Version 9.3.0')
// The legacy packageManager field already pins an exact version in the
// manifest itself, so pnpm resolution info must not leak into the lockfile.
// This keeps the v10 -> v11 transition quiet.
expect(fs.existsSync('pnpm-lock.yaml')).toBe(false)
})
test('do not switch to the pnpm version specified in the packageManager field of package.json, if pmOnFail is set to ignore', async () => { test('do not switch to the pnpm version specified in the packageManager field of package.json, if pmOnFail is set to ignore', async () => {
prepare() prepare()
const pnpmHome = path.resolve('pnpm') const pnpmHome = path.resolve('pnpm')