chore: enabled Prettier in Trunk by JoshuaKGoldberg · Pull Request #19354 · eslint/eslint

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. 🙂~~

Per #19354 (comment) & #19354 (comment), uses the same Prettier config as other repos: copied from https://github.com/eslint/json/blob/c4f4abb5c4699e89b76eb478415c4e3b71fa6675/prettier.config.js.

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.

The intended merge order of PRs to avoid main branch CI failures is:

chore: remove formatting-related lint rules internally #19473: to remove any CI check that would fail on the upcoming new formatting
chore: formatted files with Prettier via trunk fmt #19355: applies new formatting rules, and should be squash-merged to make a single commit for .git-blame-ignore-revs
👉 chore: enabled Prettier in Trunk #19354 (this one): adds Prettier checks in CI and as commit hooks via Trunk

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

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	`7d5f408`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/67dae1ea2bad5600089ce23f
😎 Deploy Preview	https://deploy-preview-19354--docs-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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: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.

github-actions · 2025-02-24T22:33:36Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

JoshuaKGoldberg · 2025-03-10T17:51:23Z

👍 split up. Noted in #19473 (comment):

I believe the three PRs are split up so that merging them sequentially won't cause failures in main. All three OPs are updated to indicate the order.

JoshuaKGoldberg · 2025-03-19T15:33:54Z

Sigh artifact from my manual touch-ups in #19355.

nzakas

LGTM.

harish-sethuraman · 2025-03-19T18:12:21Z

-    "*.md": "trunk check --fix --filter=markdownlint",
+    "*": "trunk check --fix",
    "lib/rules/*.js": [
      "node tools/update-eslint-all.js",


Sorry for coming here so late. Have we checked this #19354 (comment)?

node tools/update-eslint-all.js creates a new file which will be run by trunk check --fix. Which runs first? is it the script or format?

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

Merged

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

Comment thread package.json Outdated

harish-sethuraman reviewed Jan 21, 2025

View reviewed changes

Comment thread package.json Outdated

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

Closed

1 task

nzakas reviewed Feb 3, 2025

View reviewed changes

Comment thread package.json

github-actions Bot added the Stale label Feb 24, 2025

Merge branch 'main'

845c182

github-actions Bot added rule Relates to ESLint's core rules cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features formatter Relates to the formatters bundled with ESLint github actions labels Mar 10, 2025

JoshuaKGoldberg force-pushed the prettier branch 2 times, most recently from d7be74c to de09734 Compare March 10, 2025 13:25

github-actions Bot removed rule Relates to ESLint's core rules cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features formatter Relates to the formatters bundled with ESLint github actions labels Mar 10, 2025

JoshuaKGoldberg added 2 commits March 10, 2025 09:30

Switch .prettierrc.json to organization standard

facd31c

Merge branch 'main'

20704f7

JoshuaKGoldberg marked this pull request as ready for review March 10, 2025 17:48

Merge branch 'main'

b97a7bd

JoshuaKGoldberg commented Mar 12, 2025

View reviewed changes

Comment thread .prettierrc.json

JoshuaKGoldberg mentioned this pull request Mar 19, 2025

chore: temporarily disable prettier in trunk #19537

Merged

JoshuaKGoldberg added 2 commits March 19, 2025 11:23

Merge branch 'main' into prettier

1c8b2cf

Merge branch 'main'

7d5f408

JoshuaKGoldberg commented Mar 19, 2025

View reviewed changes

Comment thread conf/replacements.json

Copy link
Copy Markdown

Contributor Author

JoshuaKGoldberg Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh artifact from my manual touch-ups in #19355.

JoshuaKGoldberg requested a review from nzakas March 19, 2025 15:33

nzakas approved these changes Mar 19, 2025

View reviewed changes

nzakas merged commit 75adc99 into eslint:main Mar 19, 2025
31 checks passed

github-project-automation Bot moved this from Implementing to Complete in Triage Mar 19, 2025

JoshuaKGoldberg deleted the prettier branch March 19, 2025 15:51

harish-sethuraman reviewed Mar 19, 2025

View reviewed changes

fasttime mentioned this pull request Mar 19, 2025

chore: format JSON files in Trunk #19541

Merged

1 task

Uh oh!

Conversation

JoshuaKGoldberg commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

netlify Bot commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for docs-eslint ready!

Uh oh!

Uh oh!

Uh oh!

nzakas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nzakas commented Feb 3, 2025

Uh oh!

JoshuaKGoldberg commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nzakas commented Feb 5, 2025

Uh oh!

JoshuaKGoldberg commented Feb 5, 2025

Uh oh!

fasttime commented Feb 12, 2025

Uh oh!

nzakas commented Feb 12, 2025

Uh oh!

mdjermanovic commented Feb 14, 2025

Uh oh!

github-actions Bot commented Feb 24, 2025

Uh oh!

JoshuaKGoldberg commented Mar 10, 2025

Uh oh!

Uh oh!

JoshuaKGoldberg Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

nzakas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

harish-sethuraman Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JoshuaKGoldberg commented Jan 21, 2025 •

edited

Loading

netlify Bot commented Jan 21, 2025 •

edited

Loading

JoshuaKGoldberg commented Feb 5, 2025 •

edited

Loading