diff --git a/.changeset/stupid-pants-turn.md b/.changeset/stupid-pants-turn.md new file mode 100644 index 00000000000..6a850b0138d --- /dev/null +++ b/.changeset/stupid-pants-turn.md @@ -0,0 +1,6 @@ +--- +"@pnpm/lockfile-file": patch +"pnpm": patch +--- + +Fix sorting of keys in lockfile to make it more deterministic and prevent unnecessary churn in the lockfile [#5151](https://github.com/pnpm/pnpm/pull/5151). diff --git a/packages/lockfile-file/src/sortLockfileKeys.ts b/packages/lockfile-file/src/sortLockfileKeys.ts index 8c1e11b4043..bdc3657430a 100644 --- a/packages/lockfile-file/src/sortLockfileKeys.ts +++ b/packages/lockfile-file/src/sortLockfileKeys.ts @@ -53,7 +53,10 @@ function compareWithPriority (priority: Record, left: string, ri if (leftPriority && rightPriority) return leftPriority - rightPriority if (leftPriority) return -1 if (rightPriority) return 1 - return left.localeCompare(right) + // We want deterministic sorting, so we can't use .localCompare here. + // comparing strings with < and > will produce the same result on each machine. + // An alternative solution could be to use a specific culture for compare, using Intl.Collator + return left < right ? -1 : (left > right ? 1 : 0) } export function sortLockfileKeys (lockfile: LockfileFile) { diff --git a/packages/lockfile-file/test/sortLockfileKeys.test.ts b/packages/lockfile-file/test/sortLockfileKeys.test.ts new file mode 100644 index 00000000000..dce970ed9c1 --- /dev/null +++ b/packages/lockfile-file/test/sortLockfileKeys.test.ts @@ -0,0 +1,100 @@ +import { LOCKFILE_VERSION } from '@pnpm/constants' +import { sortLockfileKeys } from '../lib/sortLockfileKeys' + +test('sorts keys alphabetically', () => { + const normalizedLockfile = sortLockfileKeys({ + lockfileVersion: LOCKFILE_VERSION, + importers: { + foo: { + dependencies: { + zzz: 'link:../zzz', + bar: 'link:../bar', + aaa: 'link:../aaa', + }, + specifiers: { + zzz: 'link:../zzz', + bar: 'link:../bar', + aaa: 'link:../aaa', + }, + }, + bar: { + specifiers: { + baz: 'link:../baz', + }, + dependencies: { + baz: 'link:../baz', + }, + }, + }, + }) + + expect(normalizedLockfile).toStrictEqual({ + lockfileVersion: LOCKFILE_VERSION, + importers: { + bar: { + dependencies: { + baz: 'link:../baz', + }, + specifiers: { + baz: 'link:../baz', + }, + }, + foo: { + dependencies: { + aaa: 'link:../aaa', + bar: 'link:../bar', + zzz: 'link:../zzz', + }, + specifiers: { + aaa: 'link:../aaa', + bar: 'link:../bar', + zzz: 'link:../zzz', + }, + }, + }, + }) + expect(Object.keys(normalizedLockfile.importers?.foo.dependencies ?? {})).toStrictEqual(['aaa', 'bar', 'zzz']) + expect(Object.keys(normalizedLockfile.importers?.foo.specifiers ?? {})).toStrictEqual(['aaa', 'bar', 'zzz']) +}) + +test('sorting does not care about locale (e.g. Czech has "ch" as a single character after "h")', () => { + // The input is properly sorted according to Czech locale. + const normalizedLockfile = sortLockfileKeys({ + lockfileVersion: LOCKFILE_VERSION, + importers: { + foo: { + dependencies: { + bar: 'link:../bar', + href: 'link:../href', + chmod: 'link:../chmod', + }, + specifiers: { + bar: 'link:../bar', + href: 'link:../href', + chmod: 'link:../chmod', + }, + }, + }, + }) + + // The result should be the same as on other machines using whatever locale, e.g. English. + expect(normalizedLockfile).toStrictEqual({ + lockfileVersion: LOCKFILE_VERSION, + importers: { + foo: { + dependencies: { + bar: 'link:../bar', + chmod: 'link:../chmod', + href: 'link:../href', + }, + specifiers: { + bar: 'link:../bar', + chmod: 'link:../chmod', + href: 'link:../href', + }, + }, + }, + }) + expect(Object.keys(normalizedLockfile.importers?.foo.dependencies ?? {})).toStrictEqual(['bar', 'chmod', 'href']) + expect(Object.keys(normalizedLockfile.importers?.foo.specifiers ?? {})).toStrictEqual(['bar', 'chmod', 'href']) +})