Changelog JSON fixes (#903)

change/beachball-6873c86b-d50c-4bcc-8219-c17d2f9cfe0c.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Add an index signature to ChangelogEntry",
+  "packageName": "beachball",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/beachball-7873c86b-d50c-4bcc-8219-c17d2f9cfe0d.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Stop recording incorrect bump commits in CHANGELOG.json",
+  "packageName": "beachball",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

src/__functional__/changelog/writeChangelog.test.ts

@@ -255,7 +255,7 @@ describe('writeChangelog', () => {
     expect(readChangelogMd(monoRepo.pathTo('packages/foo'))).toMatchSnapshot();
   });
 
-  it('Verify that the changeFile transform functions are run, if provided', async () => {
+  it('runs transform.changeFiles functions if provided', async () => {
     const editedComment: string = 'Edited comment for testing';
     const monoRepo = monoRepoFactory.cloneRepository();
     monoRepo.commitChange('foo');

src/__tests__/changelog/getPackageChangelogs.test.ts

@@ -1,123 +1,203 @@
-import { describe, expect, it } from '@jest/globals';
+import { describe, expect, it, jest } from '@jest/globals';
 import { getPackageChangelogs } from '../../changelog/getPackageChangelogs';
 import { BumpInfo } from '../../types/BumpInfo';
-import { ChangeSet } from '../../types/ChangeInfo';
+import { ChangeFileInfo, ChangeSet } from '../../types/ChangeInfo';
 import { PackageInfos } from '../../types/PackageInfo';
+import { makePackageInfos } from '../../__fixtures__/packageInfos';
+
+// Mock the methods used from workspace-tools so we don't access the filesystem
+jest.mock('workspace-tools', () => ({
+  findProjectRoot: () => '.',
+  getFileAddedHash: () => 'deadbeef',
+}));
+
+function makeChangeInfo(pkg: string, overrides?: Partial<ChangeFileInfo>): ChangeSet[number] {
+  return {
+    changeFile: `${pkg}.json`,
+    change: {
+      comment: `comment for ${pkg}`,
+      dependentChangeType: 'patch',
+      email: '[email protected]',
+      packageName: pkg,
+      type: 'patch',
+      ...overrides,
+    },
+  };
+}
 
 describe('getPackageChangelogs', () => {
-  it('should have multiple comment entries when a package has a changefile AND was part of a dependent bump', () => {
+  it('generates correct changelog entries for a single package', () => {
     const changeFileChangeInfos: ChangeSet = [
-      {
-        changeFile: 'foo.json',
-        change: {
-          comment: 'comment for foo',
-          commit: 'deadbeef',
-          dependentChangeType: 'patch',
-          email: '[email protected]',
-          packageName: 'foo',
-          type: 'patch',
-        },
+      makeChangeInfo('foo'),
+      makeChangeInfo('foo', { type: 'minor', comment: 'other comment' }),
+    ];
+    const packageInfos = makePackageInfos({ foo: { version: '1.0.0' } });
+
+    const changelogs = getPackageChangelogs({
+      changeFileChangeInfos,
+      calculatedChangeTypes: { foo: 'patch' },
+      packageInfos,
+      cwd: '.',
+    });
+
+    expect(changelogs.foo).toEqual({
+      comments: {
+        minor: [{ author: '[email protected]', comment: 'other comment', commit: 'deadbeef', package: 'foo' }],
+        patch: [{ author: '[email protected]', comment: 'comment for foo', commit: 'deadbeef', package: 'foo' }],
+      },
+      date: expect.any(Date),
+      name: 'foo',
+      tag: 'foo_v1.0.0',
+      version: '1.0.0',
+    });
+    expect(changelogs.foo.comments.patch).toHaveLength(1);
+  });
+
+  it('generates correct changelog entries for multiple packages', () => {
+    const changeFileChangeInfos: ChangeSet = [makeChangeInfo('foo'), makeChangeInfo('bar')];
+    const packageInfos = makePackageInfos({
+      foo: { version: '1.0.0' },
+      bar: { version: '2.0.0' },
+    });
+
+    const changelogs = getPackageChangelogs({
+      changeFileChangeInfos,
+      calculatedChangeTypes: { foo: 'patch', bar: 'patch' },
+      packageInfos,
+      cwd: '.',
+    });
+
+    expect(changelogs.foo).toEqual({
+      comments: {
+        patch: [{ author: '[email protected]', comment: 'comment for foo', commit: 'deadbeef', package: 'foo' }],
       },
-      {
-        changeFile: 'bar.json',
-        change: {
-          comment: 'comment for bar',
-          commit: 'deadbeef',
-          dependentChangeType: 'patch',
-          email: '[email protected]',
-          packageName: 'bar',
-          type: 'patch',
-        },
+      date: expect.any(Date),
+      name: 'foo',
+      tag: 'foo_v1.0.0',
+      version: '1.0.0',
+    });
+    expect(changelogs.bar).toEqual({
+      comments: {
+        patch: [{ author: '[email protected]', comment: 'comment for bar', commit: 'deadbeef', package: 'bar' }],
       },
-    ];
+      date: expect.any(Date),
+      name: 'bar',
+      tag: 'bar_v2.0.0',
+      version: '2.0.0',
+    });
+  });
+
+  it('preserves custom properties from change files', () => {
+    const changeFileChangeInfos: ChangeSet = [makeChangeInfo('foo', { extra: 'prop' })];
+    const packageInfos: PackageInfos = makePackageInfos({ foo: { version: '1.0.0' } });
+
+    const changelogs = getPackageChangelogs({
+      changeFileChangeInfos,
+      calculatedChangeTypes: { foo: 'patch' },
+      packageInfos,
+      cwd: '.',
+    });
+
+    expect(changelogs.foo.comments.patch![0]).toMatchObject({ extra: 'prop' });
+  });
+
+  it('records dependent bumps', () => {
+    const changeFileChangeInfos: ChangeSet = [makeChangeInfo('foo')];
 
     const dependentChangedBy: BumpInfo['dependentChangedBy'] = {
       bar: new Set(['foo']),
     };
 
-    const packageInfos: PackageInfos = {
-      foo: {
-        combinedOptions: {} as any,
-        name: 'foo',
-        packageJsonPath: 'packages/foo/package.json',
-        packageOptions: {},
-        private: false,
-        version: '1.0.0',
-        dependencies: {
-          bar: '^1.0.0',
-        },
-      },
-      bar: {
-        combinedOptions: {} as any,
-        name: 'bar',
-        packageJsonPath: 'packages/bar/package.json',
-        packageOptions: {},
-        private: false,
-        version: '1.0.0',
+    const packageInfos = makePackageInfos({
+      foo: { version: '1.0.0' },
+      bar: { version: '2.0.0', dependencies: { foo: '^1.0.0' } },
+    });
+
+    const changelogs = getPackageChangelogs({
+      changeFileChangeInfos,
+      calculatedChangeTypes: { foo: 'patch', bar: 'patch' },
+      dependentChangedBy,
+      packageInfos,
+      cwd: '.',
+    });
+
+    expect(Object.keys(changelogs.foo.comments.patch!)).toHaveLength(1);
+    expect(changelogs.bar).toEqual({
+      comments: {
+        patch: [
+          {
+            author: 'beachball',
+            package: 'bar',
+            comment: 'Bump foo to v1.0.0',
+            // IMPORTANT: this should not record an actual commit hash, because it will be incorrect
+            commit: 'not available',
+          },
+        ],
       },
+      date: expect.any(Date),
+      name: 'bar',
+      tag: 'bar_v2.0.0',
+      version: '2.0.0',
+    });
+    expect(Object.keys(changelogs.bar.comments.patch!)).toHaveLength(1);
+  });
+
+  it('records multiple comment entries when a package has a change file AND was part of a dependent bump', () => {
+    const changeFileChangeInfos: ChangeSet = [makeChangeInfo('foo'), makeChangeInfo('bar')];
+
+    const dependentChangedBy: BumpInfo['dependentChangedBy'] = {
+      bar: new Set(['foo']),
     };
 
-    const changelogs = getPackageChangelogs(
+    const packageInfos = makePackageInfos({
+      foo: { version: '1.0.0' },
+      bar: { version: '2.0.0', dependencies: { foo: '^1.0.0' } },
+    });
+
+    const changelogs = getPackageChangelogs({
       changeFileChangeInfos,
-      { foo: 'patch', bar: 'patch' },
+      calculatedChangeTypes: { foo: 'patch', bar: 'patch' },
       dependentChangedBy,
       packageInfos,
-      '.'
-    );
+      cwd: '.',
+    });
 
-    expect(Object.keys(changelogs.bar.comments.patch!)).toHaveLength(2);
-    expect(Object.keys(changelogs.foo.comments.patch!)).toHaveLength(1);
+    expect(changelogs.bar.comments).toEqual({
+      patch: [
+        expect.objectContaining({ comment: 'comment for bar' }),
+        expect.objectContaining({ comment: 'Bump foo to v1.0.0' }),
+      ],
+    });
+    expect(changelogs.foo.comments).toEqual({
+      patch: [expect.objectContaining({ comment: 'comment for foo' })],
+    });
   });
 
-  it('should not generate change logs for dependent bumps of private packages', () => {
-    const changeFileChangeInfos: ChangeSet = [
-      {
-        changeFile: 'bar.json',
-        change: {
-          comment: 'comment for bar',
-          commit: 'deadbeef',
-          dependentChangeType: 'patch',
-          email: '[email protected]',
-          packageName: 'bar',
-          type: 'patch',
-        },
-      },
-    ];
+  it('does not generate changelogs for dependent bumps of private packages', () => {
+    const changeFileChangeInfos: ChangeSet = [makeChangeInfo('bar')];
 
     const dependentChangedBy: BumpInfo['dependentChangedBy'] = {
       'private-pkg': new Set(['bar']),
     };
 
-    const packageInfos: PackageInfos = {
+    const packageInfos = makePackageInfos({
       'private-pkg': {
-        combinedOptions: {} as any,
-        name: 'private-pkg',
-        packageJsonPath: 'packages/private-pkg/package.json',
-        packageOptions: {},
-        private: true,
-        version: '1.0.0',
-        dependencies: {
-          bar: '^1.0.0',
-        },
-      },
-      bar: {
-        combinedOptions: {} as any,
-        name: 'bar',
-        packageJsonPath: 'packages/bar/package.json',
-        packageOptions: {},
-        private: false,
         version: '1.0.0',
+        private: true,
+        dependencies: { bar: '^1.0.0' },
       },
-    };
+      bar: { version: '1.0.0' },
+    });
 
-    const changelogs = getPackageChangelogs(
+    const changelogs = getPackageChangelogs({
       changeFileChangeInfos,
-      { bar: 'patch', 'private-pkg': 'patch' },
+      calculatedChangeTypes: { bar: 'patch', 'private-pkg': 'patch' },
       dependentChangedBy,
       packageInfos,
-      '.'
-    );
+      cwd: '.',
+    });
 
+    expect(changelogs.bar).toBeTruthy();
     expect(changelogs['private-pkg']).toBeUndefined();
   });
 });

src/__tests__/changelog/renderChangelog.test.ts

@@ -1,6 +1,7 @@
-import { describe, expect, it } from '@jest/globals';
+import { describe, expect, it, jest } from '@jest/globals';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 import { MarkdownChangelogRenderOptions, renderChangelog, markerComment } from '../../changelog/renderChangelog';
+import { ChangelogEntry } from '../../types/ChangeLog';
 
 const previousHeader = `# Change Log - foo
 
@@ -25,18 +26,8 @@ describe('renderChangelog', () => {
         comments: {
           major: [],
           minor: [
-            {
-              comment: 'Awesome change',
-              author: '[email protected]',
-              commit: 'sha1',
-              package: 'foo',
-            },
-            {
-              comment: 'Boring change',
-              author: '[email protected]',
-              commit: 'sha2',
-              package: 'foo',
-            },
+            { comment: 'Awesome change', author: '[email protected]', commit: 'sha1', package: 'foo' },
+            { comment: 'Boring change', author: '[email protected]', commit: 'sha2', package: 'foo' },
           ],
           patch: [
             { comment: 'Fix', author: '[email protected]', commit: 'sha3', package: 'foo' },
@@ -116,4 +107,29 @@ describe('renderChangelog', () => {
     expect(result).toContain('content here'); // includes previous content
     expect(result).toMatchSnapshot();
   });
+
+  it('passes custom change file properties to renderers', async () => {
+    const options = getOptions();
+    options.newVersionChangelog.comments = {
+      patch: [
+        {
+          comment: 'Awesome change',
+          author: '[email protected]',
+          commit: 'sha1',
+          package: 'foo',
+          extra: 'custom',
+        },
+      ],
+    };
+    options.changelogOptions.customRenderers = {
+      renderEntry: jest.fn(async (entry: ChangelogEntry) => `- ${entry.comment} ${entry.extra})`),
+    };
+
+    const result = await renderChangelog(options);
+    expect(result).toContain('Awesome change custom');
+    expect(options.changelogOptions.customRenderers.renderEntry).toHaveBeenCalledWith(
+      expect.objectContaining({ extra: 'custom' }),
+      expect.anything()
+    );
+  });
 });