fix(lockfile): change sorting of keys in lockfile (#5151)

* fix(lockfile): change sorting of keys in lockfile

Make it more deterministic and prevent unnecessary churn in the lockfile.
It is useful when looking for affected projects after changes to the lockfile.
When the changes happen among various developers using various locale,
the locale specific sorting causes changes in the lockfile for extraneous projects.
Therefore, it could cause running unnecessary CI jobs, builds, releases and other
work that would not be needed otherwise.

* chore: add comment

Co-authored-by: Zoltan Kochan <z@kochan.io>

.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).

packages/lockfile-file/src/sortLockfileKeys.ts

@@ -53,7 +53,10 @@ function compareWithPriority (priority: Record<string, number>, 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) {

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'])
+})