Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHANGELOG.md change lists unwanted formatting. #1283

Open
loucyx opened this issue Dec 13, 2023 · 6 comments
Open

CHANGELOG.md change lists unwanted formatting. #1283

loucyx opened this issue Dec 13, 2023 · 6 comments

Comments

@loucyx
Copy link

loucyx commented Dec 13, 2023

Affected Packages

I believe the issue is in changeset's CLI monorepo.

Problem

Previously, when I ran pnpm changeset version the changes applied to
CHANGELOG.md were only adding a new entry to the list of versions, but a few
days ago I started noticing that the formatting for the entire CHANGELOG.md
file was changing.

To reproduce it, just have a project with a prettier formated CHANGELOG.md, in
which lists use - and are followed by 3 spaces:

-   like this

And you'll see that after running pnpm changeset and then
pnpm changeset version the spacing for list items is changed to:

- a single space 🥺

This seems to be happening even after custom changelog scripts. I have a
custom changelog.cjs script that has the formatting I want for the new list
items, but this formatting is overriding that.

Code of my changelog.cjs because maybe is relevant for y'all
/**
 * Changesets custom changelog functions.
 * @see [Changeset documentation](https://github.com/changesets/changesets/blob/main/docs/modifying-changelog-format.md)
 */
// eslint-disable-next-line functional/immutable-data
module.exports = Object.freeze(
	/** @type {const} @satisfies {import("@changesets/types").ChangelogFunctions} */ ({
		getDependencyReleaseLine: (_changesets, dependenciesUpdated) =>
			Promise.resolve(
				dependenciesUpdated.length === 0 ?
					""
				:	dependenciesUpdated
						.map(
							({ name, newVersion }) =>
								`-   ⬆️ upgrade \`${name}\` to \`v${newVersion}\``,
						)
						.join("\n"),
			),
		getReleaseLine: ({ summary }) =>
			Promise.resolve(
				summary
					.split("\n")
					.map(line =>
						line.startsWith("-") || line.startsWith("    ") ?
							line
						:	`-   ${line}`,
					)
					.join("\n"),
			),
	}),
);

So now after each pnpm changeset version I need to run prettier over
CHANGELOG.md, which isn't fun at all.

Proposed solution

Whatever changed in the way CHANGELOG.md is being parsed and then formatted
should be reverted, or if the idea is to format it, then at least it should
follow the local prettier configuration.

I'm willing to help with a PR if you can point me in the right direction to deal
with this issue.

mbeckem added a commit to open-pioneer/trails-openlayers-base-packages that referenced this issue Jan 5, 2024
mbeckem added a commit to open-pioneer/trails-core-packages that referenced this issue Jan 5, 2024
mbeckem added a commit to open-pioneer/trails-build-tools that referenced this issue Jan 5, 2024
@mbeckem
Copy link

mbeckem commented Jan 5, 2024

This seems to be a regression? See #20

@mbeckem
Copy link

mbeckem commented Jan 5, 2024

I've done some further digging: this seems to be a problem with recent versions of prettier (with prettier@3.1.1).

applyReleasePlan() uses prettierInstance.resolveConfig(cwd) (with the correct cwd) to resolve the prettier config. However prettier returns null from that call, even though a .prettierrc exists in cwd. This causes prettier to use its default formatting.

Debugging revealed that the search for the configuration file starts in dirname(cwd) instead and goes up from there. Prettier seems to interpret the input file name as an actual file and will search in that "files" parent directory, even though cwd is explicitly documented as a valid input.

Related: prettier/prettier#15879

@loucyx
Copy link
Author

loucyx commented Jan 5, 2024

Thanks for the explanation and for sharing the issue in Prettier's GitHub, @mbeckem! I'll subscribe to that thread so I can close this issue as soon as that one is completed.

@fisker
Copy link

fisker commented Jan 5, 2024

Hi, I'm a maintainer from Prettier, do you think it make sense to change it to resolve config with file path instead of process.cwd(), in theory user can have different options for different files, even if they are all .md files.

@AudunBeck
Copy link

Is there any update if its ok to upgrade prettier now?

mbeckem added a commit to open-pioneer/trails-core-packages that referenced this issue Feb 27, 2024
mbeckem added a commit to open-pioneer/trails-core-packages that referenced this issue Feb 27, 2024
mbeckem added a commit to open-pioneer/trails-core-packages that referenced this issue Feb 27, 2024
@steveluscher
Copy link

steveluscher commented Mar 6, 2024

Is this also the root cause of changes to .changeset/pre.json not being formatted with the local Prettier config after running pnpm changeset version?

mbeckem added a commit to open-pioneer/trails-core-packages that referenced this issue May 23, 2024
mbeckem added a commit to open-pioneer/trails-core-packages that referenced this issue May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants