Remove dependents from BumpInfo + use shared dep type list (#988)

change/beachball-b14f577b-da9c-4db3-9cd8-f0d6fba25af8.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Use a shared list of considered dependency types",
+  "packageName": "beachball",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch"
+}

src/__tests__/bump/updateRelatedChangeType.test.ts

@@ -19,7 +19,6 @@ describe('updateRelatedChangeType', () => {
     _.merge<BumpInfo, PartialBumpInfo>(
       {
         changeFileChangeInfos: [],
-        dependents: {},
         calculatedChangeTypes: {},
         packageInfos: makePackageInfos({
           foo: { combinedOptions: { disallowedChangeTypes: [], defaultNpmTag: 'latest' } },
@@ -46,10 +45,8 @@ describe('updateRelatedChangeType', () => {
   };
 
   it('should bump dependent packages with "patch" change type by default', () => {
+    const dependents = { foo: ['bar'] };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        foo: ['bar'],
-      },
       changeFileChangeInfos: [
         { changeFile: 'foo.json', change: { ...changeInfoFixture, type: 'minor', dependentChangeType: 'patch' } },
       ],
@@ -61,17 +58,15 @@ describe('updateRelatedChangeType', () => {
       },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBe('minor');
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('patch');
   });
 
   it('should bump dependent packages according to the dependentChangeTypes', () => {
+    const dependents = { foo: ['bar'] };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        foo: ['bar'],
-      },
       changeFileChangeInfos: [
         { changeFile: 'foo.json', change: { ...changeInfoFixture, type: 'patch', dependentChangeType: 'minor' } },
       ],
@@ -83,18 +78,18 @@ describe('updateRelatedChangeType', () => {
       },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBe('patch');
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('minor');
   });
 
   it("should bump dependent packages according to the bumpInfo.dependentChangeTypes and respect package's own change type", () => {
+    const dependents = {
+      foo: ['bar'],
+      bar: ['app'],
+    };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        foo: ['bar'],
-        bar: ['app'],
-      },
       changeFileChangeInfos: [
         {
           changeFile: 'foo.json',
@@ -115,8 +110,8 @@ describe('updateRelatedChangeType', () => {
       },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
-    updateRelatedChangeType('bar.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents, bumpDeps: true });
+    updateRelatedChangeType({ changeFile: 'bar.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBe('patch');
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('major');
@@ -124,12 +119,12 @@ describe('updateRelatedChangeType', () => {
   });
 
   it('should bump dependent packages according to the bumpInfo.dependentChangeTypes and dependentChangeInfos must stay up to date', () => {
+    const dependents = {
+      foo: ['bar'],
+      baz: ['bar'],
+      bar: ['app'],
+    };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        foo: ['bar'],
-        baz: ['bar'],
-        bar: ['app'],
-      },
       changeFileChangeInfos: [
         {
           changeFile: 'foo.json',
@@ -156,25 +151,25 @@ describe('updateRelatedChangeType', () => {
       },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBe('patch');
     expect(bumpInfo.calculatedChangeTypes['baz']).toBe('minor');
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('patch');
     expect(bumpInfo.calculatedChangeTypes['app']).toBe('patch');
 
-    updateRelatedChangeType('baz.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'baz.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('minor');
   });
 
   it('should bump dependent packages according to the bumpInfo.dependentChangeTypes and roll-up multiple change infos', () => {
+    const dependents = {
+      foo: ['bar'],
+      bar: ['app'],
+      baz: ['bar', 'app'],
+    };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        foo: ['bar'],
-        bar: ['app'],
-        baz: ['bar', 'app'],
-      },
       changeFileChangeInfos: [
         { changeFile: 'foo.json', change: { ...changeInfoFixture, type: 'patch', dependentChangeType: 'major' } },
         { changeFile: 'baz.json', change: { ...changeInfoFixture, type: 'patch', dependentChangeType: 'minor' } },
@@ -189,8 +184,8 @@ describe('updateRelatedChangeType', () => {
       },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
-    updateRelatedChangeType('baz.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents, bumpDeps: true });
+    updateRelatedChangeType({ changeFile: 'baz.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBe('patch');
     expect(bumpInfo.calculatedChangeTypes['baz']).toBe('patch');
@@ -212,7 +207,7 @@ describe('updateRelatedChangeType', () => {
       ],
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents: {}, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('minor');
     expect(bumpInfo.calculatedChangeTypes['unrelated']).toBeUndefined();
@@ -232,7 +227,7 @@ describe('updateRelatedChangeType', () => {
       packageGroups: { grp: { packageNames: ['foo', 'bar'] } },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents: {}, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('patch');
     expect(bumpInfo.calculatedChangeTypes['unrelated']).toBeUndefined();
@@ -252,17 +247,15 @@ describe('updateRelatedChangeType', () => {
       packageGroups: { grp: { packageNames: ['foo', 'bar'] } },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents: {}, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('none');
     expect(bumpInfo.calculatedChangeTypes['unrelated']).toBeUndefined();
   });
 
   it('should bump all packages in a group together as none with dependents', () => {
+    const dependents = { foo: ['bar'] };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        foo: ['bar'],
-      },
       packageInfos: {
         foo: {},
         bar: {},
@@ -274,17 +267,17 @@ describe('updateRelatedChangeType', () => {
       packageGroups: { grp: { packageNames: ['foo', 'bar'] } },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('none');
     expect(bumpInfo.calculatedChangeTypes['unrelated']).toBeUndefined();
   });
 
   it('should bump all grouped packages, if a dependency was bumped', () => {
+    const dependents = {
+      dep: ['bar'],
+    };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        dep: ['bar'],
-      },
       packageInfos: makePackageInfos({
         foo: {},
         bar: { dependencies: { dep: '1.0.0' } },
@@ -300,7 +293,7 @@ describe('updateRelatedChangeType', () => {
       packageGroups: { grp: { packageNames: ['foo', 'bar'] } },
     });
 
-    updateRelatedChangeType('dep.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'dep.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBe('minor');
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('minor');
@@ -308,11 +301,11 @@ describe('updateRelatedChangeType', () => {
   });
 
   it('should bump dependent package, if a dependency was in a group', () => {
+    const dependents = {
+      dep: ['bar'],
+      foo: ['app'],
+    };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        dep: ['bar'],
-        foo: ['app'],
-      },
       packageInfos: makePackageInfos({
         foo: {},
         bar: { dependencies: { dep: '1.0.0' } },
@@ -331,7 +324,7 @@ describe('updateRelatedChangeType', () => {
       ],
     });
 
-    updateRelatedChangeType('dep.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'dep.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBe('minor');
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('minor');
@@ -339,14 +332,14 @@ describe('updateRelatedChangeType', () => {
   });
 
   it('should propagate dependent change type across group', () => {
+    const dependents = {
+      mergeStyles: ['styling'],
+      styling: ['bar'],
+      utils: ['bar'],
+      bar: ['datetime'],
+      datetimeUtils: ['datetime'],
+    };
     const bumpInfo = getBumpInfo({
-      dependents: {
-        mergeStyles: ['styling'],
-        styling: ['bar'],
-        utils: ['bar'],
-        bar: ['datetime'],
-        datetimeUtils: ['datetime'],
-      },
       packageInfos: makePackageInfos({
         styling: { dependencies: { mergeStyles: '1.0.0' } },
         utils: {},
@@ -369,8 +362,8 @@ describe('updateRelatedChangeType', () => {
       ],
     });
 
-    updateRelatedChangeType('mergeStyles.json', bumpInfo, true);
-    updateRelatedChangeType('datetimeUtils.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'mergeStyles.json', bumpInfo, dependents, bumpDeps: true });
+    updateRelatedChangeType({ changeFile: 'datetimeUtils.json', bumpInfo, dependents, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBe('minor');
     expect(bumpInfo.calculatedChangeTypes['bar']).toBe('minor');
@@ -390,7 +383,7 @@ describe('updateRelatedChangeType', () => {
       },
     });
 
-    updateRelatedChangeType('foo.json', bumpInfo, true);
+    updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents: {}, bumpDeps: true });
 
     expect(bumpInfo.calculatedChangeTypes['foo']).toBeUndefined();
   });

src/bump/bumpInPlace.ts

@@ -1,5 +1,5 @@
 import { BumpInfo } from '../types/BumpInfo';
-import { setDependentsInBumpInfo } from './setDependentsInBumpInfo';
+import { getDependentsForPackages } from './getDependentsForPackages';
 import { updateRelatedChangeType } from './updateRelatedChangeType';
 import { bumpPackageInfoVersion } from './bumpPackageInfoVersion';
 import { BeachballOptions } from '../types/BeachballOptions';
@@ -16,9 +16,7 @@ export function bumpInPlace(bumpInfo: BumpInfo, options: BeachballOptions): void
   const { packageInfos, scopedPackages, calculatedChangeTypes, changeFileChangeInfos, modifiedPackages } = bumpInfo;
 
   // pass 1: figure out all the change types for all the packages taking into account the bumpDeps option and version groups
-  if (bumpDeps) {
-    setDependentsInBumpInfo(bumpInfo);
-  }
+  const dependents = bumpDeps ? getDependentsForPackages(bumpInfo) : {};
 
   setGroupsInBumpInfo(bumpInfo, options);
 
@@ -41,7 +39,7 @@ export function bumpInPlace(bumpInfo: BumpInfo, options: BeachballOptions): void
 
   // Calculate change types for packages and dependencies
   for (const { changeFile } of changeFileChangeInfos) {
-    updateRelatedChangeType(changeFile, bumpInfo, bumpDeps);
+    updateRelatedChangeType({ changeFile, bumpInfo, dependents, bumpDeps });
   }
 
   // pass 3: actually bump the packages in the bumpInfo in memory (no disk writes at this point)

src/bump/gatherBumpInfo.ts

@@ -31,7 +31,6 @@ export function gatherBumpInfo(options: BeachballOptions, packageInfos: PackageI
     scopedPackages: new Set(getScopedPackages(options, packageInfos)),
     dependentChangedBy: {},
     groupOptions: {},
-    dependents: {},
   };
 
   bumpInPlace(bumpInfo, options);

src/bump/getDependentsForPackages.ts

@@ -0,0 +1,35 @@
+import type { BumpInfo, PackageDependents } from '../types/BumpInfo';
+import { consideredDependencies } from '../types/PackageInfo';
+
+/**
+ * Gets dependents for all packages (child points to parents): if A depends on B, then `{B: [A]}`
+ *
+ * Example: "BigApp" deps on "SomeUtil", "BigApp" would be the dependent.
+ * => `{ "SomeUtil": ["BigApp"] }`
+ */
+export function getDependentsForPackages(
+  bumpInfo: Pick<BumpInfo, 'packageInfos' | 'scopedPackages'>
+): PackageDependents {
+  const { packageInfos, scopedPackages } = bumpInfo;
+
+  const dependents: PackageDependents = {};
+
+  for (const [pkgName, info] of Object.entries(packageInfos)) {
+    if (!scopedPackages.has(pkgName)) {
+      continue;
+    }
+
+    for (const depType of consideredDependencies) {
+      for (const dep of Object.keys(info[depType] || {})) {
+        if (packageInfos[dep]) {
+          dependents[dep] ??= [];
+          if (!dependents[dep].includes(pkgName)) {
+            dependents[dep].push(pkgName);
+          }
+        }
+      }
+    }
+  }
+
+  return dependents;
+}

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 } from '../types/BeachballOptions';
-import { PackageInfos, PackageJson } from '../types/PackageInfo';
+import { PackageInfos, PackageJson, consideredDependencies } from '../types/PackageInfo';
 import { findProjectRoot } from 'workspace-tools';
 import { npm } from '../packageManager/npm';
 import { packageManager } from '../packageManager/packageManager';
@@ -23,17 +23,16 @@ export function writePackageJson(modifiedPackages: Set<string>, packageInfos: Pa
       packageJson.version = info.version;
     }
 
-    for (const depKind of ['dependencies', 'devDependencies', 'peerDependencies', 'optionalDependencies'] as const) {
+    for (const depKind of consideredDependencies) {
       // updatedDeps contains all of the dependencies in the bump info since the beginning of a build job
-      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!
-        for (const [dep, updatedVersion] of Object.entries(updatedDepsVersions)) {
-          if (modifiedPackages.has(dep) && packageJson[depKind]?.[dep]) {
-            packageJson[depKind]![dep] = updatedVersion;
-          }
+      const updatedDeps = info[depKind] || {};
+
+      // 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!
+      for (const [dep, updatedVersion] of Object.entries(updatedDeps)) {
+        if (modifiedPackages.has(dep) && packageJson[depKind]?.[dep]) {
+          packageJson[depKind]![dep] = updatedVersion;
         }
       }
     }

src/bump/setDependentVersions.ts

@@ -1,6 +1,6 @@
 import type { BeachballOptions } from '../types/BeachballOptions';
 import { BumpInfo } from '../types/BumpInfo';
-import type { PackageInfos } from '../types/PackageInfo';
+import { consideredDependencies, type PackageInfos } from '../types/PackageInfo';
 import { bumpMinSemverRange } from './bumpMinSemverRange';
 
 /**
@@ -21,10 +21,8 @@ export function setDependentVersions(
       continue; // out of scope
     }
 
-    for (const deps of [info.dependencies, info.devDependencies, info.peerDependencies, info.optionalDependencies]) {
-      if (!deps) {
-        continue; // package doesn't have this dep type
-      }
+    for (const depType of consideredDependencies) {
+      const deps = info[depType] || {};
 
       for (const [dep, existingVersionRange] of Object.entries(deps)) {
         const depPackage = packageInfos[dep];

src/bump/setDependentsInBumpInfo.ts

@@ -1,29 +0,0 @@
-import type { BumpInfo } from '../types/BumpInfo';
-
-/**
- * Set dependents for all packages. **This mutates `bumpInfo.dependents`.**
- *
- * Example: "BigApp" deps on "SomeUtil", "BigApp" would be the dependent
- */
-export function setDependentsInBumpInfo(
-  bumpInfo: Pick<BumpInfo, 'packageInfos' | 'scopedPackages' | 'dependents'>
-): void {
-  const { packageInfos, scopedPackages, dependents } = bumpInfo;
-
-  for (const [pkgName, info] of Object.entries(packageInfos)) {
-    if (!scopedPackages.has(pkgName)) {
-      continue;
-    }
-
-    for (const deps of [info.dependencies, info.devDependencies, info.peerDependencies, info.optionalDependencies]) {
-      for (const dep of Object.keys(deps || {})) {
-        if (packageInfos[dep]) {
-          dependents[dep] ??= [];
-          if (!dependents[dep].includes(pkgName)) {
-            dependents[dep].push(pkgName);
-          }
-        }
-      }
-    }
-  }
-}

src/bump/updateRelatedChangeType.ts

@@ -1,5 +1,5 @@
 import { getMaxChangeType, MinChangeType } from '../changefile/changeTypes';
-import { BumpInfo } from '../types/BumpInfo';
+import { BumpInfo, PackageDependents } from '../types/BumpInfo';
 
 /**
  * This is the core of the bumpInfo dependency bumping logic - done once per change file
@@ -18,14 +18,20 @@ import { BumpInfo } from '../types/BumpInfo';
  *
  * What it does not do:
  * - bumpInfo.calculatedChangeTypes: will not mutate the entryPoint `pkgName` change type
- *
- * Inputs from bumpInfo are listed in the [^1] below in the function body
  */
-export function updateRelatedChangeType(changeFile: string, bumpInfo: BumpInfo, bumpDeps: boolean): void {
-  /** [^1]: all the information needed from `bumpInfo` */
-  const { calculatedChangeTypes, packageGroups, dependents, packageInfos, groupOptions } = bumpInfo;
+export function updateRelatedChangeType(params: {
+  changeFile: string;
+  bumpInfo: Pick<
+    BumpInfo,
+    'calculatedChangeTypes' | 'changeFileChangeInfos' | 'packageGroups' | 'packageInfos' | 'groupOptions'
+  >;
+  dependents: PackageDependents;
+  bumpDeps: boolean;
+}): void {
+  const { changeFile, bumpInfo, dependents, bumpDeps } = params;
+  const { calculatedChangeTypes, changeFileChangeInfos, packageGroups, packageInfos, groupOptions } = bumpInfo;
 
-  for (const info of bumpInfo.changeFileChangeInfos) {
+  for (const info of changeFileChangeInfos) {
     if (info.changeFile !== changeFile) {
       continue;
     }

src/publish/performPublishOverrides.ts

@@ -1,6 +1,6 @@
 import * as fs from 'fs-extra';
 import { getWorkspaceRange } from '../packageManager/getWorkspaceRange';
-import type { PackageInfos, PackageJson, PublishConfig } from '../types/PackageInfo';
+import { consideredDependencies, type PackageInfos, type PackageJson, type PublishConfig } from '../types/PackageInfo';
 
 const acceptedKeys: (keyof PublishConfig)[] = [
   'types',
@@ -46,8 +46,8 @@ function performPublishConfigOverrides(packageJson: PackageJson): void {
  * replacement.
  */
 function performWorkspaceVersionOverrides(packageJson: PackageJson, packageInfos: PackageInfos): void {
-  const { dependencies, devDependencies, peerDependencies, optionalDependencies } = packageJson;
-  for (const deps of [dependencies, devDependencies, peerDependencies, optionalDependencies]) {
+  for (const depType of consideredDependencies) {
+    const deps = packageJson[depType];
     if (!deps) continue;
 
     for (const [depName, depVersion] of Object.entries(deps)) {

src/types/BumpInfo.ts

@@ -21,9 +21,6 @@ export type BumpInfo = {
   /** Package group options */
   groupOptions: { [grp: string]: VersionGroupOptions };
 
-  /** Dependents cache (if A depends on B, then {B: [A]}) - child points to parents */
-  dependents: { [pkgName: string]: string[] };
-
   /** Set of packages that had been modified */
   modifiedPackages: Set<string>;
 
@@ -36,3 +33,6 @@ export type BumpInfo = {
   /** Set of packages that are in scope for this bump */
   scopedPackages: Set<string>;
 };
+
+/** Dependents cache (child points to parents): if A depends on B, then `{ B: [A] }` */
+export type PackageDependents = { [pkgName: string]: string[] };

src/types/PackageInfo.ts

@@ -64,3 +64,11 @@ export interface PackageGroupsInfo {
 }
 
 export type PackageGroups = { [groupName: string]: PackageGroupsInfo };
+
+/** Types of dependencies to consider when bumping. */
+export const consideredDependencies = [
+  'dependencies',
+  'devDependencies',
+  'peerDependencies',
+  'optionalDependencies',
+] as const;