Skip to content

Commit 3d9fd4a

Browse files
ecraig12345ecraig12345
authored
sync: fix precedence of tag options (#557)
1 parent bb9b4e8 commit 3d9fd4a

5 files changed

Lines changed: 50 additions & 32 deletions

File tree

‎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: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { PackageInfo, PackageInfos } from '../types/PackageInfo';
22
import { getPackageInfosWithOptions } from '../options/getPackageInfosWithOptions';
3-
import type { RepoOptions } from '../types/BeachballOptions';
3+
import type { CliOptions, RepoOptions } from '../types/BeachballOptions';
44
import { defaultRemoteBranchName } from './gitDefaults';
55

66
export type PartialPackageInfos = {
@@ -22,7 +22,11 @@ export type PartialPackageInfos = {
2222
* Other defaults and values are filled by the actual logic in `getPackageInfosWithOptions`,
2323
* including the overrides in `repoOptions` merged in realistic order.
2424
*/
25-
export function makePackageInfos(packageInfos: PartialPackageInfos, repoOptions?: Partial<RepoOptions>): PackageInfos {
25+
export function makePackageInfos(
26+
packageInfos: PartialPackageInfos,
27+
repoOptions?: Partial<RepoOptions>,
28+
cliOptions?: Partial<CliOptions>
29+
): PackageInfos {
2630
return getPackageInfosWithOptions(
2731
Object.entries(packageInfos).map(([name, info]) => {
2832
return {
@@ -36,7 +40,7 @@ export function makePackageInfos(packageInfos: PartialPackageInfos, repoOptions?
3640
}),
3741
{
3842
repoOptions: { branch: defaultRemoteBranchName, ...repoOptions },
39-
cliOptions: { path: '', command: '' },
43+
cliOptions: { path: '', command: '', ...cliOptions },
4044
}
4145
);
4246
}

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

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -85,18 +85,21 @@ 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: { beachball: { tag: 'latest', defaultNpmTag: 'latest' } } })
94+
const packageInfos = makePackageInfos(
95+
{ foo: {} },
96+
// The merging order is actually handled by getPackageInfosWithOptions, but having a test
97+
// like this is good documentation.
98+
{ tag: 'latest', defaultNpmTag: 'latest' },
99+
{ tag: 'beta' } // CLI args should take precedence
97100
);
98101

99-
const versions = await listPackageVersionsByTag(packageInfos, 'beta', npmOptions);
102+
const versions = await listPackageVersionsByTag(Object.values(packageInfos), npmOptions);
100103
expect(versions).toEqual({ foo: '2.0.0-beta' });
101104
expect(npmMock.mock).toHaveBeenCalledTimes(1);
102105
expect(npmMock.mock).toHaveBeenCalledWith([...commonArgs, 'foo', ...npmShowProperties], expect.anything());
@@ -107,14 +110,13 @@ describe('list npm versions', () => {
107110
foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } },
108111
bar: { 'dist-tags': { latest: '1.0.0', beta: '3.0.0-beta' } },
109112
});
110-
const packageInfos = Object.values(
111-
makePackageInfos({
112-
foo: { beachball: { tag: 'latest' } },
113-
bar: { beachball: { tag: 'latest' } },
114-
})
113+
const packageInfos = makePackageInfos(
114+
{ foo: {}, bar: {} },
115+
{ tag: 'latest' }, // repo
116+
{ tag: 'beta' } // cli
115117
);
116118

117-
const versions = await listPackageVersionsByTag(packageInfos, 'beta', npmOptions);
119+
const versions = await listPackageVersionsByTag(Object.values(packageInfos), npmOptions);
118120
expect(versions).toEqual({ foo: '2.0.0-beta', bar: '3.0.0-beta' });
119121
expect(npmMock.mock).toHaveBeenCalledTimes(2);
120122
});
@@ -123,9 +125,9 @@ describe('list npm versions', () => {
123125
const packages = 'abcdefghij'.split('');
124126
const showData = Object.fromEntries(packages.map((x, i) => [x, { 'dist-tags': { latest: `${i}.0.0` } }]));
125127
npmMock.setRegistryData(showData);
126-
const packageInfos = Object.values(makePackageInfos(Object.fromEntries(packages.map(x => [x, {}]))));
128+
const packageInfos = makePackageInfos(Object.fromEntries(packages.map(x => [x, {}])), { tag: 'latest' });
127129

128-
expect(await listPackageVersionsByTag(packageInfos, 'latest', npmOptions)).toEqual(
130+
expect(await listPackageVersionsByTag(Object.values(packageInfos), npmOptions)).toEqual(
129131
Object.fromEntries(Object.entries(showData).map(([k, v]) => [k, v['dist-tags'].latest]))
130132
);
131133
expect(npmMock.mock).toHaveBeenCalledTimes(packages.length);
@@ -137,16 +139,16 @@ describe('list npm versions', () => {
137139
makePackageInfos({ foo: { beachball: { tag: 'beta', defaultNpmTag: 'latest' } } })
138140
);
139141

140-
const versions = await listPackageVersionsByTag(packageInfos, undefined, npmOptions);
142+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
141143
expect(versions).toEqual({ foo: '2.0.0-beta' });
142144
expect(npmMock.mock).toHaveBeenCalledTimes(1);
143145
});
144146

145-
it('falls back to beachball.defaultNpmTag if beachball.tag is unset', async () => {
147+
it('falls back to defaultNpmTag if tag is unset', async () => {
146148
npmMock.setRegistryData({ foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } } });
147149
const packageInfos = Object.values(makePackageInfos({ foo: { beachball: { defaultNpmTag: 'beta' } } }));
148150

149-
const versions = await listPackageVersionsByTag(packageInfos, undefined, npmOptions);
151+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
150152
expect(versions).toEqual({ foo: '2.0.0-beta' });
151153
expect(npmMock.mock).toHaveBeenCalledTimes(1);
152154
});
@@ -155,33 +157,36 @@ describe('list npm versions', () => {
155157
npmMock.setRegistryData({});
156158
const packageInfos = Object.values(makePackageInfos({ foo: {} }));
157159

158-
const versions = await listPackageVersionsByTag(packageInfos, 'latest', npmOptions);
160+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
159161
expect(versions).toEqual({});
160162
expect(npmMock.mock).toHaveBeenCalledTimes(1);
161163
});
162164

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

167-
const versions = await listPackageVersionsByTag(packageInfos, 'missing', npmOptions);
169+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
168170
expect(versions).toEqual({});
169171
expect(npmMock.mock).toHaveBeenCalledTimes(1);
170172
});
171173

172-
it('falls back to different tag option for different packages', async () => {
174+
it('uses per-package tag option', async () => {
173175
npmMock.setRegistryData({
174176
foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } },
175177
bar: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } },
176178
});
177179
const packageInfos = Object.values(
178-
makePackageInfos({
179-
foo: { beachball: { defaultNpmTag: 'latest' } },
180-
bar: { beachball: { tag: 'beta', defaultNpmTag: 'latest' } },
181-
})
180+
makePackageInfos(
181+
{
182+
foo: {},
183+
bar: { beachball: { tag: 'beta' } },
184+
},
185+
{ defaultNpmTag: 'latest' }
186+
)
182187
);
183188

184-
const versions = await listPackageVersionsByTag(packageInfos, undefined, npmOptions);
189+
const versions = await listPackageVersionsByTag(packageInfos, npmOptions);
185190
expect(versions).toEqual({ foo: '1.0.0', bar: '2.0.0-beta' });
186191
expect(npmMock.mock).toHaveBeenCalledTimes(2);
187192
});
@@ -190,7 +195,7 @@ describe('list npm versions', () => {
190195
npmMock.setRegistryData({ foo: { 'dist-tags': { latest: '1.0.0' } } });
191196
const packageInfos = Object.values(makePackageInfos({ foo: {}, bar: {} }));
192197

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

‎src/commands/sync.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export async function sync(options: BeachballOptions, packageInfos?: PackageInfo
1919
const scopedPackages = new Set(getScopedPackages(options, packageInfos));
2020

2121
const infos = new Map(Object.entries(packageInfos).filter(([pkg, info]) => !info.private && scopedPackages.has(pkg)));
22-
const publishedVersions = await listPackageVersionsByTag([...infos.values()], options.tag, options);
22+
const publishedVersions = await listPackageVersionsByTag([...infos.values()], options);
2323

2424
const modifiedPackages = new Set<string>();
2525

‎src/packageManager/listPackageVersions.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,11 @@ async function getNpmPackageInfo(packageName: string, options: NpmOptions): Prom
5454
return packageVersionsCache[packageName];
5555
}
5656

57+
/**
58+
* List versions matching the appropriate tag for each package (based on combined CLI, package, and repo options)
59+
*/
5760
export async function listPackageVersionsByTag(
5861
packageInfos: PackageInfo[],
59-
tag: string | undefined,
6062
options: NpmOptions
6163
): Promise<{ [pkg: string]: string }> {
6264
const limit = pLimit(options.npmReadConcurrency);
@@ -67,7 +69,7 @@ export async function listPackageVersionsByTag(
6769
limit(async () => {
6870
const info = await getNpmPackageInfo(pkg.name, options);
6971
if (info) {
70-
const npmTag = tag || pkg.combinedOptions.tag || pkg.combinedOptions.defaultNpmTag;
72+
const npmTag = pkg.combinedOptions.tag || pkg.combinedOptions.defaultNpmTag;
7173
const version = npmTag && info['dist-tags']?.[npmTag];
7274
if (version) {
7375
versions[pkg.name] = version;

0 commit comments

Comments
 (0)
⚡