fix(release): changelog renderer should prefer breaking change explan…

…ation text (#20798)

e2e/release/src/release.test.ts

@@ -120,6 +120,7 @@ describe('nx release', () => {
       dependencyRelationshipLogMatch.length !== 1
     ) {
       // From JamesHenry: explicit error to assist troubleshooting NXC-143
+      // Update: after seeing this error in the wild, it somehow seems to be not finding the dependency relationship sometimes
       throw new Error(
         `
 Error: Expected to find exactly one dependency relationship log line.
@@ -128,7 +129,10 @@ If you are seeing this message then you have been impacted by some currently und
 
 Please report the full nx release version command output below to the Nx team:
 
-${versionOutput}`
+${{
+  versionOutput,
+  pkg2Contents: readFile(`${pkg2}/package.json`),
+}}`
       );
     }
     expect(dependencyRelationshipLogMatch.length).toEqual(1);

packages/nx/changelog-renderer/index.spec.ts

@@ -539,4 +539,125 @@ describe('defaultChangelogRenderer()', () => {
       expect(markdown).toMatchInlineSnapshot(`""`);
     });
   });
+
+  describe('breaking changes', () => {
+    it('should work for breaking changes with just the ! and no explanation', async () => {
+      const breakingChangeCommitWithExplanation: GitCommit = {
+        // ! after the type, no BREAKING CHANGE: in the body
+        message: 'feat(WebSocketSubject)!: no longer extends `Subject`.',
+        shortHash: '54f2f6ed1',
+        author: {
+          name: 'James Henry',
+          email: '[email protected]',
+        },
+        body:
+          'M\tpackages/rxjs/src/internal/observable/dom/WebSocketSubject.ts\n' +
+          '"',
+        authors: [
+          {
+            name: 'James Henry',
+            email: '[email protected]',
+          },
+        ],
+        description: 'no longer extends `Subject`.',
+        type: 'feat',
+        scope: 'WebSocketSubject',
+        references: [{ value: '54f2f6ed1', type: 'hash' }],
+        isBreaking: true,
+        revertedHashes: [],
+        affectedFiles: [
+          'packages/rxjs/src/internal/observable/dom/WebSocketSubject.ts',
+        ],
+      };
+
+      const markdown = await defaultChangelogRenderer({
+        projectGraph,
+        commits: [breakingChangeCommitWithExplanation],
+        releaseVersion: 'v1.1.0',
+        project: null,
+        entryWhenNoChanges: false,
+        changelogRenderOptions: {
+          includeAuthors: true,
+        },
+      });
+
+      expect(markdown).toMatchInlineSnapshot(`
+        "## v1.1.0
+
+
+        ### 🚀 Features
+
+        - ⚠️  **WebSocketSubject:** no longer extends \`Subject\`.
+
+        #### ⚠️  Breaking Changes
+
+        - ⚠️  **WebSocketSubject:** no longer extends \`Subject\`.
+
+        ### ❤️  Thank You
+
+        - James Henry"
+      `);
+    });
+
+    it('should extract the explanation of a breaking change and render it preferentially', async () => {
+      const breakingChangeCommitWithExplanation: GitCommit = {
+        // No ! after the type, but BREAKING CHANGE: in the body
+        message: 'feat(WebSocketSubject): no longer extends `Subject`.',
+        shortHash: '54f2f6ed1',
+        author: {
+          name: 'James Henry',
+          email: '[email protected]',
+        },
+        body:
+          'BREAKING CHANGE: `WebSocketSubject` is no longer `instanceof Subject`. Check for `instanceof WebSocketSubject` instead.\n' +
+          '"\n' +
+          '\n' +
+          'M\tpackages/rxjs/src/internal/observable/dom/WebSocketSubject.ts\n' +
+          '"',
+        authors: [
+          {
+            name: 'James Henry',
+            email: '[email protected]',
+          },
+        ],
+        description: 'no longer extends `Subject`.',
+        type: 'feat',
+        scope: 'WebSocketSubject',
+        references: [{ value: '54f2f6ed1', type: 'hash' }],
+        isBreaking: true,
+        revertedHashes: [],
+        affectedFiles: [
+          'packages/rxjs/src/internal/observable/dom/WebSocketSubject.ts',
+        ],
+      };
+
+      const markdown = await defaultChangelogRenderer({
+        projectGraph,
+        commits: [breakingChangeCommitWithExplanation],
+        releaseVersion: 'v1.1.0',
+        project: null,
+        entryWhenNoChanges: false,
+        changelogRenderOptions: {
+          includeAuthors: true,
+        },
+      });
+
+      expect(markdown).toMatchInlineSnapshot(`
+        "## v1.1.0
+
+
+        ### 🚀 Features
+
+        - ⚠️  **WebSocketSubject:** no longer extends \`Subject\`.
+
+        #### ⚠️  Breaking Changes
+
+        - **WebSocketSubject:** \`WebSocketSubject\` is no longer \`instanceof Subject\`. Check for \`instanceof WebSocketSubject\` instead.
+
+        ### ❤️  Thank You
+
+        - James Henry"
+      `);
+    });
+  });
 });

