feat: add experimental use-inline-specifiers-lockfile-format (#5091)

* feat: add experimental use-inline-specifiers-lockfile-format

* fix(lockfile-file): check importers key for shared lockfile format

The `convertFromLockfileFileMutable` function reverts changes from
`normalizeLockfile` when not using the shared lockfile format.

  - The non-shared lockfile format puts fields like `specifiers`,
    `dependencies`, `devDependencies`, `optionalDependencies`, and
    `dependenciesMeta` on the root of the lockfile. This is typically
    the case for a repo not using pnpm workspaces.
  - The shared lockfile format puts these under a `importers` block
    scoped by a path.

The `use-inline-specifiers-lockfile-format` feature flag removes the
`specifiers` block in favor of putting each specifier next to the
resolved version within each `dependencies`, `devDependencies`, etc
block.

This means the `convertFromLockfileFileMutable` function can no longer
check for `specifiers` to detect the whether the "shared" format is
used. @zkochan suggested checking for `importers` instead, which should
have the same effect.
https://github.com/pnpm/pnpm/pull/5091#discussion_r929326835

* test(lockfile-file): add read & write test for useInlineSpecifiersFormat

.changeset/fifty-rats-jog.md

@@ -0,0 +1,10 @@
+---
+"@pnpm/config": minor
+"@pnpm/core": minor
+"@pnpm/lockfile-file": minor
+"pnpm": minor
+---
+
+Add experimental lockfile format that should merge conflict less in the `importers` section. Enabled by setting the `use-inline-specifiers-lockfile-format = true` feature flag in `.npmrc`.
+
+If this feature flag is committed to a repo, we recommend setting the minimum allowed version of pnpm to this release in the `package.json` `engines` field. Once this is set, older pnpm versions will throw on invalid lockfile versions.

packages/config/src/Config.ts

@@ -157,6 +157,9 @@ export interface Config {
   changedFilesIgnorePattern?: string[]
   rootProjectManifest?: ProjectManifest
   userConfig: Record<string, string>
+
+  // feature flags for experimental testing
+  useInlineSpecifiersLockfileFormat?: boolean // For https://github.com/pnpm/pnpm/issues/4725
 }
 
 export interface ConfigWithDeprecatedSettings extends Config {

packages/config/src/index.ts

@@ -108,6 +108,7 @@ export const types = Object.assign({
   stream: Boolean,
   'strict-peer-dependencies': Boolean,
   'use-beta-cli': Boolean,
+  'use-inline-specifiers-lockfile-format': Boolean,
   'use-node-version': String,
   'use-running-store-server': Boolean,
   'use-store-server': Boolean,
@@ -222,6 +223,7 @@ export default async (
     'strict-peer-dependencies': true,
     'unsafe-perm': npmDefaults['unsafe-perm'],
     'use-beta-cli': false,
+    'use-inline-specifiers-lockfile-format': false,
     userconfig: npmDefaults.userconfig,
     'virtual-store-dir': 'node_modules/.pnpm',
     'workspace-concurrency': 4,

packages/core/src/install/extendInstallOptions.ts

@@ -28,6 +28,7 @@ export interface StrictInstallOptions {
   saveLockfile: boolean
   useGitBranchLockfile: boolean
   mergeGitBranchLockfiles: boolean
+  useInlineSpecifiersLockfileFormat: boolean
   linkWorkspacePackagesDepth: number
   lockfileOnly: boolean
   fixLockfile: boolean
@@ -176,6 +177,7 @@ const defaults = async (opts: InstallOptions) => {
     useLockfile: true,
     saveLockfile: true,
     useGitBranchLockfile: false,
+    useInlineSpecifiersLockfileFormat: false,
     mergeGitBranchLockfiles: false,
     userAgent: `${packageManager.name}/${packageManager.version} npm/? node/${process.version} ${process.platform} ${process.arch}`,
     verifyStoreIntegrity: true,

packages/core/src/install/index.ts

@@ -857,7 +857,12 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
   }
 
   const depsStateCache: DepsStateCache = {}
-  const lockfileOpts = { forceSharedFormat: opts.forceSharedLockfile, useGitBranchLockfile: opts.useGitBranchLockfile, mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles }
+  const lockfileOpts = {
+    forceSharedFormat: opts.forceSharedLockfile,
+    useInlineSpecifiersFormat: opts.useInlineSpecifiersLockfileFormat,
+    useGitBranchLockfile: opts.useGitBranchLockfile,
+    mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
+  }
   if (!opts.lockfileOnly && opts.enableModulesDir) {
     const result = await linkPackages(
       projects,

packages/lockfile-file/src/experiments/InlineSpecifiersLockfile.ts

@@ -0,0 +1,38 @@
+import type { Lockfile } from '@pnpm/lockfile-types'
+import type { DependenciesMeta } from '@pnpm/types'
+
+export const INLINE_SPECIFIERS_FORMAT_LOCKFILE_VERSION_SUFFIX = '-inlineSpecifiers'
+
+/**
+ * Similar to the current Lockfile importers format (lockfile version 5.4 at
+ * time of writing), but specifiers are moved to each ResolvedDependencies block
+ * instead of being declared on its own dictionary.
+ *
+ * This is an experiment to reduce one flavor of merge conflicts in lockfiles.
+ * For more info: https://github.com/pnpm/pnpm/issues/4725.
+ */
+export interface InlineSpecifiersLockfile extends Omit<Lockfile, 'lockfileVersion' | 'importers'> {
+  lockfileVersion: string
+  importers: Record<string, InlineSpecifiersProjectSnapshot>
+}
+
+/**
+ * Similar to the current ProjectSnapshot interface, but omits the "specifiers"
+ * field in favor of inlining each specifier next to its version resolution in
+ * dependency blocks.
+ */
+export interface InlineSpecifiersProjectSnapshot {
+  dependencies?: InlineSpecifiersResolvedDependencies
+  devDependencies?: InlineSpecifiersResolvedDependencies
+  optionalDependencies?: InlineSpecifiersResolvedDependencies
+  dependenciesMeta?: DependenciesMeta
+}
+
+export interface InlineSpecifiersResolvedDependencies {
+  [depName: string]: SpecifierAndResolution
+}
+
+export interface SpecifierAndResolution {
+  specifier: string
+  version: string
+}

packages/lockfile-file/src/experiments/inlineSpecifiersLockfileConverters.ts

@@ -0,0 +1,114 @@
+import type { Lockfile, ProjectSnapshot, ResolvedDependencies } from '@pnpm/lockfile-types'
+import {
+  INLINE_SPECIFIERS_FORMAT_LOCKFILE_VERSION_SUFFIX,
+  InlineSpecifiersLockfile,
+  InlineSpecifiersProjectSnapshot,
+  InlineSpecifiersResolvedDependencies,
+} from './InlineSpecifiersLockfile'
+
+export function isExperimentalInlineSpecifiersFormat (
+  lockfile: InlineSpecifiersLockfile | Lockfile
+): lockfile is InlineSpecifiersLockfile {
+  const { lockfileVersion } = lockfile
+  return typeof lockfileVersion === 'string' && lockfileVersion.endsWith(INLINE_SPECIFIERS_FORMAT_LOCKFILE_VERSION_SUFFIX)
+}
+
+export function convertToInlineSpecifiersFormat (lockfile: Lockfile): InlineSpecifiersLockfile {
+  return {
+    ...lockfile,
+    lockfileVersion: `${lockfile.lockfileVersion}${INLINE_SPECIFIERS_FORMAT_LOCKFILE_VERSION_SUFFIX}`,
+    importers: mapValues(lockfile.importers, convertProjectSnapshotToInlineSpecifiersFormat),
+  }
+}
+
+export function revertFromInlineSpecifiersFormatIfNecessary (lockfile: Lockfile | InlineSpecifiersLockfile): Lockfile {
+  return isExperimentalInlineSpecifiersFormat(lockfile)
+    ? revertFromInlineSpecifiersFormat(lockfile)
+    : lockfile
+}
+
+export function revertFromInlineSpecifiersFormat (lockfile: InlineSpecifiersLockfile): Lockfile {
+  const { lockfileVersion, importers, ...rest } = lockfile
+
+  const originalVersionStr = lockfileVersion.replace(INLINE_SPECIFIERS_FORMAT_LOCKFILE_VERSION_SUFFIX, '')
+  const originalVersion = Number(originalVersionStr)
+  if (isNaN(originalVersion)) {
+    throw new Error(`Unable to revert lockfile from inline specifiers format. Invalid version parsed: ${originalVersionStr}`)
+  }
+
+  return {
+    ...rest,
+    lockfileVersion: originalVersion,
+    importers: mapValues(importers, revertProjectSnapshot),
+  }
+}
+
+function convertProjectSnapshotToInlineSpecifiersFormat (
+  projectSnapshot: ProjectSnapshot
+): InlineSpecifiersProjectSnapshot {
+  const { specifiers, ...rest } = projectSnapshot
+  const convertBlock = (block?: ResolvedDependencies) =>
+    block != null
+      ? convertResolvedDependenciesToInlineSpecifiersFormat(block, { specifiers })
+      : block
+  return {
+    ...rest,
+    dependencies: convertBlock(projectSnapshot.dependencies),
+    optionalDependencies: convertBlock(projectSnapshot.optionalDependencies),
+    devDependencies: convertBlock(projectSnapshot.devDependencies),
+  }
+}
+
+function convertResolvedDependenciesToInlineSpecifiersFormat (
+  resolvedDependencies: ResolvedDependencies,
+  { specifiers }: { specifiers: ResolvedDependencies}
+): InlineSpecifiersResolvedDependencies {
+  return mapValues(resolvedDependencies, (version, depName) => ({
+    specifier: specifiers[depName],
+    version,
+  }))
+}
+
+function revertProjectSnapshot (from: InlineSpecifiersProjectSnapshot): ProjectSnapshot {
+  const specifiers: ResolvedDependencies = {}
+
+  function moveSpecifiers (from: InlineSpecifiersResolvedDependencies): ResolvedDependencies {
+    const resolvedDependencies: ResolvedDependencies = {}
+    for (const [depName, { specifier, version }] of Object.entries(from)) {
+      const existingValue = specifiers[depName]
+      if (existingValue != null && existingValue !== specifier) {
+        throw new Error(`Project snapshot lists the same dependency more than once with conflicting versions: ${depName}`)
+      }
+
+      specifiers[depName] = specifier
+      resolvedDependencies[depName] = version
+    }
+    return resolvedDependencies
+  }
+
+  const dependencies = from.dependencies == null
+    ? from.dependencies
+    : moveSpecifiers(from.dependencies)
+  const devDependencies = from.devDependencies == null
+    ? from.devDependencies
+    : moveSpecifiers(from.devDependencies)
+  const optionalDependencies = from.optionalDependencies == null
+    ? from.optionalDependencies
+    : moveSpecifiers(from.optionalDependencies)
+
+  return {
+    ...from,
+    specifiers,
+    dependencies,
+    devDependencies,
+    optionalDependencies,
+  }
+}
+
+function mapValues<T, U> (obj: Record<string, T>, mapper: (val: T, key: string) => U): Record<string, U> {
+  const result = {}
+  for (const [key, value] of Object.entries(obj)) {
+    result[key] = mapper(value, key)
+  }
+  return result
+}

packages/lockfile-file/src/read.ts

@@ -18,6 +18,7 @@ import logger from './logger'
 import { LockfileFile } from './write'
 import { getWantedLockfileName } from './lockfileName'
 import { getGitBranchLockfileNames } from './gitBranchLockfile'
+import { revertFromInlineSpecifiersFormatIfNecessary } from './experiments/inlineSpecifiersLockfileConverters'
 
 export async function readCurrentLockfile (
   virtualStoreDir: string,
@@ -101,7 +102,7 @@ async function _read (
     })
   }
   if (lockfileFile) {
-    const lockfile = convertFromLockfileFileMutable(lockfileFile)
+    const lockfile = revertFromInlineSpecifiersFormatIfNecessary(convertFromLockfileFileMutable(lockfileFile))
     const lockfileSemver = comverToSemver((lockfile.lockfileVersion ?? 0).toString())
     /* eslint-enable @typescript-eslint/dot-notation */
     if (typeof opts.wantedVersion !== 'number' || semver.major(lockfileSemver) === semver.major(comverToSemver(opts.wantedVersion.toString()))) {
@@ -225,10 +226,10 @@ async function _readGitBranchLockfiles (
  * Reverts changes from the "forceSharedFormat" write option if necessary.
  */
 function convertFromLockfileFileMutable (lockfileFile: LockfileFile): Lockfile {
-  if (typeof lockfileFile?.['specifiers'] !== 'undefined') {
+  if (typeof lockfileFile?.['importers'] === 'undefined') {
     lockfileFile.importers = {
       '.': {
-        specifiers: lockfileFile['specifiers'],
+        specifiers: lockfileFile['specifiers'] ?? {},
         dependenciesMeta: lockfileFile['dependenciesMeta'],
       },
     }

packages/lockfile-file/src/write.ts

@@ -11,6 +11,7 @@ import writeFileAtomicCB from 'write-file-atomic'
 import logger from './logger'
 import { sortLockfileKeys } from './sortLockfileKeys'
 import { getWantedLockfileName } from './lockfileName'
+import { convertToInlineSpecifiersFormat } from './experiments/inlineSpecifiersLockfileConverters'
 
 async function writeFileAtomic (filename: string, data: string) {
   return new Promise<void>((resolve, reject) => writeFileAtomicCB(filename, data, {}, (err?: Error) => (err != null) ? reject(err) : resolve()))
@@ -29,6 +30,7 @@ export async function writeWantedLockfile (
   wantedLockfile: Lockfile,
   opts?: {
     forceSharedFormat?: boolean
+    useInlineSpecifiersFormat?: boolean
     useGitBranchLockfile?: boolean
     mergeGitBranchLockfiles?: boolean
   }
@@ -48,13 +50,16 @@ export async function writeCurrentLockfile (
   return writeLockfile('lock.yaml', virtualStoreDir, currentLockfile, opts)
 }
 
+interface LockfileFormatOptions {
+  forceSharedFormat?: boolean
+  useInlineSpecifiersFormat?: boolean
+}
+
 async function writeLockfile (
   lockfileFilename: string,
   pkgPath: string,
   wantedLockfile: Lockfile,
-  opts?: {
-    forceSharedFormat?: boolean
-  }
+  opts?: LockfileFormatOptions
 ) {
   const lockfilePath = path.join(pkgPath, lockfileFilename)
 
@@ -63,7 +68,11 @@ async function writeLockfile (
     return rimraf(lockfilePath)
   }
 
-  const yamlDoc = yamlStringify(wantedLockfile, opts?.forceSharedFormat === true)
+  const lockfileToStringify = (opts?.useInlineSpecifiersFormat ?? false)
+    ? convertToInlineSpecifiersFormat(wantedLockfile) as unknown as Lockfile
+    : wantedLockfile
+
+  const yamlDoc = yamlStringify(lockfileToStringify, opts?.forceSharedFormat === true)
 
   return writeFileAtomic(lockfilePath, yamlDoc)
 }
@@ -145,6 +154,7 @@ export function normalizeLockfile (lockfile: Lockfile, forceSharedFormat: boolea
 export default async function writeLockfiles (
   opts: {
     forceSharedFormat?: boolean
+    useInlineSpecifiersFormat?: boolean
     wantedLockfile: Lockfile
     wantedLockfileDir: string
     currentLockfile: Lockfile
@@ -167,7 +177,10 @@ export default async function writeLockfiles (
   }
 
   const forceSharedFormat = opts?.forceSharedFormat === true
-  const yamlDoc = yamlStringify(opts.wantedLockfile, forceSharedFormat)
+  const wantedLockfileToStringify = (opts.useInlineSpecifiersFormat ?? false)
+    ? convertToInlineSpecifiersFormat(opts.wantedLockfile) as unknown as Lockfile
+    : opts.wantedLockfile
+  const yamlDoc = yamlStringify(wantedLockfileToStringify, forceSharedFormat)
 
   // in most cases the `pnpm-lock.yaml` and `node_modules/.pnpm-lock.yaml` are equal
   // in those cases the YAML document can be stringified only once for both files

packages/lockfile-file/test/__snapshots__/write.test.ts.snap

@@ -29,3 +29,18 @@ packages:
     resolution: {integrity: sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=}
 "
 `;
+
+exports[`writeLockfiles() when useInlineSpecifiersFormat 1`] = `
+"lockfileVersion: 5.4-inlineSpecifiers
+
+dependencies:
+  foo:
+    specifier: ^1.0.0
+    version: 1.0.0
+
+packages:
+
+  /foo/1.0.0:
+    resolution: {integrity: sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=}
+"
+`;

packages/lockfile-file/test/fixtures/7/pnpm-lock.yaml

@@ -0,0 +1,12 @@
+lockfileVersion: 5.3-inlineSpecifiers
+
+specifiers: {}
+
+dependencies:
+  is-positive:
+    specifier: '^1.0.0'
+    version: '1.0.0'
+
+packages:
+  /is-positive/1.0.0:
+    resolution: {integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g='}

packages/lockfile-file/test/read.test.ts

@@ -261,3 +261,30 @@ test('readWantedLockfile() when useGitBranchLockfile and mergeGitBranchLockfiles
     },
   })
 })
+
+test('readWantedLockfile() with inlineSpecifiersFormat', async () => {
+  const wantedLockfile = {
+    importers: {
+      '.': {
+        dependencies: {
+          'is-positive': '1.0.0',
+        },
+        specifiers: {
+          'is-positive': '^1.0.0',
+        },
+      },
+    },
+    packages: {
+      '/is-positive/1.0.0': {
+        resolution: {
+          integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
+        },
+      },
+    },
+    registry: 'https://registry.npmjs.org',
+  }
+
+  const lockfile = await readWantedLockfile(path.join('fixtures', '7'), { ignoreIncompatible: false })
+  expect(lockfile?.importers).toEqual(wantedLockfile.importers)
+  expect(lockfile?.packages).toEqual(wantedLockfile.packages)
+})

packages/lockfile-file/test/write.test.ts

@@ -231,3 +231,39 @@ test('writeLockfiles() when useGitBranchLockfile', async () => {
   expect(fs.existsSync(path.join(projectPath, WANTED_LOCKFILE))).toBeFalsy()
   expect(fs.existsSync(path.join(projectPath, `pnpm-lock.${branchName}.yaml`))).toBeTruthy()
 })
+
+test('writeLockfiles() when useInlineSpecifiersFormat', async () => {
+  const projectPath = tempy.directory()
+  const wantedLockfile = {
+    importers: {
+      '.': {
+        dependencies: {
+          foo: '1.0.0',
+        },
+        specifiers: {
+          foo: '^1.0.0',
+        },
+      },
+    },
+    lockfileVersion: LOCKFILE_VERSION,
+    packages: {
+      '/foo/1.0.0': {
+        resolution: {
+          integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
+        },
+      },
+    },
+  }
+
+  await writeLockfiles({
+    currentLockfile: wantedLockfile,
+    currentLockfileDir: projectPath,
+    wantedLockfile,
+    wantedLockfileDir: projectPath,
+    useInlineSpecifiersFormat: true,
+  })
+
+  expect(await readCurrentLockfile(projectPath, { ignoreIncompatible: false })).toEqual(wantedLockfile)
+  expect(await readWantedLockfile(projectPath, { ignoreIncompatible: false })).toEqual(wantedLockfile)
+  expect(fs.readFileSync(path.join(projectPath, WANTED_LOCKFILE), 'utf8')).toMatchSnapshot()
+})