mirror of
https://github.com/pnpm/pnpm.git
synced 2026-04-27 18:46:18 -04:00
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:
7
.changeset/packageManager-no-lockfile.md
Normal file
7
.changeset/packageManager-no-lockfile.md
Normal 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.
|
||||||
@@ -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
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -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)
|
||||||
|
|||||||
@@ -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
|
||||||
}
|
}
|
||||||
|
|||||||
32
pnpm/src/shouldPersistLockfile.test.ts
Normal file
32
pnpm/src/shouldPersistLockfile.test.ts
Normal 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)
|
||||||
|
})
|
||||||
|
})
|
||||||
19
pnpm/src/shouldPersistLockfile.ts
Normal file
19
pnpm/src/shouldPersistLockfile.ts
Normal 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
|
||||||
|
}
|
||||||
@@ -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,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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 () => {
|
||||||
|
|||||||
@@ -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')
|
||||||
|
|||||||
Reference in New Issue
Block a user