diff --git a/.changeset/nasty-views-sniff.md b/.changeset/nasty-views-sniff.md new file mode 100644 index 0000000000..a6b1b08b6d --- /dev/null +++ b/.changeset/nasty-views-sniff.md @@ -0,0 +1,7 @@ +--- +"@pnpm/lockfile.verification": minor +"pnpm": patch +"@pnpm/tsconfig": patch +--- + +Improve the way the error message displays mismatched specifiers. Show differences instead of 2 whole objects [#9598](https://github.com/pnpm/pnpm/pull/9598). diff --git a/__utils__/tsconfig/tsconfig.json b/__utils__/tsconfig/tsconfig.json index a853803ff2..3ea46e3dd4 100644 --- a/__utils__/tsconfig/tsconfig.json +++ b/__utils__/tsconfig/tsconfig.json @@ -12,7 +12,7 @@ "removeComments": false, "sourceMap": true, "strict": true, - "target": "es2021" + "target": "es2022" }, "atom": { "rewriteTsconfig": true diff --git a/lockfile/verification/src/diffFlatRecords.ts b/lockfile/verification/src/diffFlatRecords.ts new file mode 100644 index 0000000000..6a89f820da --- /dev/null +++ b/lockfile/verification/src/diffFlatRecords.ts @@ -0,0 +1,47 @@ +export interface AddedRemoved { + key: Key + value: Value +} + +export interface Modified { + key: Key + left: Value + right: Value +} + +export interface Diff { + /** Entries that are absent in `left` but present in `right`. */ + added: Array> + /** Entries that are present in `left` but absent in `right`. */ + removed: Array> + /** Entries that are present in both `left` and `right` but the values are different. */ + modified: Array> +} + +export function diffFlatRecords (left: Record, right: Record): Diff { + const result: Diff = { + added: [], + removed: [], + modified: [], + } + + for (const [key, value] of Object.entries(left) as Array<[Key, Value]>) { + if (!Object.hasOwn(right, key)) { + result.removed.push({ key, value }) + } else if (value !== right[key]) { + result.modified.push({ key, left: value, right: right[key] }) + } + } + + for (const [key, value] of Object.entries(right) as Array<[Key, Value]>) { + if (!Object.hasOwn(left, key)) { + result.added.push({ key, value }) + } + } + + return result +} + +export function isEqual ({ added, removed, modified }: Diff): boolean { + return added.length === 0 && removed.length === 0 && modified.length === 0 +} diff --git a/lockfile/verification/src/satisfiesPackageManifest.ts b/lockfile/verification/src/satisfiesPackageManifest.ts index af35ab8998..f530095cc4 100644 --- a/lockfile/verification/src/satisfiesPackageManifest.ts +++ b/lockfile/verification/src/satisfiesPackageManifest.ts @@ -6,6 +6,7 @@ import { import equals from 'ramda/src/equals' import pickBy from 'ramda/src/pickBy' import omit from 'ramda/src/omit' +import { type Diff, diffFlatRecords, isEqual } from './diffFlatRecords' export function satisfiesPackageManifest ( opts: { @@ -36,10 +37,11 @@ export function satisfiesPackageManifest ( existingDeps = pickNonLinkedDeps(existingDeps) specs = pickNonLinkedDeps(specs) } - if (!equals(existingDeps, specs)) { + const specsDiff = diffFlatRecords(specs, existingDeps) + if (!isEqual(specsDiff)) { return { satisfies: false, - detailedReason: `specifiers in the lockfile (${JSON.stringify(specs)}) don't match specs in package.json (${JSON.stringify(existingDeps)})`, + detailedReason: `specifiers in the lockfile don't match specifiers in package.json:\n${displaySpecDiff(specsDiff)}`, } } if (importer.publishDirectory !== pkg.publishConfig?.directory) { @@ -101,3 +103,28 @@ export function satisfiesPackageManifest ( function countOfNonLinkedDeps (lockfileDeps: { [depName: string]: string }): number { return Object.values(lockfileDeps).filter((ref) => !ref.includes('link:') && !ref.includes('file:')).length } + +function displaySpecDiff ({ added, removed, modified }: Diff): string { + let result = '' + + if (added.length !== 0) { + result += `* ${added.length} dependencies were added: ` + result += added.map(({ key, value }) => `${key}@${value}`).join(', ') + result += '\n' + } + + if (removed.length !== 0) { + result += `* ${removed.length} dependencies were removed: ` + result += removed.map(({ key, value }) => `${key}@${value}`).join(', ') + result += '\n' + } + + if (modified.length !== 0) { + result += `* ${modified.length} dependencies are mismatched:\n` + for (const { key, left, right } of modified) { + result += ` - ${key} (lockfile: ${left}, manifest: ${right})\n` + } + } + + return result +} diff --git a/lockfile/verification/test/diffFlatRecords.test.ts b/lockfile/verification/test/diffFlatRecords.test.ts new file mode 100644 index 0000000000..7a9987c68e --- /dev/null +++ b/lockfile/verification/test/diffFlatRecords.test.ts @@ -0,0 +1,26 @@ +import { type Diff, diffFlatRecords } from '../src/diffFlatRecords' + +test('diffFlatRecords', () => { + const diff = diffFlatRecords({ + 'is-positive': '1.0.0', + 'is-negative': '2.0.0', + }, { + 'is-negative': '2.1.0', + 'is-odd': '1.0.0', + }) + expect(diff).toStrictEqual({ + added: [{ + key: 'is-odd', + value: '1.0.0', + }], + removed: [{ + key: 'is-positive', + value: '1.0.0', + }], + modified: [{ + key: 'is-negative', + left: '2.0.0', + right: '2.1.0', + }], + } as Diff) +}) diff --git a/lockfile/verification/test/satisfiesPackageManifest.ts b/lockfile/verification/test/satisfiesPackageManifest.ts index 1c962c7908..308da4c4fd 100644 --- a/lockfile/verification/test/satisfiesPackageManifest.ts +++ b/lockfile/verification/test/satisfiesPackageManifest.ts @@ -77,7 +77,10 @@ test('satisfiesPackageManifest()', () => { } )).toStrictEqual({ satisfies: false, - detailedReason: 'specifiers in the lockfile ({"foo":"^1.0.0"}) don\'t match specs in package.json ({"foo":"^1.1.0"})', + detailedReason: `specifiers in the lockfile don't match specifiers in package.json: +* 1 dependencies are mismatched: + - foo (lockfile: ^1.0.0, manifest: ^1.1.0) +`, }) expect(satisfiesPackageManifest( {}, @@ -91,7 +94,9 @@ test('satisfiesPackageManifest()', () => { } )).toStrictEqual({ satisfies: false, - detailedReason: 'specifiers in the lockfile ({"foo":"^1.0.0"}) don\'t match specs in package.json ({"foo":"^1.0.0","bar":"2.0.0"})', + detailedReason: `specifiers in the lockfile don't match specifiers in package.json: +* 1 dependencies were added: bar@2.0.0 +`, }) expect(satisfiesPackageManifest( @@ -154,7 +159,9 @@ test('satisfiesPackageManifest()', () => { } expect(satisfiesPackageManifest({}, importer, pkg)).toStrictEqual({ satisfies: false, - detailedReason: 'specifiers in the lockfile ({"bar":"2.0.0","qar":"^1.0.0"}) don\'t match specs in package.json ({"bar":"2.0.0"})', + detailedReason: `specifiers in the lockfile don't match specifiers in package.json: +* 1 dependencies were removed: qar@^1.0.0 +`, }) }