-
Notifications
You must be signed in to change notification settings - Fork 91
Open
Labels
Description
Currently there's some inconsistency with how groups' disallowedChangeTypes are handled, specifically for dependent bumps. This test (which would go in bump.test.ts) shows the issue:
it('should bump all grouped packages, if a dependency was bumped, following group disallow list', async () => {
repositoryFactory = new RepositoryFactory({
folders: {
packages: {
foo: { version: '1.0.0' },
bar: { version: '1.0.0', dependencies: { foo: '1.0.0' } },
baz: { version: '1.0.0', beachball: { disallowedChangeTypes: [] } },
},
},
});
repo = repositoryFactory.cloneRepository();
const { originalPackageInfos, options, parsedOptions } = getOptionsAndPackages({
bumpDeps: true,
groups: [{ include: ['packages/bar', 'packages/baz'], name: 'grp', disallowedChangeTypes: ['major'] }],
});
generateChangeFiles([{ packageName: 'foo', type: 'major', dependentChangeType: 'major' }], options);
repo.push();
await bump(options, originalPackageInfos);
const packageInfos = getPackageInfos(parsedOptions);
expect(packageInfos['foo'].version).toBe('2.0.0');
// Expected: as below. Current: they're major bumped (ignoring group disallowedChangeTypes)
expect(packageInfos['bar'].version).toBe('1.1.0');
expect(packageInfos['baz'].version).toBe('1.1.0');
});Most immediately, this is because the logic in updateRelatedChangeType doesn't consider disallowedChangeTypes from groups.
A better solution would be to merge group disallowedChangeTypes into PackageInfo.combinedOptions in getPackageInfos (and review all related logic to see if any other updates are needed). This would require calculating the package infos and groups together instead of having separate helpers.
I marked this as a breaking change since it has the potential to cause subtle logic changes.