Ensure mismatching versions are not ignroed for moved plugins

Signed-off-by: Philipp Hugenroth <philipph@spotify.com>

packages/cli/src/commands/versions/bump.ts

@@ -14,32 +14,32 @@
  * limitations under the License.
  */
 
-import fs from 'fs-extra';
-import chalk from 'chalk';
-import ora from 'ora';
-import semver from 'semver';
-import { minimatch } from 'minimatch';
-import { OptionValues } from 'commander';
-import { isError, NotFoundError } from '@backstage/errors';
-import { resolve as resolvePath } from 'path';
-import { run } from '../../lib/run';
-import { paths } from '../../lib/paths';
-import {
-  mapDependencies,
-  fetchPackageInfo,
-  Lockfile,
-  YarnInfoInspectData,
-} from '../../lib/versioning';
-import { forbiddenDuplicatesFilter } from './lint';
 import { BACKSTAGE_JSON } from '@backstage/cli-common';
-import { runParallelWorkers } from '../../lib/parallel';
+import { PackageGraph } from '@backstage/cli-node';
+import { isError, NotFoundError } from '@backstage/errors';
 import {
   getManifestByReleaseLine,
   getManifestByVersion,
   ReleaseManifest,
 } from '@backstage/release-manifests';
+import chalk from 'chalk';
+import { OptionValues } from 'commander';
+import fs from 'fs-extra';
 import 'global-agent/bootstrap';
-import { PackageGraph } from '@backstage/cli-node';
+import { minimatch } from 'minimatch';
+import ora from 'ora';
+import { resolve as resolvePath } from 'path';
+import semver from 'semver';
+import { runParallelWorkers } from '../../lib/parallel';
+import { paths } from '../../lib/paths';
+import { run } from '../../lib/run';
+import {
+  fetchPackageInfo,
+  Lockfile,
+  mapDependencies,
+  YarnInfoInspectData,
+} from '../../lib/versioning';
+import { forbiddenDuplicatesFilter } from './lint';
 
 const DEP_TYPES = [
   'dependencies',
@@ -108,6 +108,8 @@ export default async (opts: OptionValues) => {
   const versionBumps = new Map<string, PkgVersionInfo[]>();
   // Track package versions that we want to remove from yarn.lock in order to trigger a bump
   const unlocked = Array<{ name: string; range: string; target: string }>();
+  // All backstage package versions in yarn.lock that are not in `unlocked`
+  const outsideRange = Array<{ name: string; range: string; target: string }>();
   // Moved packages & their new package version info
   const moved = new Map<
     string,
@@ -170,6 +172,7 @@ export default async (opts: OptionValues) => {
         // Ignore lockfile entries that don't satisfy the version range, since
         // these can't cause the package to be locked to an older version
         if (!semver.satisfies(target, entry.range)) {
+          outsideRange.push({ name, range: entry.range, target });
           continue;
         }
         // Unlock all entries that are within range but on the old version
@@ -180,31 +183,31 @@ export default async (opts: OptionValues) => {
 
   await runParallelWorkers({
     parallelismFactor: 4,
-    items: unlocked,
+    items: [...unlocked, ...outsideRange],
     async worker({ name }) {
-      let info = await fetchPackageInfo(name);
+      if (!moved.get(name)) {
+        let info = await fetchPackageInfo(name);
 
-      // TODO: To be removed
-      if (name === '@backstage/plugin-catalog-react') {
-        info.backstage = {
-          ...info.backstage,
-          moved: '@pagerduty/backstage-plugin',
-        };
-      }
+        // TODO: To be removed
+        if (name === '@backstage/plugin-catalog-react') {
+          info.backstage = {
+            ...info.backstage,
+            moved: '@pagerduty/backstage-plugin',
+          };
+        }
 
-      if (info.backstage?.moved) {
-        info = await fetchPackageInfo(info.backstage.moved);
-        moved.set(name, {
-          name: info.name,
-          range: `^${info['dist-tags'].latest}`,
-          target: info['dist-tags'].latest,
-        });
+        if (info.backstage?.moved) {
+          info = await fetchPackageInfo(info.backstage.moved);
+          moved.set(name, {
+            name: info.name,
+            range: `^${info['dist-tags'].latest}`,
+            target: info['dist-tags'].latest,
+          });
+        }
       }
     },
   });
 
-  console.log();
-
   // Write all discovered version bumps to package.json in this repo
   if (versionBumps.size === 0 && unlocked.length === 0) {
     console.log(chalk.green('All Backstage packages are up to date!'));
@@ -212,8 +215,9 @@ export default async (opts: OptionValues) => {
     console.log(chalk.yellow('Some packages are outdated, updating'));
     console.log();
 
-    if (unlocked.length > 0) {
+    if (unlocked.length > 0 || outsideRange.length > 0) {
       const removed = new Set<string>();
+
       for (const unlockedPackage of unlocked) {
         const movedPackage = moved.get(unlockedPackage.name);
         const { name, range, target } = movedPackage
@@ -245,6 +249,25 @@ export default async (opts: OptionValues) => {
           }
         }
       }
+
+      for (const outsideRangePackage of outsideRange) {
+        const movedPackage = moved.get(outsideRangePackage.name);
+        if (movedPackage) {
+          const { name, range } = movedPackage;
+          const key = JSON.stringify({ name, range });
+          if (!removed.has(key)) {
+            removed.add(key);
+            // TODO: Does this need special instructions as there might be custom bumping needed?
+            console.log(
+              `${chalk.yellow('moving')} ${movedPackage.name}@${
+                movedPackage.range
+              } ~> ${chalk.yellow(`${name}@${range}`)}`,
+            );
+            lockfile.remove(movedPackage.name, movedPackage.range);
+          }
+        }
+      }
+
       await lockfile.save(lockfilePath);
     }