Revert prebump version change, stop passing packageInfos

change/beachball-2aa1650c-1853-4549-bd89-242886aecfa8.json

@@ -0,0 +1,7 @@
+{
+  "type": "major",
+  "comment": "`hooks.prebump` no longer receives `packageInfos`. This was never in the signature, and trying to modify it is likely to not fully behave as you might expect. Please open an issue if you were using this and we can find an alternative.",
+  "packageName": "beachball",
+  "email": "elcraig@microsoft.com",
+  "dependentChangeType": "patch"
+}

docs/overview/v3-migration.md

@@ -70,10 +70,6 @@ To migrate, simply remove the leading `!` from all `exclude` patterns.
 
 The logic for determining the comparison remote and branch is stricter: beachball now throws if no remotes are defined, or if the root `package.json` specifies a `repository` field but no matching remote is found. If your `branch` option contains a `/`, beachball checks whether the leading segment matches a configured remote name, and falls back to the default remote otherwise.
 
-### `prebump` hook version parameter now matches the docs
-
-In v2, `hooks.prebump` received the post-bump package version even though the hook runs before Beachball writes version changes to disk. In v3, this is corrected to receive the pre-bump version as documented.
-
 ### Custom changelog rendering changes
 
 Only relevant for custom changelog renderers: `PackageChangelog.tag` and `ChangelogJsonEntry.tag` are now `undefined` when the package had no associated git tag (previously a value was always present).
@@ -98,18 +94,15 @@ In v3, `shouldPublish: false` packages are full participants in all steps of the
 - If a published package has a `shouldPublish: false` package in its production dependencies, Beachball will exit with an error (same as with `private: true` deps)
 - Since `shouldPublish: false` is redundant with `private: true`, `beachball migrate` reports this as an error
 
-### Renamed options
-
-- Rename `BeachballOptions.changelog.groups[*].masterPackageName` to `mainPackageName`
-
-### Removed options
+### `BeachballOptions`/`RepoOptions` updates
 
-- `new`: This option was never needed if PR builds run `beachball check` (a new package without a change file already causes an error), and it had a significant performance cost because it checked the registry for _all_ unmodified packages.
-- `packStyle`: packing always uses the layered style now.
-- `help` and `version` properties: these still work on the command line but were removed from `BeachballOptions`
+- Rename `changelog.groups[*].masterPackageName` to `mainPackageName`.
+- Removed rarely-used options:
+  - `new`: This option was never needed if PR builds run `beachball check` (a new package without a change file already causes an error), and it had a significant performance cost because it checked the registry for _all_ unmodified packages.
+  - `packStyle`: packing always uses the layered style now.
+  - `help` and `version` properties: these still work on the command line but were removed from `BeachballOptions`
+- `hooks.prebump` no longer receives `packageInfos`. This was never in the signature, and trying to modify it may lead to unexpected behavior. Please open an issue if you were using this and we can find an alternative.
 
 ### Other changes
 
-- If you're deep importing beachball's internal helpers:
-  - Some deprecated signatures have been removed. Use the new signatures (which pre-calculate and share context) instead.
-  - `BumpInfo` now contains `originalPackageInfos` for `prebump` hook correctness.
+- If you're deep importing beachball's internal helpers, some deprecated signatures have been removed. Use the new signatures (which pre-calculate and share context) instead.

packages/beachball/src/__e2e__/bump.test.ts

@@ -555,11 +555,12 @@ describe('bump command', () => {
         prebump: jest.fn<NonNullable<HooksOptions['prebump']>>(async (packagePath, name, version) => {
           expect(packagePath.endsWith('pkg-1')).toBeTruthy();
           expect(name).toBe('pkg-1');
-          expect(version).toBe('1.0.0');
+          // "prebump" receives the bumped version, but is called before writing to disk
+          expect(version).toBe('1.1.0');
 
           await new Promise(resolve => setTimeout(resolve, 0)); // simulate async work
           const jsonPath = path.join(packagePath, 'package.json');
-          expect(readJson<PackageJson>(jsonPath).version).toBe('1.0.0');
+          expect(readJson<PackageJson>(jsonPath).version).toBe('1.0.0'); // not bumped on disk yet
         }),
         postbump: jest.fn<NonNullable<HooksOptions['postbump']>>(async (packagePath, name, version) => {
           expect(packagePath.endsWith('pkg-1')).toBeTruthy();

packages/beachball/src/__functional__/commands/migrate.test.ts

@@ -2,7 +2,7 @@ import { describe, expect, it, afterEach, jest } from '@jest/globals';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { migrate } from '../../commands/migrate';
 import { getOptions as _getOptions } from '../../options/getOptions';
-import type { RepoOptions } from '../../types/BeachballOptions';
+import type { HooksOptions, RepoOptions } from '../../types/BeachballOptions';
 import { removeTempDir } from '../../__fixtures__/tmpdir';
 import { createTestFileStructureType, updateJsonFile } from '../../__fixtures__/createTestFileStructure';
 import fs from 'fs';
@@ -68,6 +68,20 @@ describe('migrate command', () => {
     `);
   });
 
+  it('errors on "hooks.prebump" with more than 3 params', () => {
+    tempRoot = createTestFileStructureType('single');
+    const fn: HooksOptions['postbump'] = (_pth, _name, _version, _pkgInfos) => {};
+    const options = getOptions({
+      hooks: { prebump: fn as HooksOptions['prebump'] },
+    });
+
+    expect(() => migrate(options)).toThrow(BeachballError);
+    expect(logs.getMockLines('all')).toMatchInlineSnapshot(`
+      "[error] The following updates are needed for v3:
+      [error]   • \`hooks.prebump\` no longer receives \`packageInfos\`. See migration guide."
+    `);
+  });
+
   it('warns on public packages using shouldPublish option', () => {
     tempRoot = createTestFileStructureType('monorepo');
     updateJsonFile(path.join(tempRoot, 'packages/foo/package.json'), { beachball: { shouldPublish: false } });

packages/beachball/src/__functional__/publish/publishToRegistry.test.ts

@@ -26,7 +26,6 @@ describe('publishToRegistry', () => {
   const npmMock = initNpmMock();
   const logs = initMockLogs();
 
-  /** Needs a real fake temp root for mock npm publish */
   let tempRoot: string;
   let defaultOptions: BeachballOptions;
 
@@ -62,7 +61,6 @@ describe('publishToRegistry', () => {
     return {
       changeFileChangeInfos: [],
       packageInfos,
-      originalPackageInfos: makePackageInfos(partialPackageInfos, { path: tempRoot }),
       calculatedChangeTypes: Object.fromEntries(names.map(n => [n, 'patch' as const])),
       packageGroups: {},
       modifiedPackages: new Set(names),
@@ -231,7 +229,7 @@ describe('publishToRegistry', () => {
       expect(prebump).toHaveBeenCalledTimes(1);
       expect(postbump).toHaveBeenCalledTimes(1);
       const fooPath = expect.stringMatching(/packages[\\/]foo$/);
-      expect(prebump as typeof postbump).toHaveBeenCalledWith(fooPath, 'foo', '1.0.0', expect.anything());
+      expect(prebump).toHaveBeenCalledWith(fooPath, 'foo', '1.0.1');
       expect(postbump).toHaveBeenCalledWith(fooPath, 'foo', '1.0.1', expect.anything());
     });