chore: move all prettier formatting into individual packages #2478
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 6 Skipped Deployments
|
cprussin
force-pushed
the
cprussin/move-prettier-to-turbo
branch
from
585d7ab to
f32efec
Compare
keyvankhademi
approved these changes
Mar 12, 2025
ali-bahjati
requested changes
Mar 13, 2025
There was a problem hiding this comment.
I think still a couple of us would like to use pre-commit (for personal convenience) and you can just solve it by adding the command to the pre-commit.
On another note, it's difficult to review this PR in a single commit because it contains a lot of mechanical changes in addition to your changes. Would suggest putting the prettier changes in a separate commit.
jayantk
approved these changes
Mar 13, 2025
cprussin
force-pushed
the
cprussin/move-prettier-to-turbo
branch
4 times, most recently
from
a3a1a0c to
08bb7fa
Compare
cprussin
force-pushed
the
cprussin/move-prettier-to-turbo
branch
2 times, most recently
from
60d7968 to
f1bc986
Compare
This commit makes the following connected changes: - Removes prettier from `pre-commit` and instead moves it to be run by `turbo`, hooking into `test:format` and `fix:format` - Fixes some of the linting scripts where they were incorrectly configured for turbo (i.e. renames the scripts to `fix:lint` and `test:lint` as needed) - Upgrades prettier in all packgaes
This commit fixes formatting & linting issues caused by reconfiguring prettier & eslint in the prior commit
This commit cleans up and updates the turbo config so that: 1. Tests and fixes can run on the [root package](https://turbo.build/repo/docs/crafting-your-repository/configuring-tasks#registering-root-tasks), e.g. to format or lint any files that don't exist inside a workspace package 2. More config is shared and we don't have as much config scoped to individual packages 3. `pnpm install` automatically runs at the right times (i.e. you don't have to manually run an install ever)
This workflow is now redundant since everything pre-commit does is covered by more comprehensive CI jobs. Moving forward, pre-commit should be treated as a devex optimization; no checks should go into pre-commit alone.
cprussin
force-pushed
the
cprussin/move-prettier-to-turbo
branch
from
8907c56 to
a909544
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes a few related changes:
Stops using
pre-committo run prettier anywhere in the monorepo, instead we now run prettier from each package and trigger the runs using turborepo at the root.Upgrade the prettier version used in all packages that were previously relying on
pre-committo run prettier.Reformat files that need it due to the new prettier version.
Cleans up the turborepo config so that it now runs some tasks in the root package, and also automatically runs
pnpm installwhen needed.Sets up
pre-committo runTURBO_SCM_BASE="HEAD^1" pnpm turbo fix --affectedwhich triggers auto-fixes for any package that has changed in this commit, or any package depending on one that's changed in this commit. Note there are possible downsides of doing this, we'll have to keep an eye on these and maybe update in a follow up:We might want to consider removing TURBO_SCM_BASE which will make turbo compare against main instead of HEAD^1. In effect this would cause pre-commit to fix anything that has changed since main in the current branch, not just in the current commit. I didn't do this because it would cause runs to be even slower and I assume the expectation is for pre-commit to fixup things that matter just for your one commit, not the whole branch.
This script still might be quite slow -- especially if you are changing a lot of packages or a package that has a lot of dependencies -- as it's not filtering down the sub-tasks under fix to just the files that have changes. We can probably do something here to filter further, maybe there's some way to pass the filenames in through turbo or something? It's a bit tricky as you can make changes that cause things to need to be fixed in other files, so just running fixes on affected files isn't really actually reliable....
Finally, this PR removes pre-commit from CI entirely, as pre-commit is now entirely redundant of other more comprehensive CI checks, and pre-commit should only be used moving forward as a devex optimization.
Note this PR is structured in logically separated commits, so please look through them independently for an easy review.