diff --git a/change/beachball-b14f577b-da9c-4db3-9cd8-f0d6fba25af8.json b/change/beachball-b14f577b-da9c-4db3-9cd8-f0d6fba25af8.json new file mode 100644 index 000000000..7a34e3947 --- /dev/null +++ b/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" +} diff --git a/src/__tests__/bump/updateRelatedChangeType.test.ts b/src/__tests__/bump/updateRelatedChangeType.test.ts index c39115f28..c0a59686d 100644 --- a/src/__tests__/bump/updateRelatedChangeType.test.ts +++ b/src/__tests__/bump/updateRelatedChangeType.test.ts @@ -19,7 +19,6 @@ describe('updateRelatedChangeType', () => { _.merge( { 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(); }); diff --git a/src/bump/bumpInPlace.ts b/src/bump/bumpInPlace.ts index 1b7846537..bbea2e3e5 100644 --- a/src/bump/bumpInPlace.ts +++ b/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) diff --git a/src/bump/gatherBumpInfo.ts b/src/bump/gatherBumpInfo.ts index 81128456b..786b10cff 100644 --- a/src/bump/gatherBumpInfo.ts +++ b/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); diff --git a/src/bump/getDependentsForPackages.ts b/src/bump/getDependentsForPackages.ts new file mode 100644 index 000000000..f5ab3955e --- /dev/null +++ b/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 +): 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; +} diff --git a/src/bump/performBump.ts b/src/bump/performBump.ts index a9bc8dec4..cf093fc2f 100644 --- a/src/bump/performBump.ts +++ b/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, 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; } } } diff --git a/src/bump/setDependentVersions.ts b/src/bump/setDependentVersions.ts index bebf00328..9c51f75c4 100644 --- a/src/bump/setDependentVersions.ts +++ b/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]; diff --git a/src/bump/setDependentsInBumpInfo.ts b/src/bump/setDependentsInBumpInfo.ts deleted file mode 100644 index 34bcab2cc..000000000 --- a/src/bump/setDependentsInBumpInfo.ts +++ /dev/null @@ -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 -): 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); - } - } - } - } - } -} diff --git a/src/bump/updateRelatedChangeType.ts b/src/bump/updateRelatedChangeType.ts index 685811fcb..cb4288d96 100644 --- a/src/bump/updateRelatedChangeType.ts +++ b/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; - - for (const info of bumpInfo.changeFileChangeInfos) { +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 changeFileChangeInfos) { if (info.changeFile !== changeFile) { continue; } diff --git a/src/publish/performPublishOverrides.ts b/src/publish/performPublishOverrides.ts index a88edca7e..a3a7d82c3 100644 --- a/src/publish/performPublishOverrides.ts +++ b/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)) { diff --git a/src/types/BumpInfo.ts b/src/types/BumpInfo.ts index 240380976..39437b6fe 100644 --- a/src/types/BumpInfo.ts +++ b/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; @@ -36,3 +33,6 @@ export type BumpInfo = { /** Set of packages that are in scope for this bump */ scopedPackages: Set; }; + +/** Dependents cache (child points to parents): if A depends on B, then `{ B: [A] }` */ +export type PackageDependents = { [pkgName: string]: string[] }; diff --git a/src/types/PackageInfo.ts b/src/types/PackageInfo.ts index cb0ee5ebf..815a18cf8 100644 --- a/src/types/PackageInfo.ts +++ b/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;