remove bump exception

Author: ecraig12345

src/__tests__/publish/getPackagesToPublish.test.ts

@@ -2,7 +2,6 @@ import { describe, it, expect } from '@jest/globals';
 import { makePackageInfos, type PartialPackageInfos } from '../../__fixtures__/packageInfos';
 import { getPackagesToPublish } from '../../publish/getPackagesToPublish';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
-import type { BeachballOptions } from '../../types/BeachballOptions';
 
 type PartialBumpInfo = Parameters<typeof getPackagesToPublish>[0];
 
@@ -14,12 +13,10 @@ describe('getPackagesToPublish', () => {
    * - all packages are in scope and modified
    * - all modified packages have changeType: 'patch'
    * - logging enabled
-   * - `bump: true`
    */
   function getPackagesToPublishWrapper(
     partialBumpInfo: { packageInfos: PartialPackageInfos } & Partial<Omit<PartialBumpInfo, 'packageInfos'>>,
-    options?: Pick<BeachballOptions, 'bump'>,
-    params?: Parameters<typeof getPackagesToPublish>[2]
+    params?: Parameters<typeof getPackagesToPublish>[1]
   ) {
     const { packageInfos, modifiedPackages = new Set(Object.keys(packageInfos)), ...rest } = partialBumpInfo;
     const bumpInfo: PartialBumpInfo = {
@@ -29,7 +26,7 @@ describe('getPackagesToPublish', () => {
       calculatedChangeTypes: Object.fromEntries([...modifiedPackages].map(pkg => [pkg, 'patch'])),
       ...rest,
     };
-    return getPackagesToPublish(bumpInfo, { bump: true, ...options }, { logSkipped: true, ...params });
+    return getPackagesToPublish(bumpInfo, { logSkipped: true, ...params });
   }
 
   it('returns empty if no modified packages', () => {
@@ -113,7 +110,7 @@ describe('getPackagesToPublish', () => {
   });
 
   // This happens for reasons outlined in https://github.com/microsoft/beachball/issues/1123
-  it('excludes packages without calculated change type with bump: true (default)', () => {
+  it('excludes packages without calculated change type', () => {
     const result = getPackagesToPublishWrapper({
       packageInfos: { 'pkg-a': {}, 'pkg-b': {} },
       modifiedPackages: new Set(['pkg-a', 'pkg-b']),
@@ -126,21 +123,6 @@ describe('getPackagesToPublish', () => {
     `);
   });
 
-  // This might change later if bump: false is actually thought through...
-  // https://github.com/microsoft/beachball/issues/1125
-  it('includes packages without calculated change type with bump: false', () => {
-    const result = getPackagesToPublishWrapper(
-      {
-        packageInfos: { 'pkg-a': {}, 'pkg-b': {} },
-        modifiedPackages: new Set(['pkg-a', 'pkg-b']),
-        calculatedChangeTypes: { 'pkg-a': 'patch' },
-      },
-      { bump: false }
-    );
-    expect(result).toEqual(['pkg-a', 'pkg-b']);
-    expect(logs.mocks.log).not.toHaveBeenCalled();
-  });
-
   it('returns packages in topological order if requested', () => {
     // Sorting is tested in toposortPackages, so just use a simple graph here
     const result = getPackagesToPublishWrapper(
@@ -151,7 +133,6 @@ describe('getPackagesToPublish', () => {
           'pkg-c': {},
         },
       },
-      undefined,
       { toposort: true }
     );
     expect(result).toEqual(['pkg-c', 'pkg-b', 'pkg-a']);

src/publish/getPackagesToPublish.ts

@@ -1,5 +1,4 @@
 import { bulletedList } from '../logging/bulletedList';
-import type { BeachballOptions } from '../types/BeachballOptions';
 import type { PublishBumpInfo } from '../types/BumpInfo';
 import { toposortPackages } from './toposortPackages';
 
@@ -12,7 +11,6 @@ export function getPackagesToPublish(
     PublishBumpInfo,
     'modifiedPackages' | 'newPackages' | 'packageInfos' | 'calculatedChangeTypes' | 'scopedPackages'
   >,
-  options: Pick<BeachballOptions, 'bump'>,
   params?: {
     /** If true, topologically sort the package names based on the dependency graph (may be slow) */
     toposort?: boolean;
@@ -40,10 +38,7 @@ export function getPackagesToPublish(
       skipReason = 'is private';
     } else if (!scopedPackages.allInScope && !scopedPackages.has(pkg)) {
       skipReason = 'is out-of-scope';
-    } else if (options.bump && !changeType && !newPackages?.includes(pkg)) {
-      // bump=false is the only time when this might be valid, but that behavior needs more thought...
-      // TODO: remove `options.bump` exception and/or figure out how it should actually be handled
-      // https://github.com/microsoft/beachball/issues/1125
+    } else if (!changeType && !newPackages?.includes(pkg)) {
       skipReason = 'is not bumped (no calculated change type)';
     }
 

src/publish/publishToRegistry.ts

@@ -27,7 +27,7 @@ export async function publishToRegistry(bumpInfo: PublishBumpInfo, options: Beac
   }
 
   // get the packages to publish, reducing the set by packages that don't need publishing
-  const packagesToPublish = getPackagesToPublish(bumpInfo, options, { toposort: true, logSkipped: true });
+  const packagesToPublish = getPackagesToPublish(bumpInfo, { toposort: true, logSkipped: true });
 
   let invalid = false;
   // TODO: for bump=false, this should validate the on-disk versions, not in-memory bumped versions

src/validation/validate.ts

@@ -213,7 +213,7 @@ export function validate(
     // Unfortunately, to get full info about which dependents would be bumped, it's probably necessary
     // to calculate the full bump info.
     bumpInfo = bumpInMemory(options, { originalPackageInfos, packageGroups, changeSet, scopedPackages });
-    const packagesToPublish = getPackagesToPublish(bumpInfo, options);
+    const packagesToPublish = getPackagesToPublish(bumpInfo);
 
     if (!validatePackageDependencies(packagesToPublish, bumpInfo.packageInfos)) {
       logValidationError(`One or more published packages depend on an unpublished package!