Skip to content

Bug: splitMessages function fails to handle multiple conventional commits #2357

@dgokcin

Description

@dgokcin

Thank you to the creators of release-please for your hard work and dedication to the project.

I wanted to bring to your attention an issue with the current implementation of the parsing logic in src/commit.ts. It appears that the logic does not handle multiple conventional commits within a single squash merge correctly. The implementation seems to rely heavily on specific formatting and structure that does not fully align with the Conventional Commits specification.

For example, problems arise when commit messages include bullet points, empty lines, or varying indentations, as demonstrated in the experiment examples provided. Additionally, the documentation does not mention the requirement for body messages to be indented by two spaces, as seen in some examples in the repository's README. This discrepancy suggests that the current logic may not fully adhere to the specification, which the repository aims to support.

Here is the conventional commit format which I believe the examples below all fit in.

<type>[optional scope]: <description>

[optional body]

[optional footer(s)]

Here are some examples from the experiments I made

Slightly modified example of the example in the README - Parsed correctly, however there needs to be a new line between body and title
feat: adds v4 UUID to crypto

This adds support for v4 UUIDs to the library.

fix(utils): unicode no longer throws exception
- some more stuff

feat(utils): update encode to support unicode
- does stuff
- more stuff
Same example except this time with bullets are at the body (Parsed Incorrectly)
feat: adds v4 UUID to crypto

This adds support for v4 UUIDs to the library.

fix(utils): unicode no longer throws exception

- some more stuff

feat(utils): update encode to support unicode

- does stuff
- more stuff
Same example with an empty line between the description and the body (Parsed Incorrectly - only the first commit is treated as a conventional commit)
feat: adds v4 UUID to crypto

This adds support for v4 UUIDs to the library.

fix(utils): unicode no longer throws exception

- some more stuff

feat(utils): update encode to support unicode

- does stuff
- more stuff
Multiple commits with a simple body (Parsed Incorrectly - only the first commit is treated as a conventional commit)
feat: adds v4 UUID to crypto

This adds support for v4 UUIDs to the library.

fix(utils): unicode no longer throws exception

some more stuff

feat(utils): update encode to support unicode

does stuff
more stuff

Additional Details

  • I found out that the BEGIN_NESTED_COMMIT and END_NESTED_COMMIT could be used to customize the nested commits, however I think it would be awesome if release please can handle these automatically.
  • I feel like the splitMessages function could be modified to support the aforementioned commits and improve the parsing. A simple change like below supports the commit types I mentioned above.(It brakes only 5 tests)
function splitMessages(message: string): string[] {
  const parts = message.split('BEGIN_NESTED_COMMIT');
  const messages = [parts.shift()!];
  for (const part of parts) {
    const [newMessage, ...rest] = part.split('END_NESTED_COMMIT');
    messages.push(newMessage);
    messages[0] = messages[0] + rest.join('END_NESTED_COMMIT');
  }

  // Split the first message into conventional commits
  const conventionalCommits = messages[0].split(/\n(?=(?:feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.*?\))?: )/);

  return [...conventionalCommits, ...messages.slice(1)];
}

Environment details

  • OS: macos sonoma 14.5
  • Node.js version: v22.2.0
  • npm version: 10.7.0
  • release-please version: 16.12.0

Steps to reproduce

  1. Create a new txt file from the examples above in the test/fixtures/commit-messages/ directory
  2. Create a new test in test/commits.ts with the following content
  it('test custom commits', async () => {
  const commits = [buildCommitFromFixture('name-of-the-fixture')];
  const conventionalCommits = parseConventionalCommits(commits);
  console.log('Conventional Commits:');
  console.log(JSON.stringify(conventionalCommits, null, 2));
  expect(conventionalCommits).lengthOf(1);
  expect(conventionalCommits[0].breaking).to.be.true;
  expect(conventionalCommits[0].message).not.include('I should be removed');
});

3. Run `npm test` > out.txt
5. Search the `out.txt` file for "Conventional Commits:" to inspect the conventionalCommits array.

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions