diff --git a/change/beachball-848fed39-4933-483a-a038-1470bebe299b.json b/change/beachball-848fed39-4933-483a-a038-1470bebe299b.json new file mode 100644 index 00000000..30b30239 --- /dev/null +++ b/change/beachball-848fed39-4933-483a-a038-1470bebe299b.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Remove newPackages from main BumpInfo", + "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 892508c9..ee64a407 100644 --- a/src/__tests__/bump/updateRelatedChangeType.test.ts +++ b/src/__tests__/bump/updateRelatedChangeType.test.ts @@ -26,7 +26,6 @@ describe('updateRelatedChangeType', () => { baz: { combinedOptions: { disallowedChangeTypes: [], defaultNpmTag: 'latest' } }, }), modifiedPackages: new Set(), - newPackages: new Set(), packageGroups: {}, scopedPackages: new Set(), dependentChangedBy: {}, diff --git a/src/__tests__/publish/tagPackages.test.ts b/src/__tests__/publish/tagPackages.test.ts index 965328b1..617ce185 100644 --- a/src/__tests__/publish/tagPackages.test.ts +++ b/src/__tests__/publish/tagPackages.test.ts @@ -4,7 +4,6 @@ import { gitFailFast } from 'workspace-tools'; import { initMockLogs } from '../../__fixtures__/mockLogs'; import { tagPackages } from '../../publish/tagPackages'; import { generateTag } from '../../git/generateTag'; -import { BumpInfo } from '../../types/BumpInfo'; import { makePackageInfos } from '../../__fixtures__/packageInfos'; jest.mock('workspace-tools', () => ({ @@ -15,7 +14,10 @@ const createTagParameters = (tag: string, cwd: string) => { return [['tag', '-a', '-f', tag, '-m', tag], { cwd }]; }; -const noTagBumpInfo: Partial = { +type TagBumpInfo = Parameters[0]; + +/** foo and bar disable gitTags */ +const noTagBumpInfo: TagBumpInfo = { calculatedChangeTypes: { foo: 'minor', bar: 'major', @@ -31,14 +33,12 @@ const noTagBumpInfo: Partial = { }, }), modifiedPackages: new Set(['foo', 'bar']), - newPackages: new Set(), + newPackages: [], }; -const oneTagBumpInfo: Partial = { - calculatedChangeTypes: { - foo: 'minor', - bar: 'major', - }, +/** foo enables gitTags, bar disables it */ +const oneTagBumpInfo: TagBumpInfo = { + ...noTagBumpInfo, packageInfos: makePackageInfos({ foo: { version: '1.0.0', @@ -49,15 +49,13 @@ const oneTagBumpInfo: Partial = { combinedOptions: { gitTags: false }, }, }), - modifiedPackages: new Set(['foo', 'bar']), - newPackages: new Set(), }; -const emptyBumpInfo: Partial = { +const emptyBumpInfo: TagBumpInfo = { calculatedChangeTypes: {}, packageInfos: {}, modifiedPackages: new Set(), - newPackages: new Set(), + newPackages: [], }; describe('tagPackages', () => { @@ -71,14 +69,14 @@ describe('tagPackages', () => { jest.restoreAllMocks(); }); - it('does not create tag for packages with gitTags=false', () => { + it('does not create package tag for packages with gitTags=false', () => { // Also verifies that if `gitTags` is false overall, it doesn't create a git tag for the dist tag (`tag`) - tagPackages(noTagBumpInfo as BumpInfo, { path: '', gitTags: false, tag: '' }); + tagPackages(noTagBumpInfo, { path: '', gitTags: false, tag: '' }); expect(gitFailFast).not.toHaveBeenCalled(); }); - it('creates tag for packages with gitTags=true', () => { - tagPackages(oneTagBumpInfo as BumpInfo, { path: '', gitTags: false, tag: '' }); + it('creates package tag for packages with gitTags=true', () => { + tagPackages(oneTagBumpInfo, { path: '', gitTags: false, tag: '' }); expect(gitFailFast).toHaveBeenCalledTimes(1); // verify git is being called to create new auto tag for foo and bar @@ -86,18 +84,35 @@ describe('tagPackages', () => { expect(gitFailFast).toHaveBeenCalledWith(...createTagParameters(newFooTag, '')); }); - it('does not create git tag for empty dist tag', () => { - tagPackages(emptyBumpInfo as BumpInfo, { path: '', gitTags: true, tag: '' }); + it('creates package tag for new packages with gitTags=true', () => { + tagPackages( + { ...oneTagBumpInfo, newPackages: ['foo'], modifiedPackages: new Set() }, + { path: '', gitTags: false, tag: '' } + ); + expect(gitFailFast).toHaveBeenCalledTimes(1); + + // verify git is being called to create new auto tag for foo + const newFooTag = generateTag('foo', oneTagBumpInfo.packageInfos!['foo'].version); + expect(gitFailFast).toHaveBeenCalledWith(...createTagParameters(newFooTag, '')); + }); + + it('does not create package tag for packages with changeType none', () => { + tagPackages({ ...oneTagBumpInfo, calculatedChangeTypes: { foo: 'none' } }, { path: '', gitTags: false, tag: '' }); expect(gitFailFast).not.toHaveBeenCalled(); }); - it('does not create git tag for "latest" dist tag', () => { - tagPackages(emptyBumpInfo as BumpInfo, { path: '', gitTags: true, tag: 'latest' }); + it('does not create overall git tag for empty dist tag', () => { + tagPackages(emptyBumpInfo, { path: '', gitTags: true, tag: '' }); expect(gitFailFast).not.toHaveBeenCalled(); }); - it('creates git tag for non-"latest" dist tag', () => { - tagPackages(emptyBumpInfo as BumpInfo, { path: '', gitTags: true, tag: 'abc' }); + it('does not create overall git tag for "latest" dist tag', () => { + tagPackages(emptyBumpInfo, { path: '', gitTags: true, tag: 'latest' }); + expect(gitFailFast).not.toHaveBeenCalled(); + }); + + it('creates overall git tag for non-"latest" dist tag', () => { + tagPackages(emptyBumpInfo, { path: '', gitTags: true, tag: 'abc' }); expect(gitFailFast).toBeCalledTimes(1); expect(gitFailFast).toHaveBeenCalledWith(...createTagParameters('abc', '')); }); diff --git a/src/bump/gatherBumpInfo.ts b/src/bump/gatherBumpInfo.ts index f699d375..e6a0b290 100644 --- a/src/bump/gatherBumpInfo.ts +++ b/src/bump/gatherBumpInfo.ts @@ -28,7 +28,6 @@ export function gatherBumpInfo(options: BeachballOptions, packageInfos: PackageI packageGroups: getPackageGroups(packageInfos, options.path, options.groups), changeFileChangeInfos: changes, modifiedPackages: new Set(), - newPackages: new Set(), scopedPackages: new Set(getScopedPackages(options, packageInfos)), dependentChangedBy: {}, }; diff --git a/src/bump/getDependentsForPackages.ts b/src/bump/getDependentsForPackages.ts index f5ab3955..35037337 100644 --- a/src/bump/getDependentsForPackages.ts +++ b/src/bump/getDependentsForPackages.ts @@ -12,7 +12,7 @@ export function getDependentsForPackages( ): PackageDependents { const { packageInfos, scopedPackages } = bumpInfo; - const dependents: PackageDependents = {}; + const dependents: { [pkgName: string]: string[] } = {}; for (const [pkgName, info] of Object.entries(packageInfos)) { if (!scopedPackages.has(pkgName)) { diff --git a/src/commands/publish.ts b/src/commands/publish.ts index 8a3cd118..880939b7 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -7,6 +7,7 @@ import { bumpAndPush } from '../publish/bumpAndPush'; import { publishToRegistry } from '../publish/publishToRegistry'; import { getNewPackages } from '../publish/getNewPackages'; import { getPackageInfos } from '../monorepo/getPackageInfos'; +import { PublishBumpInfo } from '../types/BumpInfo'; export async function publish(options: BeachballOptions): Promise { console.log('\nPreparing to publish'); @@ -57,14 +58,21 @@ export async function publish(options: BeachballOptions): Promise { gitFailFast(['checkout', '-b', publishBranch], { cwd }); console.log(`\nGathering info ${options.bump ? 'to bump versions' : 'about versions and changes'}`); - const bumpInfo = gatherBumpInfo(options, oldPackageInfos); + const bumpInfo: PublishBumpInfo = gatherBumpInfo(options, oldPackageInfos); + if (options.new) { // Publish newly created packages even if they don't have change files // (this is unlikely unless the packages were pushed without a PR that runs "beachball check") - bumpInfo.newPackages = new Set(await getNewPackages(bumpInfo, options)); + console.log( + '\nFetching all unmodified packages from the registry to check if there are any ' + + "newly-added packages that didn't have a change file...\n" + + '(If your PR build runs `beachball check`, it should be safe to disable this step by ' + + 'removing `new: true` from your config or removing `--new` from your publish command.)' + ); + bumpInfo.newPackages = await getNewPackages(bumpInfo, options); } - // Step 1. Bump + npm publish + // Step 1. Bump on disk + npm publish // npm / yarn publish if (options.publish) { console.log('\nBumping versions and publishing to npm'); @@ -75,7 +83,8 @@ export async function publish(options: BeachballOptions): Promise { } // Step 2. - // - reset, fetch latest from origin/master (to ensure less chance of conflict), then bump again + commit + // - reset, fetch latest from origin/master (to ensure less chance of conflict), + // then bump on disk again + commit if (options.bump && branch && options.push) { // this does its own section logging await bumpAndPush(bumpInfo, publishBranch, options); diff --git a/src/publish/getPackagesToPublish.ts b/src/publish/getPackagesToPublish.ts index c3e45c5e..e919a2d7 100644 --- a/src/publish/getPackagesToPublish.ts +++ b/src/publish/getPackagesToPublish.ts @@ -1,5 +1,5 @@ import { formatList } from '../logging/format'; -import { BumpInfo } from '../types/BumpInfo'; +import { PublishBumpInfo } from '../types/BumpInfo'; import { toposortPackages } from './toposortPackages'; /** @@ -10,10 +10,10 @@ import { toposortPackages } from './toposortPackages'; * based on the dependency graph to ensure they're published in the correct order, and any * new/modified packages that will be skipped (and why) are logged to the console. */ -export function getPackagesToPublish(bumpInfo: BumpInfo, validationMode?: boolean): string[] { - const { modifiedPackages, newPackages, packageInfos } = bumpInfo; +export function getPackagesToPublish(bumpInfo: PublishBumpInfo, validationMode?: boolean): string[] { + const { modifiedPackages, newPackages, packageInfos, calculatedChangeTypes, scopedPackages } = bumpInfo; - let packages = [...modifiedPackages, ...newPackages]; + let packages = [...modifiedPackages, ...(newPackages || [])]; if (!validationMode) { // skip this step when called from `validate` since it's not needed and might be slow packages = toposortPackages(packages, packageInfos); @@ -22,15 +22,15 @@ export function getPackagesToPublish(bumpInfo: BumpInfo, validationMode?: boolea const skippedPackageReasons: string[] = []; for (const pkg of packages) { - const packageInfo = bumpInfo.packageInfos[pkg]; - const changeType = bumpInfo.calculatedChangeTypes[pkg]; + const packageInfo = packageInfos[pkg]; + const changeType = calculatedChangeTypes[pkg]; let skipReason = ''; if (changeType === 'none') { skipReason = 'has change type none'; } else if (packageInfo.private) { skipReason = 'is private'; - } else if (!bumpInfo.scopedPackages.has(pkg)) { + } else if (!scopedPackages.has(pkg)) { skipReason = 'is out-of-scope'; } diff --git a/src/publish/publishToRegistry.ts b/src/publish/publishToRegistry.ts index 24a9a3c6..ca6f7169 100644 --- a/src/publish/publishToRegistry.ts +++ b/src/publish/publishToRegistry.ts @@ -1,6 +1,6 @@ import _ from 'lodash'; import { performBump } from '../bump/performBump'; -import { BumpInfo } from '../types/BumpInfo'; +import { PublishBumpInfo } from '../types/BumpInfo'; import { BeachballOptions } from '../types/BeachballOptions'; import { packagePublish } from '../packageManager/packagePublish'; import { validatePackageVersions } from './validatePackageVersions'; @@ -14,7 +14,7 @@ import { callHook } from '../bump/callHook'; * Publish all the bumped packages to the registry. * This will bump packages on the filesystem first if `options.bump` is true. */ -export async function publishToRegistry(originalBumpInfo: BumpInfo, options: BeachballOptions): Promise { +export async function publishToRegistry(originalBumpInfo: PublishBumpInfo, options: BeachballOptions): Promise { const bumpInfo = _.cloneDeep(originalBumpInfo); if (options.bump) { @@ -37,7 +37,8 @@ export async function publishToRegistry(originalBumpInfo: BumpInfo, options: Bea process.exit(1); } - // performing publishConfig and workspace version overrides requires this procedure to ONLY be run right before npm publish, but NOT in the git push + // performing publishConfig and workspace version overrides requires this procedure to + // ONLY be run right before npm publish, but NOT in the git push performPublishOverrides(packagesToPublish, bumpInfo.packageInfos); // if there is a prepublish hook perform a prepublish pass, calling the routine on each package diff --git a/src/publish/tagPackages.ts b/src/publish/tagPackages.ts index 8f233a6b..540497b9 100644 --- a/src/publish/tagPackages.ts +++ b/src/publish/tagPackages.ts @@ -1,7 +1,8 @@ -import { BumpInfo } from '../types/BumpInfo'; +import { PublishBumpInfo } from '../types/BumpInfo'; import { generateTag } from '../git/generateTag'; import { gitFailFast } from 'workspace-tools'; import { BeachballOptions } from '../types/BeachballOptions'; +import { DeepReadonly } from '../types/DeepReadonly'; function createTag(tag: string, cwd: string): void { gitFailFast(['tag', '-a', '-f', tag, '-m', tag], { cwd }); @@ -12,13 +13,19 @@ function createTag(tag: string, cwd: string): void { * Also, if git tags aren't disabled for the repo and the overall dist-tag (`options.tag`) has a * non-default value (not "latest"), create a git tag for the dist-tag. */ -export function tagPackages(bumpInfo: BumpInfo, options: Pick): void { +export function tagPackages( + bumpInfo: Pick< + DeepReadonly, + 'modifiedPackages' | 'newPackages' | 'packageInfos' | 'calculatedChangeTypes' + >, + options: Pick +): void { const { gitTags, tag: distTag, path: cwd } = options; - const { modifiedPackages, newPackages } = bumpInfo; + const { modifiedPackages, newPackages, packageInfos, calculatedChangeTypes } = bumpInfo; - for (const pkg of [...modifiedPackages, ...newPackages]) { - const packageInfo = bumpInfo.packageInfos[pkg]; - const changeType = bumpInfo.calculatedChangeTypes[pkg]; + for (const pkg of [...modifiedPackages, ...(newPackages || [])]) { + const packageInfo = packageInfos[pkg]; + const changeType = calculatedChangeTypes[pkg]; // Do not tag change type of "none", private packages, or packages opting out of tagging if (changeType === 'none' || packageInfo.private || !packageInfo.combinedOptions.gitTags) { continue; diff --git a/src/types/BumpInfo.ts b/src/types/BumpInfo.ts index 40fbbb11..96de649d 100644 --- a/src/types/BumpInfo.ts +++ b/src/types/BumpInfo.ts @@ -21,9 +21,6 @@ export type BumpInfo = { /** Set of packages that had been modified */ modifiedPackages: Set; - /** Set of new packages detected in this info */ - newPackages: Set; - /** Map from package name to its internal dependency names that were bumped. */ dependentChangedBy: { [pkgName: string]: Set }; @@ -32,4 +29,16 @@ export type BumpInfo = { }; /** Dependents cache (child points to parents): if A depends on B, then `{ B: [A] }` */ -export type PackageDependents = { [pkgName: string]: string[] }; +export type PackageDependents = { readonly [pkgName: string]: ReadonlyArray }; + +/** + * Bump info with additional property set/used only during publishing (not while calculating + * packages to bump). + */ +export type PublishBumpInfo = BumpInfo & { + /** + * Set of packages detected in this info which weren't previously published and didn't have + * change files. (Only populated if `options.new` is set.) + */ + newPackages?: ReadonlyArray; +};