fix: Fix versions-check cli command

Signed-off-by: Gustaf Räntilä <g.rantila@gmail.com>

.changeset/wild-doors-cheat.md

@@ -0,0 +1,5 @@
+---
+'@backstage/cli': patch
+---
+
+Fix `versions:check --fix` when `yarn.lock` has multiple joint versions in the same section

packages/cli-node/package.json

@@ -35,7 +35,7 @@
     "@backstage/errors": "workspace:^",
     "@backstage/types": "workspace:^",
     "@manypkg/get-packages": "^1.1.3",
-    "@yarnpkg/parsers": "^3.0.0-rc.4",
+    "@yarnpkg/parsers": "^3.0.0",
     "fs-extra": "^11.2.0",
     "semver": "^7.5.3",
     "zod": "^3.22.4"

packages/cli/package.json

@@ -79,7 +79,7 @@
     "@typescript-eslint/eslint-plugin": "^6.12.0",
     "@typescript-eslint/parser": "^6.7.2",
     "@yarnpkg/lockfile": "^1.1.0",
-    "@yarnpkg/parsers": "^3.0.0-rc.4",
+    "@yarnpkg/parsers": "^3.0.0",
     "bfj": "^8.0.0",
     "buffer": "^6.0.3",
     "chalk": "^4.0.0",

packages/cli/src/lib/versioning/Lockfile.ts

@@ -98,6 +98,24 @@ export class Lockfile {
     return Lockfile.parse(lockfileContents);
   }
 
