fix changeset parser fail when summary is empty #740
Conversation
🦋 Changeset detectedLatest commit: 2ff6345 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ba5dcff:
|
packages/parse/src/index.test.ts
Outdated
| @@ -148,6 +148,17 @@ describe("parsing a changeset", () => { | |||
| summary: "" | |||
| }); | |||
| }); | |||
| it("should be fine if the changeset only contains frontmatter", () => { | |||
There was a problem hiding this comment.
This is a somewhat different test case that was originally reported. The reported case was about an empty summary but it had some releases declared in the frontmatter. I believe that it's worth adding tests for both of those cases.
packages/parse/src/index.test.ts
Outdated
| --- | ||
| `; | ||
|
|
||
| const changeset = parse(changesetMd); |
There was a problem hiding this comment.
outdent removes a trailing newline by default and all but it feels a little bit implicit here. I would prefer if we could do this explicitly here:
| const changeset = parse(changesetMd); | |
| const changeset = parse('---\n---'); |
There was a problem hiding this comment.
I totally overlooked that you have outdent. Updated!
packages/parse/src/index.ts
Outdated
| @@ -1,7 +1,7 @@ | |||
| import yaml from "js-yaml"; | |||
| import { Release, VersionType } from "@changesets/types"; | |||
|
|
|||
| const mdRegex = /\s*---([^]*?)\n\s*---\s*\n([^]*)/; | |||
| const mdRegex = /\s*---([^]*?)\n\s*---(?:\s*\n([^]*))?$/; | |||
There was a problem hiding this comment.
This version of the regexp doesn't allow for a valid~ input like "---\n--- "
We can fix this with:
| const mdRegex = /\s*---([^]*?)\n\s*---(?:\s*\n([^]*))?$/; | |
| const mdRegex = /\s*---([^]*?)\n\s*---(\s*(?:\n|$)[^]*)/; |
By using this suggestion we can also revert the change in the summary declaration as this regexp doesn't use an optional capturing group so it will always end up using an empty string for a roughSummary match.
There was a problem hiding this comment.
Very nice catch! This is updated now. Also added a test for the case you mentioned
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Done! |
Fix changesets/action#138
@Andarist Could you do me a favor to upgrade
changeset-actionto use the latest parser when we merge this PR? Thank you so much!