chore: enabled Prettier in Trunk #19354

JoshuaKGoldberg · 2025-01-21T15:31:07Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain: internal tooling change

Fixes #17814 (part 1 of 2).

What changes did you make? (Give an overview)

Removes the disabling of prettier in Trunk and expands lint-staged so that roughly all files will be formatted with Prettier.

The .markdownlint.yml, docs/.stylelintrc.json, and eslint.config.js config files needed to be changed to exclude formatting rules that conflict with Prettier. This change additionally adds ignore entries to trunk.yaml to exclude:

the auto-generated changelog, as with markdownlint
hard-to-format _includes in docs
docs files for rules that conflict with Prettier
test fixtures

I chose to add "trailingCommas": "none" in the .prettierrc.json because it seems to be the existing style. The line changes excluding inline config comments in #19355 came out to:

"all" (default): 206012 insertions(+), 178431 deletions(-)
"none": 178480 insertions(+), 151397 deletions(-)

I personally prefer the default for consistency and to remove a config file, though, and would be very up for changing it if requested. 🙂

Is there anything you'd like reviewers to focus on?

CI failures are expected. This PR does not include the actual formatting of files with Prettier. That would make for a huge nigh-unreviewable diff. Keeping the formatting application to a separate PR & commit also makes it much easier to .git-blame-ignore-revs them away.

Subsequent formatting PR that includes these changes: #19355. If & when these two PRs are approved I would recommend merging this one first, then that one as a squash commit.

netlify · 2025-01-21T15:31:27Z

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	`de09734`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/679d43bb3e868b00085ef542

package.json

nzakas

Thanks for taking a look at this. Is your recommendation that this be merged first and then #19355?

And we should discuss what we want for Prettier options. We've started a new format in the rewrite repo, so maybe we want to stick with that here?

nzakas · 2025-02-03T17:38:03Z

package.json

@@ -72,8 +72,7 @@
    "pre-commit": "lint-staged"
  },
  "lint-staged": {
-    "*.js": "trunk check --fix --filter=eslint",
-    "*.md": "trunk check --fix --filter=markdownlint",
+    "*": "trunk check --fix",


This means every single file type will be run through Trunk, and I'm not sure we want that at this point.

Let's go slow and just stick with JS and Markdown. We can explore other file types later.

nzakas · 2025-02-03T17:46:56Z

Note there are linting errors.

JoshuaKGoldberg · 2025-02-05T13:42:28Z

Thanks for taking a look at this. Is your recommendation that this be merged first and then #19355?

Yes. The order isn't extremely important - in theory that one could be merged first, then this one have the .git-blame-ignore-revs file added to it before merge. IME it's nice to get the tooling (this) PR in first. That way if folks branch off main between the two PRs are merged, their branches will have the updated formatting hooks.

OTOH, there's an argument that merging the formatting PR in first is friendlier. It's more likely of a PR to have unexpected delays. Minimizing the time when main's formatting and tooling are out of sync is nice. 🤷 I don't have a strong preference or recommendation personally.

And we should discuss what we want for Prettier options. We've started a new format in the rewrite repo, so maybe we want to stick with that here?

👍 That'd be my vote: going with the defaults unless directed otherwise:

Not having to include a .prettierrc file is a nice little simplification
I think they're generally good defaults
The Prettier defaults have become the most common settings amongst web repositories by virtue of being the default of the most popular dedicated formatter

Note there are linting errors.

Just confirming: are you referring to the Verify job failure from Trunk? https://github.com/eslint/eslint/actions/runs/13081264060/job/36505118543:

.trunk/trunk.yaml
 -:-  fmt  Incorrect formatting, autoformat by running 'trunk fmt'  prettier

eslint.config.js
 -:-  fmt  Incorrect formatting, autoformat by running 'trunk fmt'  prettier

...

If so: 👍 this is expected. Trunk is applying formatting concerns as configured in this PR but the files haven't been formatted yet.

npx eslint . locally produces no lint reports for me.

nzakas · 2025-02-05T17:26:51Z

Yes, that's the failure I'm referring to. That's part of why I was asking about the order of merging these PRs, as we don't want to merge something that will cause a CI failure.

The Prettier defaults don't work well in all situations. Here's what we have in the new repos:
https://github.com/eslint/json/blob/main/prettier.config.js

@eslint/eslint-tsc do we want to go with that in this repo too? My vote is 👍

JoshuaKGoldberg · 2025-02-05T19:53:48Z

If we stick with two PRs then I don't think there's a way to avoid CI failures. This PR fails CI because it changes formatting & linting settings without changes code. The other PR fails because it changes code without changing formatting & linting settings.

We could avoid a failure by sticking to one PR. We'd have to merge it carefully and not squash the commits.

Alternately, we could go with three PRs:

Remove all linter checks that interfere with formatting/Prettier (part of this PR)
Format code (the other PR)
Enable Prettier in Trunk (the rest of this PR)

fasttime · 2025-02-12T12:22:23Z

Yes, that's the failure I'm referring to. That's part of why I was asking about the order of merging these PRs, as we don't want to merge something that will cause a CI failure.

The Prettier defaults don't work well in all situations. Here's what we have in the new repos: https://github.com/eslint/json/blob/main/prettier.config.js

@eslint/eslint-tsc do we want to go with that in this repo too? My vote is 👍

No objections to enforcing that files are formatted with Prettier in this repo from my end, and I think that using the same settings as in other repos is a good idea.

We'll end up with a mess of merge conflicts in other pull requests but that's a temporary effect.

nzakas · 2025-02-12T16:39:25Z

Ping @mdjermanovic

mdjermanovic · 2025-02-14T09:38:33Z

I agree that we use the same Prettier settings as in other repos.

eslint-github-bot bot added the chore This change is not user-facing label Jan 21, 2025

github-actions bot added the github actions label Jan 21, 2025

JoshuaKGoldberg mentioned this pull request Jan 21, 2025

chore: formatted files with Prettier via trunk fmt #19355

Open

1 task

JoshuaKGoldberg force-pushed the prettier branch from 13ed57d to efa7e1a Compare January 21, 2025 15:59

harish-sethuraman reviewed Jan 21, 2025

View reviewed changes

package.json Outdated Show resolved Hide resolved

harish-sethuraman reviewed Jan 21, 2025

View reviewed changes

package.json Outdated Show resolved Hide resolved

JoshuaKGoldberg force-pushed the prettier branch from 03e0e56 to 4dd2dc7 Compare January 21, 2025 16:50

github-actions bot removed the github actions label Jan 21, 2025

JoshuaKGoldberg force-pushed the prettier branch from c77c923 to 1d067f1 Compare January 31, 2025 18:54

JoshuaKGoldberg changed the title ~~chore: add Prettier format script~~ chore: enabled Prettier in Trunk Jan 31, 2025

JoshuaKGoldberg force-pushed the prettier branch 4 times, most recently from 9f0ed89 to 2fd4fc5 Compare January 31, 2025 21:30

chore: enable Prettier in Trunk

de09734

JoshuaKGoldberg force-pushed the prettier branch from 2fd4fc5 to de09734 Compare January 31, 2025 21:42

JoshuaKGoldberg marked this pull request as ready for review January 31, 2025 21:53

JoshuaKGoldberg requested a review from a team as a code owner January 31, 2025 21:54

JoshuaKGoldberg mentioned this pull request Jan 31, 2025

Change Request: Consider adopting prettier internally #17814

Open

1 task

nzakas reviewed Feb 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: enabled Prettier in Trunk #19354

chore: enabled Prettier in Trunk #19354

JoshuaKGoldberg commented Jan 21, 2025 •

edited

Loading

netlify bot commented Jan 21, 2025 •

edited

Loading

nzakas left a comment

nzakas Feb 3, 2025

nzakas commented Feb 3, 2025

JoshuaKGoldberg commented Feb 5, 2025 •

edited

Loading

nzakas commented Feb 5, 2025

JoshuaKGoldberg commented Feb 5, 2025

fasttime commented Feb 12, 2025

nzakas commented Feb 12, 2025

mdjermanovic commented Feb 14, 2025

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!

chore: enabled Prettier in Trunk #19354

Are you sure you want to change the base?

chore: enabled Prettier in Trunk #19354

Conversation

JoshuaKGoldberg commented Jan 21, 2025 • edited Loading

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Jan 21, 2025 • edited Loading

✅ Deploy Preview for docs-eslint canceled.

nzakas left a comment

Choose a reason for hiding this comment

nzakas Feb 3, 2025

Choose a reason for hiding this comment

nzakas commented Feb 3, 2025

JoshuaKGoldberg commented Feb 5, 2025 • edited Loading

nzakas commented Feb 5, 2025

JoshuaKGoldberg commented Feb 5, 2025

fasttime commented Feb 12, 2025

nzakas commented Feb 12, 2025

mdjermanovic commented Feb 14, 2025

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!

JoshuaKGoldberg commented Jan 21, 2025 •

edited

Loading

netlify bot commented Jan 21, 2025 •

edited

Loading

JoshuaKGoldberg commented Feb 5, 2025 •

edited

Loading