Simplify some logic in bump and publish (#866)

change/beachball-e71eeee6-a3d6-4cc5-9205-a53d327dcf03.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Simplify some logic in bump and publish",
+  "packageName": "beachball",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

scripts/jestSetup.js

@@ -1,6 +1,10 @@
 // @ts-check
-const { jest } = require('@jest/globals');
+const { jest, afterAll } = require('@jest/globals');
 
 jest.spyOn(process, 'exit').mockImplementation(code => {
   throw new Error(`process.exit called with code ${code}`);
 });
+
+afterAll(() => {
+  jest.restoreAllMocks();
+});

src/bump/performBump.ts

@@ -4,7 +4,7 @@ import { unlinkChangeFiles } from '../changefile/unlinkChangeFiles';
 import { writeChangelog } from '../changelog/writeChangelog';
 import { BumpInfo } from '../types/BumpInfo';
 import { BeachballOptions, HooksOptions } from '../types/BeachballOptions';
-import { PackageDeps, PackageInfos } from '../types/PackageInfo';
+import { PackageInfos, PackageJson } from '../types/PackageInfo';
 import { findProjectRoot } from 'workspace-tools';
 import { npm } from '../packageManager/npm';
 
@@ -15,28 +15,26 @@ export function writePackageJson(modifiedPackages: Set<string>, packageInfos: Pa
       console.warn(`Skipping ${pkgName} since package.json does not exist`);
       continue;
     }
-    const packageJson = fs.readJSONSync(info.packageJsonPath);
+    const packageJson: PackageJson = fs.readJSONSync(info.packageJsonPath);
 
     if (!info.private) {
       packageJson.version = info.version;
     }
 
-    ['dependencies', 'devDependencies', 'peerDependencies'].forEach(depKind => {
+    for (const depKind of ['dependencies', 'devDependencies', 'peerDependencies'] as const) {
       // updatedDeps contains all of the dependencies in the bump info since the beginning of a build job
-      const updatedDepsVersions: PackageDeps | undefined = (info as any)[depKind];
+      const updatedDepsVersions = info[depKind];
       if (updatedDepsVersions) {
         // to be cautious, only update internal && modifiedPackages, since some other dependency
         // changes could have occurred since the beginning of the build job and the next merge step
         // would overwrite those incorrectly!
-        const modifiedDeps = Object.keys(updatedDepsVersions).filter(dep => modifiedPackages.has(dep));
-
-        for (const dep of modifiedDeps) {
-          if (packageJson[depKind] && packageJson[depKind][dep]) {
-            packageJson[depKind][dep] = updatedDepsVersions[dep];
+        for (const [dep, updatedVersion] of Object.entries(updatedDepsVersions)) {
+          if (modifiedPackages.has(dep) && packageJson[depKind]?.[dep]) {
+            packageJson[depKind]![dep] = updatedVersion;
           }
         }
       }
-    });
+    }
 
     fs.writeJSONSync(info.packageJsonPath, packageJson, { spaces: 2 });
   }

src/bump/setDependentVersions.ts

@@ -9,37 +9,37 @@ export function setDependentVersions(
 ) {
   const dependentChangedBy: { [dependent: string]: Set<string> } = {};
 
-  Object.keys(packageInfos).forEach(pkgName => {
+  for (const [pkgName, info] of Object.entries(packageInfos)) {
     if (!scopedPackages.has(pkgName)) {
-      return;
+      continue;
     }
 
-    const info = packageInfos[pkgName];
-    const depTypes = ['dependencies', 'devDependencies', 'peerDependencies'] as const;
-    depTypes.forEach(depKind => {
-      const deps = info[depKind];
-      if (deps) {
-        Object.keys(deps).forEach(dep => {
-          const packageInfo = packageInfos[dep];
-          if (packageInfo) {
-            const existingVersionRange = deps[dep];
-            const bumpedVersionRange = bumpMinSemverRange(packageInfo.version, existingVersionRange);
-            if (existingVersionRange !== bumpedVersionRange) {
-              deps[dep] = bumpedVersionRange;
+    for (const deps of [info.dependencies, info.devDependencies, info.peerDependencies]) {
+      if (!deps) {
+        continue;
+      }
+
+      for (const [dep, existingVersionRange] of Object.entries(deps)) {
+        const packageInfo = packageInfos[dep];
+        if (!packageInfo) {
+          continue;
+        }
 
-              dependentChangedBy[pkgName] = dependentChangedBy[pkgName] || new Set<string>();
-              dependentChangedBy[pkgName].add(dep);
-              if (verbose) {
-                console.log(
-                  `${pkgName} needs to be bumped because ${dep} ${existingVersionRange} -> ${bumpedVersionRange}`
-                );
-              }
-            }
+        const bumpedVersionRange = bumpMinSemverRange(packageInfo.version, existingVersionRange);
+        if (existingVersionRange !== bumpedVersionRange) {
+          deps[dep] = bumpedVersionRange;
+
+          dependentChangedBy[pkgName] ??= new Set<string>();
+          dependentChangedBy[pkgName].add(dep);
+          if (verbose) {
+            console.log(
+              `${pkgName} needs to be bumped because ${dep} ${existingVersionRange} -> ${bumpedVersionRange}`
+            );
           }
-        });
+        }
       }
-    });
-  });
+    }
+  }
 
   return dependentChangedBy;
 }

src/bump/setDependentsInBumpInfo.ts

@@ -7,30 +7,24 @@ import type { BumpInfo } from '../types/BumpInfo';
  */
 export function setDependentsInBumpInfo(bumpInfo: BumpInfo): void {
   const { packageInfos, scopedPackages } = bumpInfo;
-  const packages = Object.keys(packageInfos);
   const dependents: BumpInfo['dependents'] = {};
 
-  packages.forEach(pkgName => {
+  for (const [pkgName, info] of Object.entries(packageInfos)) {
     if (!scopedPackages.has(pkgName)) {
-      return;
+      continue;
     }
 
-    const info = packageInfos[pkgName];
-    const depTypes = ['dependencies', 'devDependencies', 'peerDependencies'] as const;
-    depTypes.forEach(depType => {
-      const deps = info[depType];
-      if (deps) {
-        for (let dep of Object.keys(deps)) {
-          if (packages.includes(dep)) {
-            dependents[dep] = dependents[dep] || [];
-            if (!dependents[dep].includes(pkgName)) {
-              dependents[dep].push(pkgName);
-            }
+    for (const deps of [info.dependencies, info.devDependencies, info.peerDependencies]) {
+      for (const dep of Object.keys(deps || {})) {
+        if (packageInfos[dep]) {
+          dependents[dep] ??= [];
+          if (!dependents[dep].includes(pkgName)) {
+            dependents[dep].push(pkgName);
           }
         }
       }
-    });
-  });
+    }
+  }
 
   bumpInfo.dependents = dependents;
 }

src/publish/toposortPackages.ts

@@ -1,8 +1,8 @@
 import toposort from 'toposort';
-import { PackageInfos, PackageDeps } from '../types/PackageInfo';
+import { PackageInfos } from '../types/PackageInfo';
 
 /**
- * Topological sort the packages based on its dependency graph.
+ * Topologically sort the packages based on their dependency graph.
  * Dependency comes first before dependent.
  * @param packages Packages to be sorted.
  * @param packageInfos PackagesInfos for the sorted packages.
@@ -11,35 +11,29 @@ export function toposortPackages(packages: string[], packageInfos: PackageInfos)
   const packageSet = new Set(packages);
   const dependencyGraph: [string | undefined, string][] = [];
 
-  packages.forEach(pkgName => {
-    let allDeps: string[] = [];
-
-    ['dependencies', 'devDependencies', 'peerDependencies'].forEach(depKind => {
-      const info = packageInfos[pkgName];
-      if (!info) {
-        throw new Error(`Package info is missing for ${pkgName}.`);
-      }
-
-      const deps: PackageDeps | undefined = (info as any)[depKind];
-      if (deps) {
-        const depPkgNames = Object.keys(deps);
-        allDeps = allDeps.concat(depPkgNames);
-      }
-    });
+  for (const pkgName of packageSet) {
+    const info = packageInfos[pkgName];
+    if (!info) {
+      throw new Error(`Package info is missing for ${pkgName}.`);
+    }
 
-    allDeps = [...new Set(allDeps)].filter(pkg => packageSet.has(pkg));
-    if (allDeps.length > 0) {
-      allDeps.forEach(depPkgName => {
+    const allDeps = new Set(
+      [info.dependencies, info.devDependencies, info.peerDependencies]
+        .flatMap(deps => Object.keys(deps || {}))
+        .filter(pkg => packageSet.has(pkg))
+    );
+    if (allDeps.size) {
+      for (const depPkgName of allDeps) {
         dependencyGraph.push([depPkgName, pkgName]);
-      });
+      }
     } else {
       dependencyGraph.push([undefined, pkgName]);
     }
-  });
+  }
 
   try {
     return toposort(dependencyGraph).filter((pkg): pkg is string => !!pkg);
   } catch (err) {
-    throw new Error(`Failed to do toposort for packages: ${err?.message}`);
+    throw new Error(`Failed to topologically sort packages: ${err?.message}`);
   }
 }