Skip to content

Commit f0fbced

Browse files
ecraig12345ecraig12345
committed
sync: fix precedence of tag options
1 parent 4c7ba47 commit f0fbced

13 files changed

Lines changed: 112 additions & 94 deletions

‎change/beachball-321fb1a2-d8d9-431d-94e1-fa0b7408664f.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "sync: fix precedence of tag options",
4+
"packageName": "beachball",
5+
"email": "elcraig@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

‎src/__fixtures__/packageInfos.ts‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,40 @@
1-
import type { BeachballOptions } from '../types/BeachballOptions';
21
import type { PackageInfo, PackageInfos } from '../types/PackageInfo';
3-
import { getDefaultOptions } from '../options/getDefaultOptions';
4-
5-
const defaultOptions = getDefaultOptions();
2+
import { getPackageInfosWithOptions, type TestPackageInfosOptions } from '../options/getPackageInfosWithOptions';
63

74
export type PartialPackageInfos = {
8-
[name: string]: Partial<Omit<PackageInfo, 'combinedOptions'>> & { combinedOptions?: Partial<BeachballOptions> };
5+
[name: string]: Omit<Partial<PackageInfo>, 'combinedOptions' | 'packageOptions'> & {
6+
beachball?: PackageInfo['packageOptions'];
7+
};
98
};
109

