Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
Sep 10, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c0a150
Remove dependents from BumpInfo
ecraig12345 Sep 10, 2024
1e82c2a
shared dep types
ecraig12345 Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/beachball-b14f577b-da9c-4db3-9cd8-f0d6fba25af8.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Use a shared list of considered dependency types",
"packageName": "beachball",
"email": "[email protected]",
"dependentChangeType": "patch"
}
103 changes: 48 additions & 55 deletions src/__tests__/bump/updateRelatedChangeType.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ describe('updateRelatedChangeType', () => {
_.merge<BumpInfo, PartialBumpInfo>(
{
changeFileChangeInfos: [],
dependents: {},
calculatedChangeTypes: {},
packageInfos: makePackageInfos({
foo: { combinedOptions: { disallowedChangeTypes: [], defaultNpmTag: 'latest' } },
Expand All @@ -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' } },
],
Expand All @@ -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' } },
],
Expand All @@ -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',
Expand All @@ -115,21 +110,21 @@ 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');
expect(bumpInfo.calculatedChangeTypes['app']).toBe('minor');
});

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',
Expand All @@ -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' } },
Expand All @@ -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');
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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: {},
Expand All @@ -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' } },
Expand All @@ -300,19 +293,19 @@ 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');
expect(bumpInfo.calculatedChangeTypes['unrelated']).toBeUndefined();
});

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' } },
Expand All @@ -331,22 +324,22 @@ 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');
expect(bumpInfo.calculatedChangeTypes['app']).toBe('minor');
});

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: {},
Expand All @@ -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');
Expand All @@ -390,7 +383,7 @@ describe('updateRelatedChangeType', () => {
},
});

updateRelatedChangeType('foo.json', bumpInfo, true);
updateRelatedChangeType({ changeFile: 'foo.json', bumpInfo, dependents: {}, bumpDeps: true });

expect(bumpInfo.calculatedChangeTypes['foo']).toBeUndefined();
});
Expand Down
8 changes: 3 additions & 5 deletions src/bump/bumpInPlace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);

Expand All @@ -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)
Expand Down
1 change: 0 additions & 1 deletion src/bump/gatherBumpInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export function gatherBumpInfo(options: BeachballOptions, packageInfos: PackageI
scopedPackages: new Set(getScopedPackages(options, packageInfos)),
dependentChangedBy: {},
groupOptions: {},
dependents: {},
};

bumpInPlace(bumpInfo, options);
Expand Down
35 changes: 35 additions & 0 deletions src/bump/getDependentsForPackages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
}
21 changes: 10 additions & 11 deletions src/bump/performBump.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
}
}
}
Expand Down
Loading