Include workspace and catalog bumps in the changelog (#1216)

Author: ecraig12345

change/change-7d418d34-8558-4c18-905c-69ed9fdd678c.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "type": "patch",
+      "comment": "Include dependent bumps for `workspace:` and `catalog:` dependencies in the changelog",
+      "packageName": "beachball",
+      "email": "elcraig@microsoft.com",
+      "dependentChangeType": "patch"
+    }
+  ]
+}

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

@@ -485,10 +485,9 @@ describe('bump command', () => {
     // this gets a changelog entry since the workspace:^1.0.0 dep was updated
     // (a little debatable whether the current text is correct)
     expect(pkg3Changelog!.entries[0].comments.patch![0].comment).toBe('Bump pkg-2 to v1.0.1');
-    // Current behavior: dependentChangedBy misses deps like workspace:~ that don't change,
-    // so the bump of pkg-1 will be missing from pkg-2's changelog.
-    // https://github.com/microsoft/beachball/issues/981
-    expect(readChangelogJson(repo.pathTo('packages/pkg-2'))).toBeNull();
+    // pkg-2 also gets a changelog entry even though its workspace:~ range string is unchanged.
+    const pkg2Changelog = readChangelogJson(repo.pathTo('packages/pkg-2'));
+    expect(pkg2Changelog!.entries[0].comments.patch![0].comment).toBe('Bump pkg-1 to v1.1.0');
   });
 
   it('bumps dependents with catalog: deps', async () => {
@@ -527,10 +526,11 @@ describe('bump command', () => {
     expect(packageInfos['pkg-3']).toEqual({ ...originalPackageInfos['pkg-3'], version: '1.0.1' });
 
     expect(readChangelogJson(repo.pathTo('packages/pkg-1'))).not.toBeNull();
-    // Current behavior: dependentChangedBy misses catalog: deps, so there are no changelog entries
-    // https://github.com/microsoft/beachball/issues/981
-    expect(readChangelogJson(repo.pathTo('packages/pkg-2'))).toBeNull();
-    expect(readChangelogJson(repo.pathTo('packages/pkg-3'))).toBeNull();
+    // catalog: deps don't change the dep range string, but dependents still get changelog entries.
+    const pkg2Changelog = readChangelogJson(repo.pathTo('packages/pkg-2'));
+    expect(pkg2Changelog!.entries[0].comments.patch![0].comment).toBe('Bump pkg-1 to v1.1.0');
+    const pkg3Changelog = readChangelogJson(repo.pathTo('packages/pkg-3'));
+    expect(pkg3Changelog!.entries[0].comments.patch![0].comment).toBe('Bump pkg-2 to v1.0.1');
   });
 
   // Explicit tests for sync/async hooks aren't necessary, especially since these are slow tests.

packages/beachball/src/__tests__/bump/bumpInMemory.test.ts

@@ -260,8 +260,7 @@ describe('bumpInMemory', () => {
     const { packageInfos, modifiedPackages, calculatedChangeTypes, dependentChangedBy } = bumpInfo;
     expect(modifiedPackages).toEqual(new Set(['pkg-1', 'pkg-2', 'pkg-3']));
     expect(calculatedChangeTypes).toEqual({ 'pkg-1': 'minor', 'pkg-2': 'patch', 'pkg-3': 'patch' });
-    // Current behavior: dependentChangedBy misses file: deps, so the packages won't be in the changelog.
-    // https://github.com/microsoft/beachball/issues/981
+    // file: deps don't produce changelog entries — currently this is intentional, per https://github.com/microsoft/beachball/pull/1080
     expect(dependentChangedBy).toEqual({});
 
     // All the packages are bumped despite the file: dep specs.
@@ -287,11 +286,8 @@ describe('bumpInMemory', () => {
     expect(modifiedPackages).toEqual(new Set(['pkg-1', 'pkg-2', 'pkg-3']));
     expect(calculatedChangeTypes).toEqual({ 'pkg-1': 'minor', 'pkg-2': 'patch', 'pkg-3': 'patch' });
     expect(dependentChangedBy).toEqual({
+      'pkg-2': new Set(['pkg-1']),
       'pkg-3': new Set(['pkg-2']),
-      // Current behavior: dependentChangedBy misses deps like workspace:~ that don't change,
-      // so the bump of pkg-1 will be missing from pkg-2's changelog.
-      // https://github.com/microsoft/beachball/issues/981
-      // 'pkg-2': new Set(['pkg-1']),
     });
 
     // All the dependent packages are bumped despite the workspace: dep specs
@@ -310,7 +306,6 @@ describe('bumpInMemory', () => {
       //   pkg-2: workspace:^1.0.0
       packageFolders: {
         'pkg-1': { version: '1.0.0' },
-        // both of these are detected as dependent bumps but currently missed from changelog
         'pkg-2': { version: '1.0.0', dependencies: { 'pkg-1': 'catalog:' } },
         'pkg-3': { version: '1.0.0', dependencies: { 'pkg-2': 'catalog:' } },
       },
@@ -322,10 +317,8 @@ describe('bumpInMemory', () => {
     expect(modifiedPackages).toEqual(new Set(['pkg-1', 'pkg-2', 'pkg-3']));
     expect(calculatedChangeTypes).toEqual({ 'pkg-1': 'minor', 'pkg-2': 'patch', 'pkg-3': 'patch' });
     expect(dependentChangedBy).toEqual({
-      // Current behavior: dependentChangedBy misses all catalog: deps pointing to workspace: versions
-      // https://github.com/microsoft/beachball/issues/981
-      // 'pkg-2': new Set(['pkg-1']),
-      // 'pkg-3': new Set(['pkg-2']),
+      'pkg-2': new Set(['pkg-1']),
+      'pkg-3': new Set(['pkg-2']),
     });
 
     // All the dependent packages are bumped despite the catalog: dep specs

packages/beachball/src/__tests__/bump/bumpMinSemverRange.test.ts

@@ -4,24 +4,24 @@ import { bumpMinSemverRange } from '../../bump/bumpMinSemverRange';
 describe('bumpMinSemverRange', () => {
   it('preserves *', () => {
     const result = bumpMinSemverRange({ newVersion: '1.0.0', currentRange: '*' });
-    expect(result).toBe('*');
+    expect(result).toBeUndefined();
   });
 
   it('preserves file: protocol with relative path', () => {
     const result = bumpMinSemverRange({ newVersion: '1.0.0', currentRange: 'file:../local-package' });
-    expect(result).toBe('file:../local-package');
+    expect(result).toBeUndefined();
   });
 
   it('preserves file: protocol with absolute path', () => {
     const result = bumpMinSemverRange({ newVersion: '1.0.0', currentRange: 'file:/absolute/path/to/package' });
-    expect(result).toBe('file:/absolute/path/to/package');
+    expect(result).toBeUndefined();
   });
 
   it('preserves catalog: protocol', () => {
     let result = bumpMinSemverRange({ newVersion: '1.0.0', currentRange: 'catalog:' });
-    expect(result).toBe('catalog:');
+    expect(result).toBe(true);
     result = bumpMinSemverRange({ newVersion: '1.0.0', currentRange: 'catalog:foo' });
-    expect(result).toBe('catalog:foo');
+    expect(result).toBe(true);
   });
 
   it.each(['~', '^'])('preserves %s and bumps to new version', prefix => {
@@ -46,7 +46,7 @@ describe('bumpMinSemverRange', () => {
 
   it.each(['workspace:*', 'workspace:~', 'workspace:^'])('preserves %s', workspaceVersion => {
     const result = bumpMinSemverRange({ newVersion: '1.3.0', currentRange: workspaceVersion });
-    expect(result).toBe(workspaceVersion);
+    expect(result).toBe(true);
   });
 
   it('bumps workspace:~x.y.z to workspace range with new version', () => {
@@ -71,6 +71,6 @@ describe('bumpMinSemverRange', () => {
 
   it('preserves unrecognized range if new version satisfies it', () => {
     const result = bumpMinSemverRange({ newVersion: '1.3.0', currentRange: '1' });
-    expect(result).toBe('1');
+    expect(result).toBeUndefined();
   });
 });

packages/beachball/src/__tests__/bump/setDependentVersions.test.ts

@@ -4,7 +4,7 @@ import { makePackageInfos, type PartialPackageInfos } from '../../__fixtures__/p
 import { consideredDependencies } from '../../types/PackageInfo';
 import { initMockLogs } from '../../__fixtures__/mockLogs';
 
-type PartialBumpInfo = Parameters<typeof setDependentVersions>[0];
+type PartialBumpInfo = Parameters<typeof setDependentVersions>[0]['bumpInfo'];
 
 describe('setDependentVersions', () => {
   const logs = initMockLogs({ alsoLog: ['warn', 'error'] });
@@ -35,7 +35,7 @@ describe('setDependentVersions', () => {
       modifiedPackages: new Set(['pkg-a']),
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    const result = setDependentVersions({ bumpInfo, options: {} });
 
     expect(result).toEqual({});
   });
@@ -46,7 +46,7 @@ describe('setDependentVersions', () => {
       modifiedPackages: new Set(),
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    const result = setDependentVersions({ bumpInfo, options: {} });
 
     expect(result).toEqual({});
   });
@@ -61,7 +61,7 @@ describe('setDependentVersions', () => {
       },
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    const result = setDependentVersions({ bumpInfo, options: {} });
 
     expect(bumpInfo.packageInfos['pkg-b'].dependencies!['pkg-a']).toBe('^2.0.0');
     expect(result).toEqual({ 'pkg-b': new Set(['pkg-a']) });
@@ -78,7 +78,7 @@ describe('setDependentVersions', () => {
       },
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    const result = setDependentVersions({ bumpInfo, options: {} });
 
     expect(bumpInfo.packageInfos['pkg-b'].dependencies!['pkg-a']).toBe('^1.1.0');
     expect(bumpInfo.packageInfos['pkg-c'].dependencies!['pkg-a']).toBe('~1.1.0');
@@ -93,7 +93,7 @@ describe('setDependentVersions', () => {
       },
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    const result = setDependentVersions({ bumpInfo, options: {} });
 
     expect(bumpInfo.packageInfos['pkg-b'][depType]!['pkg-a']).toBe('^1.1.0');
     expect(result).toEqual({ 'pkg-b': new Set(['pkg-a']) });
@@ -109,7 +109,7 @@ describe('setDependentVersions', () => {
       },
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    const result = setDependentVersions({ bumpInfo, options: {} });
 
     expect(bumpInfo.packageInfos['pkg-a'].dependencies!['external']).toBe('^1.0.0');
     expect(result).toEqual({});
@@ -124,7 +124,7 @@ describe('setDependentVersions', () => {
       },
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    const result = setDependentVersions({ bumpInfo, options: {} });
 
     expect(bumpInfo.packageInfos['pkg-c'].dependencies!['pkg-a']).toBe('^1.0.1');
     expect(bumpInfo.packageInfos['pkg-c'].peerDependencies!['pkg-b']).toBe('^1.1.0');
@@ -139,44 +139,90 @@ describe('setDependentVersions', () => {
       },
     });
 
-    setDependentVersions(bumpInfo, { verbose: true });
+    setDependentVersions({ bumpInfo, options: { verbose: true } });
 
     expect(logs.mocks.log).toHaveBeenCalledWith('pkg-b needs to be bumped because pkg-a ^1.0.0 -> ^2.0.0');
   });
 
-  // Documenting this issue
   // https://github.com/microsoft/beachball/issues/981
-  it('currently misses bumps of workspace: ranges', () => {
+  it.each(['workspace:*', 'workspace:^', 'workspace:~'])(
+    'records dependent bumps for %s ranges (range string unchanged)',
+    workspaceVersion => {
+      const bumpInfo = makeBumpInfo({
+        packageInfos: {
+          'pkg-a': { version: '1.1.0' },
+          'pkg-b': { version: '1.0.1', dependencies: { 'pkg-a': workspaceVersion } },
+        },
+      });
+
+      const result = setDependentVersions({ bumpInfo, options: {} });
+
+      expect(bumpInfo.packageInfos['pkg-b'].dependencies!['pkg-a']).toBe(workspaceVersion);
+      expect(result).toEqual({ 'pkg-b': new Set(['pkg-a']) });
+    }
+  );
+
+  // https://github.com/microsoft/beachball/issues/981
+  it.each(['catalog:', 'catalog:foo'])(
+    'records dependent bumps for %s ranges (range string unchanged)',
+    catalogVersion => {
+      const bumpInfo = makeBumpInfo({
+        packageInfos: {
+          'pkg-a': { version: '1.1.0' },
+          'pkg-b': { version: '1.0.1', dependencies: { 'pkg-a': catalogVersion } },
+        },
+      });
+
+      const result = setDependentVersions({ bumpInfo, options: {} });
+
+      expect(bumpInfo.packageInfos['pkg-b'].dependencies!['pkg-a']).toBe(catalogVersion);
+      expect(result).toEqual({ 'pkg-b': new Set(['pkg-a']) });
+    }
+  );
+
+  // file: deps are intentionally excluded — see #1080
+  it('does not record dependent bumps for file: ranges', () => {
+    const bumpInfo = makeBumpInfo({
+      packageInfos: {
+        'pkg-a': { version: '1.1.0' },
+        'pkg-b': { version: '1.0.1', dependencies: { 'pkg-a': 'file:../pkg-a' } },
+      },
+    });
+
+    const result = setDependentVersions({ bumpInfo, options: {} });
+
+    expect(bumpInfo.packageInfos['pkg-b'].dependencies!['pkg-a']).toBe('file:../pkg-a');
+    expect(result).toEqual({});
+  });
+
+  it('does not record dependent bumps for workspace:/catalog: with skipImplicitBumps: true', () => {
     const bumpInfo = makeBumpInfo({
       packageInfos: {
         'pkg-a': { version: '1.1.0' },
-        // * means an exact dependency and should definitely cause a bump
         'pkg-b': { version: '1.0.1', dependencies: { 'pkg-a': 'workspace:*' } },
+        'pkg-c': { version: '1.0.1', dependencies: { 'pkg-a': 'catalog:foo' } },
       },
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    const result = setDependentVersions({ bumpInfo, options: {}, skipImplicitBumps: true });
 
     expect(bumpInfo.packageInfos['pkg-b'].dependencies!['pkg-a']).toBe('workspace:*');
-    expect(result).toEqual({}); // should have pkg-b depending on pkg-a
+    expect(bumpInfo.packageInfos['pkg-c'].dependencies!['pkg-a']).toBe('catalog:foo');
+    expect(result).toEqual({});
   });
 
-  // Documenting this issue
-  // https://github.com/microsoft/beachball/issues/981
-  it('currently misses bumps of catalog: ranges', () => {
+  it('logs a different verbose message when the range is unchanged', () => {
     const bumpInfo = makeBumpInfo({
       packageInfos: {
         'pkg-a': { version: '1.1.0' },
-        // Assume there's a catalog like this:
-        // catalog:
-        //   pkg-a: ^1.1.0
-        'pkg-b': { version: '1.0.1', dependencies: { 'pkg-a': 'catalog:' } },
+        'pkg-b': { version: '1.0.1', dependencies: { 'pkg-a': 'workspace:*' } },
       },
     });
 
-    const result = setDependentVersions(bumpInfo, {});
+    setDependentVersions({ bumpInfo, options: { verbose: true } });
 
-    expect(bumpInfo.packageInfos['pkg-b'].dependencies!['pkg-a']).toBe('catalog:');
-    expect(result).toEqual({}); // should have pkg-b depending on pkg-a
+    expect(logs.mocks.log).toHaveBeenCalledWith(
+      'pkg-b needs a changelog entry because pkg-a was bumped to 1.1.0 (range workspace:* unchanged)'
+    );
   });
 });

packages/beachball/src/__tests__/changelog/__snapshots__/renderChangelog.test.ts.snap

@@ -143,6 +143,34 @@ Thu, 22 Aug 2019 21:20:40 GMT
 "
 `;
 
+exports[`renderChangelog renders grouped changelog 1`] = `
+"# Change Log - group
+
+<!-- This log was last generated on Thu, 22 Aug 2019 21:20:40 GMT and should not be manually modified. -->
+
+<!-- Start content -->
+
+## 1.2.3
+
+Thu, 22 Aug 2019 21:20:40 GMT
+
+### Minor changes
+
+- \`bar\`
+  - Awesome change (user1@example.com)
+- \`foo\`
+  - Boring change (user2@example.com)
+
+### Patches
+
+- \`baz\`
+  - Fix (user1@example.com)
+  - stuff (a@example.com)
+- \`foo\`
+  - stuff (user2@example.com)
+"
+`;
+
 exports[`renderChangelog trims previous versions if over maxVersions 1`] = `
 "# Change Log - foo
 

packages/beachball/src/__tests__/changelog/renderChangelog.test.ts

@@ -85,6 +85,18 @@ describe('renderChangelog', () => {
     expect(result).toMatchSnapshot();
   });
 
+  it('renders grouped changelog', async () => {
+    const options = getOptions();
+    options.previousContent = '';
+    options.isGrouped = true;
+    options.newVersionChangelog.name = 'group';
+    // Distribute entries across packages so the grouped renderer has something to group
+    options.newVersionChangelog.comments.minor![0].package = 'bar';
+    options.newVersionChangelog.comments.patch![0].package = 'baz';
+    options.newVersionChangelog.comments.patch!.push({ package: 'baz', comment: 'stuff', author: 'a@example.com' });
+    expect(await renderChangelog(options)).toMatchSnapshot();
+  });
+
   it('trims previous versions if over maxVersions', async () => {
     const options = getOptions();
     options.previousContent += '\n\n' + changelogFromVersions(['1.1.9', '1.1.8', '1.1.7']);

packages/beachball/src/bump/bumpInMemory.ts

@@ -70,7 +70,7 @@ export function bumpInMemory(options: BeachballOptions, context: Omit<CommandCon
   }
 
   // step 5: Bump all the dependency version ranges and collect dependentChangedBy for the changelog.
-  const dependentChangedBy = setDependentVersions(bumpInfo, options);
+  const dependentChangedBy = setDependentVersions({ bumpInfo, options });
   // For now, add any modifiedPackages not previously detected (due to bumpDeps: false or scopes).
   // TODO: Rethink all of this... https://github.com/microsoft/beachball/issues/1123
   Object.keys(dependentChangedBy).forEach(pkg => bumpInfo.modifiedPackages.add(pkg));