1110
/**
1211
* Makes a properly typed PackageInfos object from a partial object, filling in defaults:
13-
* ```
12+
* ```js
1413
* {
1514
* name: '<key>',
1615
* version: '1.0.0',
1716
* private: false,
18-
* combinedOptions: {},
19-
* packageOptions: {},
2017
* packageJsonPath: ''
2118
* }
2219
* ```
20+
* Other defaults and values are filled by the actual logic in `getPackageInfosWithOptions`,
21+
* including the overrides in `testOptions` merged in realistic order.
2322
*/
24-
export function makePackageInfos(packageInfos: PartialPackageInfos): PackageInfos {
25-
const acc: PackageInfos = {};
26-
for (const [name, info] of Object.entries(packageInfos)) {
27-
const { combinedOptions, ...rest } = info;
28-
acc[name] = {
29-
name,
30-
version: '1.0.0',
31-
private: false,
32-
combinedOptions: { ...defaultOptions, ...combinedOptions },
33-
packageOptions: {},
34-
packageJsonPath: '',
35-
...rest,
36-
};
37-
}
38-
return acc;
23+
export function makePackageInfos(
24+
packageInfos: PartialPackageInfos,
25+
testOptions?: TestPackageInfosOptions
26+
): PackageInfos {
27+
return getPackageInfosWithOptions(
28+
Object.entries(packageInfos).map(([name, info]) => {
29+
return {
30+
name,
31+
version: '1.0.0',
32+
private: false,
33+
packageOptions: {},
34+
packageJsonPath: '',
35+
...info,
36+
};
37+
}),
38+
testOptions
39+
);
3940
}

‎src/__functional__/monorepo/getPackageInfos.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { PackageInfos, PackageInfo } from '../../types/PackageInfo';
88
import { getDefaultOptions } from '../../options/getDefaultOptions';
99
import { initMockLogs } from '../../__fixtures__/mockLogs';
1010
import type { PackageOptions } from '../../types/BeachballOptions';
11+
import { _cloneObject } from '../../publish/cloneBumpInfo';
1112

1213
const defaultOptions = getDefaultOptions();
1314

@@ -21,8 +22,7 @@ function cleanPath(root: string, filePath: string) {
2122
function cleanPackageInfos(root: string, packageInfos: PackageInfos) {
2223
const cleanedInfos: PackageInfos = {};
2324
for (const [pkgName, originalInfo] of Object.entries(packageInfos)) {
24-
// Make a copy deep enough to cover anything that will be modified
25-
const pkgInfo = (cleanedInfos[pkgName] = { ...originalInfo, combinedOptions: { ...originalInfo.combinedOptions } });
25+
const pkgInfo = (cleanedInfos[pkgName] = _cloneObject(originalInfo));
2626

2727
// Remove absolute paths
2828
pkgInfo.packageJsonPath = cleanPath(root, pkgInfo.packageJsonPath);

‎src/__tests__/bump/updateRelatedChangeType.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ describe('updateRelatedChangeType', () => {
227227
bar: {},
228228
foo: {
229229
dependencies: { bar: '1.0.0' },
230-
combinedOptions: { disallowedChangeTypes: ['minor', 'major'] },
230+
beachball: { disallowedChangeTypes: ['minor', 'major'] },
231231
},
232232
},
233233
});

‎src/__tests__/changefile/getDisallowedChangeTypes.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ describe('getDisallowedChangeTypes', () => {
1414

1515
it('returns disallowedChangeTypes for package', () => {
1616
const packageInfos = makePackageInfos({
17-
foo: { combinedOptions: { disallowedChangeTypes: ['major', 'minor'] } },
17+
foo: { beachball: { disallowedChangeTypes: ['major', 'minor'] } },
1818
});
1919
expect(getDisallowedChangeTypes('foo', packageInfos, {})).toEqual(['major', 'minor']);
2020
});
@@ -29,7 +29,7 @@ describe('getDisallowedChangeTypes', () => {
2929

3030
it('returns disallowedChangeTypes for package if not in a group', () => {
3131
const packageInfos = makePackageInfos({
32-
foo: { combinedOptions: { disallowedChangeTypes: ['patch'] } },
32+
foo: { beachball: { disallowedChangeTypes: ['patch'] } },
3333
});
3434
const packageGroups: PackageGroups = {
3535
group: { packageNames: ['bar'], disallowedChangeTypes: ['major', 'minor'] },
@@ -39,7 +39,7 @@ describe('getDisallowedChangeTypes', () => {
3939

4040
it('prefers disallowedChangeTypes for group over package', () => {
4141
const packageInfos = makePackageInfos({
42-
foo: { combinedOptions: { disallowedChangeTypes: ['patch'] } },
42+
foo: { beachball: { disallowedChangeTypes: ['patch'] } },
4343
});
4444
const packageGroups: PackageGroups = {
4545
group: { packageNames: ['foo'], disallowedChangeTypes: ['major', 'minor'] },

‎src/__tests__/changefile/getQuestionsForPackage.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ describe('getQuestionsForPackage', () => {
5353
it('errors if options.type is disallowed', () => {
5454
const questions = getQuestionsForPackage({
5555
...defaultQuestionsParams,
56-
packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { disallowedChangeTypes: ['major'] } } }),
56+
packageInfos: makePackageInfos({ [pkg]: { beachball: { disallowedChangeTypes: ['major'] } } }),
5757
options: { type: 'major', message: '' },
5858
});
5959
expect(questions).toBeUndefined();
@@ -64,7 +64,7 @@ describe('getQuestionsForPackage', () => {
6464
const questions = getQuestionsForPackage({
6565
...defaultQuestionsParams,
6666
packageInfos: makePackageInfos({
67-
[pkg]: { combinedOptions: { disallowedChangeTypes: ['major', 'minor', 'patch', 'none'] } },
67+
[pkg]: { beachball: { disallowedChangeTypes: ['major', 'minor', 'patch', 'none'] } },
6868
}),
6969
});
7070
expect(questions).toBeUndefined();
@@ -74,7 +74,7 @@ describe('getQuestionsForPackage', () => {
7474
it('respects disallowedChangeTypes', () => {
7575
const questions = getQuestionsForPackage({
7676
...defaultQuestionsParams,
77-
packageInfos: makePackageInfos({ [pkg]: { combinedOptions: { disallowedChangeTypes: ['major'] } } }),
77+
packageInfos: makePackageInfos({ [pkg]: { beachball: { disallowedChangeTypes: ['major'] } } }),
7878
});
7979
const choices = (questions![0].choices as prompts.Choice[]).map(c => c.value as ChangeType);
8080
expect(choices).toEqual(['patch', 'minor', 'none']);
@@ -94,7 +94,7 @@ describe('getQuestionsForPackage', () => {
9494
const questions = getQuestionsForPackage({
9595
...defaultQuestionsParams,
9696
packageInfos: makePackageInfos({
97-
[pkg]: { version: '1.0.0-beta.1', combinedOptions: { disallowedChangeTypes: ['prerelease'] } },
97+
[pkg]: { version: '1.0.0-beta.1', beachball: { disallowedChangeTypes: ['prerelease'] } },
9898
}),
9999
});
100100
const choices = (questions![0].choices as prompts.Choice[]).map(c => c.value as ChangeType);
@@ -114,7 +114,7 @@ describe('getQuestionsForPackage', () => {
114114
const questions = getQuestionsForPackage({
115115
...defaultQuestionsParams,
116116
packageInfos: makePackageInfos({
117-
[pkg]: { combinedOptions: { disallowedChangeTypes: ['major', 'minor', 'none'] } },
117+
[pkg]: { beachball: { disallowedChangeTypes: ['major', 'minor', 'none'] } },
118118
}),
119119
});
120120
expect(questions).toHaveLength(1);
@@ -127,7 +127,7 @@ describe('getQuestionsForPackage', () => {
127127
packageInfos: makePackageInfos({
128128
[pkg]: {
129129
version: '1.0.0-beta.1',
130-
combinedOptions: { disallowedChangeTypes: ['major', 'minor', 'patch', 'none'] },
130+
beachball: { disallowedChangeTypes: ['major', 'minor', 'patch', 'none'] },
131131
},
132132
}),
133133
});

‎src/__tests__/changefile/promptForChange.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ describe('promptForChange', () => {
102102
...defaultParams(),
103103
packageInfos: makePackageInfos({
104104
foo: {},
105-
bar: { combinedOptions: { disallowedChangeTypes: ['major', 'minor', 'patch', 'none'] } },
105+
bar: { beachball: { disallowedChangeTypes: ['major', 'minor', 'patch', 'none'] } },
106106
}),
107107
});
108108

‎src/__tests__/packageManager/listPackageVersions.test.ts‎

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -85,18 +85,23 @@ describe('list npm versions', () => {
8585

8686
describe('listPackageVersionsByTag', () => {
8787
it('succeeds with nothing to do', async () => {
88-
expect(await listPackageVersionsByTag([], undefined, npmOptions)).toEqual({});
89-
expect(await listPackageVersionsByTag([], 'beta', npmOptions)).toEqual({});
88+
expect(await listPackageVersionsByTag([], npmOptions)).toEqual({});
9089
expect(npmMock.mock).not.toHaveBeenCalled();
9190
});
9291

9392
it('returns requested tag for one package', async () => {
9493
npmMock.setRegistryData({ foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } } });
95-
const packageInfos = Object.values(
96-
makePackageInfos({ foo: { combinedOptions: { tag: 'latest', defaultNpmTag: 'latest' } } })
94+
const packageInfos = makePackageInfos(
95+
{ foo: {} },
96+
{
97+
// The merging order is actually handled by getPackageInfosWithOptions, but having a test
98+
// like this is good documentation.
99+
repoOptions: { tag: 'latest', defaultNpmTag: 'latest' },
100+
argv: ['--tag', 'beta'],
101+
}
97102
);
98103

99-
const versions = await listPackageVersionsByTag(packageInfos, 'beta', npmOptions);
104+
const versions = await listPackageVersionsByTag(Object.values(packageInfos), npmOptions);
100105
expect(versions).toEqual({ foo: '2.0.0-beta' });
101106
expect(npmMock.mock).toHaveBeenCalledTimes(1);
102107
expect(npmMock.mock).toHaveBeenCalledWith([...commonArgs, 'foo', ...npmShowProperties], expect.anything());
@@ -107,14 +112,15 @@ describe('list npm versions', () => {
107112
foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } },
108113
bar: { 'dist-tags': { latest: '1.0.0', beta: '3.0.0-beta' } },
109114
});
110-
const packageInfos = Object.values(
111-
makePackageInfos({
112-
foo: { combinedOptions: { tag: 'latest' } },
113-
bar: { combinedOptions: { tag: 'latest' } },
114-
})
115+
const packageInfos = makePackageInfos(
116+
{ foo: {}, bar: {} },
117+
{
118+
repoOptions: { tag: 'latest' },
119+
argv: ['--tag', 'beta'],
120+
}
115121
);
116122

117-
const versions = await listPackageVersionsByTag(packageInfos, 'beta', npmOptions);
123+
const versions = await listPackageVersionsByTag(Object.values(packageInfos), npmOptions);
118124
expect(versions).toEqual({ foo: '2.0.0-beta', bar: '3.0.0-beta' });
119125
expect(npmMock.mock).toHaveBeenCalledTimes(2);
120126
});
@@ -123,30 +129,21 @@ describe('list npm versions', () => {
123129
const packages = 'abcdefghij'.split('');
124130
const showData = Object.fromEntries(packages.map((x, i) => [x, { 'dist-tags': { latest: `${i}.0.0` } }]));
125131
npmMock.setRegistryData(showData);
126-
const packageInfos = Object.values(makePackageInfos(Object.fromEntries(packages.map(x => [x, {}]))));
132+
const packageInfos = makePackageInfos(Object.fromEntries(packages.map(x => [x, {}])), {
133+
repoOptions: { tag: 'latest' },
134+
});
127135

128-
expect(await listPackageVersionsByTag(packageInfos, 'latest', npmOptions)).toEqual(
136+
expect(await listPackageVersionsByTag(Object.values(packageInfos), npmOptions)).toEqual(
129137
Object.fromEntries(Object.entries(showData).map(([k, v]) => [k, v['dist-tags'].latest]))
130138
);
131139
expect(npmMock.mock).toHaveBeenCalledTimes(packages.length);
132140
});
133141

134-
it('falls back to combinedOptions.tag', async () => {
135-
npmMock.setRegistryData({ foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } } });
136-
const packageInfos = Object.values(
137-
makePackageInfos({ foo: { combinedOptions: { tag: 'beta', defaultNpmTag: 'latest' } } })
138-
);
139-
140-
const versions = await listPackageVersionsByTag(packageInfos, undefined, npmOptions);
141-
expect(versions).toEqual({ foo: '2.0.0-beta' });
142-
expect(npmMock.mock).toHaveBeenCalledTimes(1);
143-
});
144-
145-
it('falls back to combinedOptions.defaultNpmTag if combinedOptions.tag is unset', async () => {
142+
it('falls back to defaultNpmTag if tag is unset', async () => {
146143
npmMock.setRegistryData({ foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } } });
147-
const packageInfos = Object.values(makePackageInfos({ foo: { combinedOptions: { defaultNpmTag: 'beta' } } }));
144+
const packageInfos = Object.values(makePackageInfos({ foo: { beachball: { defaultNpmTag: 'beta' } } }));
148145

149-
const versions = await listPackageVersionsByTag(packageInfos, undefined, npmOptions);
146+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
150147
expect(versions).toEqual({ foo: '2.0.0-beta' });
151148
expect(npmMock.mock).toHaveBeenCalledTimes(1);
152149
});
@@ -155,33 +152,36 @@ describe('list npm versions', () => {
155152
npmMock.setRegistryData({});
156153
const packageInfos = Object.values(makePackageInfos({ foo: {} }));
157154

158-
const versions = await listPackageVersionsByTag(packageInfos, 'latest', npmOptions);
155+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
159156
expect(versions).toEqual({});
160157
expect(npmMock.mock).toHaveBeenCalledTimes(1);
161158
});
162159

163160
it('returns empty if no matching dist-tags available', async () => {
164161
npmMock.setRegistryData({ foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } } });
165-
const packageInfos = Object.values(makePackageInfos({ foo: {} }));
162+
const packageInfos = Object.values(makePackageInfos({ foo: {} }, { repoOptions: { tag: 'missing' } }));
166163

167-
const versions = await listPackageVersionsByTag(packageInfos, 'missing', npmOptions);
164+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
168165
expect(versions).toEqual({});
169166
expect(npmMock.mock).toHaveBeenCalledTimes(1);
170167
});
171168

172-
it('falls back to different tag option for different packages', async () => {
169+
it('uses per-package tag option', async () => {
173170
npmMock.setRegistryData({
174171
foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } },
175172
bar: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } },
176173
});
177174
const packageInfos = Object.values(
178-
makePackageInfos({
179-
foo: { combinedOptions: { defaultNpmTag: 'latest' } },
180-
bar: { combinedOptions: { tag: 'beta', defaultNpmTag: 'latest' } },
181-
})
175+
makePackageInfos(
176+
{
177+
foo: {},
178+
bar: { beachball: { tag: 'beta' } },
179+
},
180+
{ repoOptions: { defaultNpmTag: 'latest' } }
181+
)
182182
);
183183

184-
const versions = await listPackageVersionsByTag(packageInfos, undefined, npmOptions);
184+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
185185
expect(versions).toEqual({ foo: '1.0.0', bar: '2.0.0-beta' });
186186
expect(npmMock.mock).toHaveBeenCalledTimes(2);
187187
});
@@ -190,7 +190,7 @@ describe('list npm versions', () => {
190190
npmMock.setRegistryData({ foo: { 'dist-tags': { latest: '1.0.0' } } });
191191
const packageInfos = Object.values(makePackageInfos({ foo: {}, bar: {} }));
192192

193-
const versions = await listPackageVersionsByTag(packageInfos, 'latest', npmOptions);
193+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
194194
expect(versions).toEqual({ foo: '1.0.0' });
195195
expect(npmMock.mock).toHaveBeenCalledTimes(2);
196196
});

‎src/__tests__/packageManager/npmArgs.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ describe('getNpmPublishArgs', () => {
2929

3030
const packageInfos = makePackageInfos({
3131
basic: {},
32-
tag: { combinedOptions: { tag: 'testTag', defaultNpmTag: 'testDefaultTag' } },
33-
defaultTag: { combinedOptions: { defaultNpmTag: 'testDefaultTag' } },
32+
tag: { beachball: { tag: 'testTag', defaultNpmTag: 'testDefaultTag' } },
33+
defaultTag: { beachball: { defaultNpmTag: 'testDefaultTag' } },
3434
'@scoped/foo': {},
3535
});
3636

‎src/__tests__/publish/tagPackages.test.ts‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,19 @@ const createTagParameters = (tag: string, cwd: string) => {
1515

1616
type TagBumpInfo = Parameters<typeof tagPackages>[0];
1717

18-
/** foo and bar disable gitTags */
18+
/** repo options disable gitTags */
1919
const noTagBumpInfo: TagBumpInfo = {
2020
calculatedChangeTypes: {
2121
foo: 'minor',
2222
bar: 'major',
2323
},
24-
packageInfos: makePackageInfos({
25-
foo: {
26-
version: '1.0.0',
27-
combinedOptions: { gitTags: false },
24+
packageInfos: makePackageInfos(
25+
{
26+
foo: { version: '1.0.0' },
27+
bar: { version: '1.0.1' },
2828
},
29-
bar: {
30-
version: '1.0.1',
31-
combinedOptions: { gitTags: false },
32-
},
33-
}),
29+
{ repoOptions: { gitTags: false } }
30+
),
3431
modifiedPackages: new Set(['foo', 'bar']),
3532
newPackages: [],
3633
};
@@ -41,11 +38,11 @@ const oneTagBumpInfo: TagBumpInfo = {
4138
packageInfos: makePackageInfos({
4239
foo: {
4340
version: '1.0.0',
44-
combinedOptions: { gitTags: true },
41+
beachball: { gitTags: true },
4542
},
4643
bar: {
4744
version: '1.0.1',
48-
combinedOptions: { gitTags: false },
45+
beachball: { gitTags: false },
4946
},
5047
}),
5148
};

0 commit comments

Comments
 (0)
⚡