+  static #getRangesFromDataKey(key: string): string[] {
+    const [, name, ranges] = key.match(ENTRY_PATTERN) ?? [];
+    if (!name) {
+      throw new Error(`Failed to parse yarn.lock entry '${key}'`);
+    }
+
+    return ranges.split(/\s*,\s*/).map(rangePart => {
+      let range = rangePart;
+      if (range.startsWith(`${name}@`)) {
+        range = range.slice(`${name}@`.length);
+      }
+      if (range.startsWith('npm:')) {
+        range = range.slice('npm:'.length);
+      }
+      return range;
+    });
+  }
+
   static parse(content: string) {
     const legacy = LEGACY_REGEX.test(content);
 
@@ -113,7 +131,7 @@ export class Lockfile {
     for (const [key, value] of Object.entries(data)) {
       if (SPECIAL_OBJECT_KEYS.includes(key)) continue;
 
-      const [, name, ranges] = ENTRY_PATTERN.exec(key) ?? [];
+      const [, name] = ENTRY_PATTERN.exec(key) ?? [];
       if (!name) {
         throw new Error(`Failed to parse yarn.lock entry '${key}'`);
       }
@@ -123,13 +141,8 @@ export class Lockfile {
         queries = [];
         packages.set(name, queries);
       }
-      for (let range of ranges.split(/\s*,\s*/)) {
-        if (range.startsWith(`${name}@`)) {
-          range = range.slice(`${name}@`.length);
-        }
-        if (range.startsWith('npm:')) {
-          range = range.slice('npm:'.length);
-        }
+      const ranges = Lockfile.#getRangesFromDataKey(key);
+      for (const range of ranges) {
         queries.push({ range, version: value.version, dataKey: key });
       }
     }
@@ -276,9 +289,34 @@ export class Lockfile {
   }
 
   remove(name: string, range: string): boolean {
-    const query = `${name}@${range}`;
+    const simpleQuery = `${name}@${range}`;
+    const query = this.getEntryOf(name, range);
+
     const existed = Boolean(this.data[query]);
-    delete this.data[query];
+
+    if (simpleQuery === query) {
+      // Single-versioned entry, just delete
+      delete this.data[query];
+    } else {
+      // Remove this version from the entry key. This modifies the key, so the
+      // package queries' <dataKey> needs to be updated too.
+      const newRanges = Lockfile.#getRangesFromDataKey(query).filter(
+        q => q !== simpleQuery,
+      );
+      const newQuery = newRanges.join(', ');
+
+      // Replace the entry with a new one without this particular range
+      const entry = this.data[query];
+      delete this.data[query];
+      this.data[newQuery] = entry;
+
+      // Fix all package queries pointing to the old query
+      this.packages.get(name)?.forEach(q => {
+        if (q.dataKey === query) {
+          q.dataKey = newQuery;
+        }
+      });
+    }
 
     const newEntries = this.packages.get(name)?.filter(e => e.range !== range);
     if (newEntries) {
@@ -288,19 +326,35 @@ export class Lockfile {
     return existed;
   }
 
+  getEntryOf(name: string, range: string) {
+    const query = this.packages.get(name)?.find(q => q.range === range);
+    if (!query) {
+      throw new Error(`No entry data for ${name}@${range}`);
+    }
+
+    return query.dataKey;
+  }
+
   /** Modifies the lockfile by bumping packages to the suggested versions */
   replaceVersions(results: AnalyzeResultNewVersion[]) {
+    // When replacing versions, we might replace the same version multiple times,
+    // as a query may contain multiple versions. This keeps the original version,
+    // to ensure we don't make mistakes.
+    const oldVersions = Object.fromEntries(
+      Object.entries(this.data).map(([key, val]) => [key, val.version]),
+    );
+
     for (const { name, range, oldVersion, newVersion } of results) {
-      const query = `${name}@${range}`;
+      const query = this.getEntryOf(name, range);
 
       // Update the backing data
       const entryData = this.data[query];
       if (!entryData) {
         throw new Error(`No entry data for ${query}`);
       }
-      if (entryData.version !== oldVersion) {
+      if (oldVersions[query] !== oldVersion) {
         throw new Error(
-          `Expected existing version data for ${query} to be ${oldVersion}, was ${entryData.version}`,
+          `Expected existing version data for ${query} to be ${oldVersion}, was ${oldVersions[query]}`,
         );
       }
 

plugins/devtools-backend/package.json

@@ -41,7 +41,7 @@
     "@manypkg/get-packages": "^1.1.3",
     "@types/express": "*",
     "@yarnpkg/lockfile": "^1.1.0",
-    "@yarnpkg/parsers": "^3.0.0-rc.4",
+    "@yarnpkg/parsers": "^3.0.0",
     "express": "^4.18.1",
     "express-promise-router": "^4.1.0",
     "fs-extra": "^11.0.0",

yarn.lock

@@ -3642,7 +3642,7 @@ __metadata:
     "@backstage/errors": "workspace:^"
     "@backstage/types": "workspace:^"
     "@manypkg/get-packages": ^1.1.3
-    "@yarnpkg/parsers": ^3.0.0-rc.4
+    "@yarnpkg/parsers": ^3.0.0
     fs-extra: ^11.2.0
     semver: ^7.5.3
     zod: ^3.22.4
@@ -3716,7 +3716,7 @@ __metadata:
     "@typescript-eslint/parser": ^6.7.2
     "@vitejs/plugin-react": ^4.0.4
     "@yarnpkg/lockfile": ^1.1.0
-    "@yarnpkg/parsers": ^3.0.0-rc.4
+    "@yarnpkg/parsers": ^3.0.0
     bfj: ^8.0.0
     buffer: ^6.0.3
     chalk: ^4.0.0
@@ -5777,7 +5777,7 @@ __metadata:
     "@types/supertest": ^2.0.8
     "@types/yarnpkg__lockfile": ^1.1.4
     "@yarnpkg/lockfile": ^1.1.0
-    "@yarnpkg/parsers": ^3.0.0-rc.4
+    "@yarnpkg/parsers": ^3.0.0
     express: ^4.18.1
     express-promise-router: ^4.1.0
     fs-extra: ^11.0.0
@@ -19325,13 +19325,13 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@yarnpkg/parsers@npm:^3.0.0-rc.4":
-  version: 3.0.0-rc.21
-  resolution: "@yarnpkg/parsers@npm:3.0.0-rc.21"
+"@yarnpkg/parsers@npm:^3.0.0":
+  version: 3.0.0
+  resolution: "@yarnpkg/parsers@npm:3.0.0"
   dependencies:
     js-yaml: ^3.10.0
     tslib: ^2.4.0
-  checksum: c0741ef01089a7d452dfb75b8c24a82ddcc3e218dac22b794a06f2e50c7070378c460f63f63fb7ed0f62f634114e448e3d85a75ccd4db84e1526242ae9b4a7d5
+  checksum: fefe5ecafb5bfa2b678ac9ba9259810fdda40142afd9d0b7e0e5cc1cce1fd824dffc52217c5e429807481d8fd18ead074bd317e64fd626335d3c9f1a320bade2
   languageName: node
   linkType: hard