packages/nx/changelog-renderer/index.ts

@@ -143,7 +143,16 @@ const defaultChangelogRenderer: ChangelogRenderer = async ({
           const line = formatCommit(commit, repoSlug);
           markdownLines.push(line);
           if (commit.isBreaking) {
-            breakingChanges.push(line);
+            const breakingChangeExplanation = extractBreakingChangeExplanation(
+              commit.body
+            );
+            breakingChanges.push(
+              breakingChangeExplanation
+                ? `- ${
+                    commit.scope ? `**${commit.scope.trim()}:** ` : ''
+                  }${breakingChangeExplanation}`
+                : line
+            );
           }
         }
       }
@@ -188,7 +197,16 @@ const defaultChangelogRenderer: ChangelogRenderer = async ({
         const line = formatCommit(commit, repoSlug);
         markdownLines.push(line + '\n');
         if (commit.isBreaking) {
-          breakingChanges.push(line);
+          const breakingChangeExplanation = extractBreakingChangeExplanation(
+            commit.body
+          );
+          breakingChanges.push(
+            breakingChangeExplanation
+              ? `- ${
+                  commit.scope ? `**${commit.scope.trim()}:** ` : ''
+                }${breakingChangeExplanation}`
+              : line
+          );
         }
       }
     }
@@ -294,11 +312,37 @@ function groupBy(items: any[], key: string) {
 function formatCommit(commit: GitCommit, repoSlug?: RepoSlug): string {
   let commitLine =
     '- ' +
-    (commit.scope ? `**${commit.scope.trim()}:** ` : '') +
     (commit.isBreaking ? '⚠️  ' : '') +
+    (commit.scope ? `**${commit.scope.trim()}:** ` : '') +
     commit.description;
   if (repoSlug) {
     commitLine += formatReferences(commit.references, repoSlug);
   }
   return commitLine;
 }
+
+/**
+ * It is common to add further information about a breaking change in the commit body,
+ * and it is naturally that information that should be included in the BREAKING CHANGES
+ * section of changelog, rather than repeating the commit title/description.
+ */
+function extractBreakingChangeExplanation(message: string): string | null {
+  const breakingChangeIdentifier = 'BREAKING CHANGE:';
+  const startIndex = message.indexOf(breakingChangeIdentifier);
+
+  if (startIndex === -1) {
+    // "BREAKING CHANGE:" not found in the message
+    return null;
+  }
+
+  const startOfBreakingChange = startIndex + breakingChangeIdentifier.length;
+  const endOfBreakingChange = message.indexOf('\n', startOfBreakingChange);
+
+  if (endOfBreakingChange === -1) {
+    // No newline character found, extract till the end of the message
+    return message.substring(startOfBreakingChange).trim();
+  }
+
+  // Extract and return the breaking change message
+  return message.substring(startOfBreakingChange, endOfBreakingChange).trim();
+}

packages/nx/src/command-line/release/utils/git.ts

@@ -285,7 +285,8 @@ export function parseGitCommit(commit: RawGitCommit): GitCommit | null {
 
   const scope = match.groups.scope || '';
 
-  const isBreaking = Boolean(match.groups.breaking);
+  const isBreaking =
+    Boolean(match.groups.breaking) || commit.body.includes('BREAKING CHANGE:');
   let description = match.groups.description;
 
   // Extract references from message