5 ms·

I don’t really get it. Prettier isn’t something that you need to run that often on the entire repo; you’re probably just running it on save in a handful of mill

by lakpan 3y ago

I don’t really get it. Prettier isn’t something that you need to run that often on the entire repo; you’re probably just running it on save in a handful of milliseconds or on a pre-commit on hopefully less than 10 files. There are plenty other tools that often run on the entire repo, like tests, lint, build, type-checking, etc. focus on those. Bun showed that there’s a lot of space for improvement.

biorach 3y ago
Many organisations have a CI step that runs a linter or formatter on all the code and fails if the tool makes changes. The idea is to catch cases where contributors failed to run the tool on save. On large code bases this can be time consuming.
- zegl 3y ago
  The trick to make CI in large codebases fast, or how to make any system fast, is to only test what’s changed. In CI you only need to lint the files that have changed, or run the tests that depend on code that has changed etc. This way the time it takes to execute the tests scales with the amount of changes, and not with the total amount of code.
  - DougBTX 3y ago
    The difficulty with that is trust, if there’s any chance it will miss things by accident then I’d rather eat the slow runs. Test runners that run new/changed tests first though are cool. Faster feedback on error but still comprehensive.
sophiabits 3y ago
Adding on to the other comment about CI setups, when I benchmarked removing eslint-plugin-prettier from my work’s ESLint configuration we saw a 37.5% speedup. On our fairly slow CI agents that ended up being pretty significant (~34s per run). I _wanted_ to drop the plugin and have Prettier automatically run on as a pre-commit hook (as you suggested) but in the end I lost :) We wound up keeping the plugin and eating the slower pipeline time. Of course there’s more overhead than just Prettier in this case—the ESLint plugin itself will be contributing towards the added runtime!—but depending on your codebase and CI machines, Prettier can definitely slow things down quite a bit. CI taking over a minute or two is a big drag on developer productivity imo. Ideal world is that everyone is on board with code formatting as a pre-commit task that only touches modified files so it doesn’t bloat CI unnecessarily, but it’s not always possible :(
- liamfd 3y ago
  What we did to avoid that bottleneck was just turn off the prettier eslint rule in CI and just run the `prettier --check` command they recommend in their docs. We also have it set to only run in changed files which helps a lot. We generally don't link pre-commit hooks for standards though, hence the CI focus. Too easy to circumvent, and I'd rather pay for an external machine to check it when it matters (pre-merge by doing it on PR commits) than block my devs when it doesn't (every time they commit to a non-main branch).
- a-ungurianu 3y ago
  I’ve done some research on this in the past. The reason that is so slow is because the eslint plugin runs prettier, does a diff between the result of prettier and the real file and then converts each diff entry into a eslint issues. Most of the pain comes from this remarshaling cost
- cpojer 3y ago
  I ran into the same issue and have been separating the prettier and eslint steps ever since: https://cpojer.net/posts/fastest-frontend-tooling-in-2022#eslint--prettier https://cpojer.net/posts/fastest-frontend-tooling-in-2022#es...
tuananh 3y ago
it's very common to have a lint/format step that make sure all files are well-formatted in Pull request.
- crabmusket 3y ago
  This is such a waste of electricity IMO. Just occasionally batch-fix all files for syntax. Semantic linting can run in CI, as it's about catching bugs. But formatting is for long-term consistency and doesn't need to be done on every commit.
  - yawboakye 3y ago
    you may have to force approve edits by the formatter then, since they’re pretty annoying when included in changes for code review.
  - dbrgn 3y ago
    Occasionally batch-fixing syntax makes "git blame" far less useful.
    - crabmusket 3y ago
      https://akrabat.com/ignoring-revisions-with-git-blame/ https://akrabat.com/ignoring-revisions-with-git-blame/
      - dbrgn 3y ago
        Yup, but you need to reference the commit ID, meaning that you always need to create two commits, one for the reformatting, and a second one for updating the .git-blame-ignore-revs file. Additionally, you may not use git rebase, or any other workflow that modifies the history, or the references will be broken. Third, the git config needs to be set per-project (can't be set globally), so all developers need to manually adjust their config, for this feature to be useful. If you value a clean commit history, then simply reformat as part of the code change. With autoformatting on save (e.g. through prettier), that's a complete non-issue anyways.
the_other 3y ago
Or, better still, just remove prettier and free yourself to write more expressive code again.
volent 3y ago
I believe you don't see the big picture. What you just described is running on thousands of machines several times a day.

by lakpan 3y ago

biorach 3y ago

Many organisations have a CI step that runs a linter or formatter on all the code and fails if the tool makes changes. The idea is to catch cases where contributors failed to run the tool on save. On large code bases this can be time consuming.

zegl 3y ago
The trick to make CI in large codebases fast, or how to make any system fast, is to only test what’s changed. In CI you only need to lint the files that have changed, or run the tests that depend on code that has changed etc. This way the time it takes to execute the tests scales with the amount of changes, and not with the total amount of code.
- DougBTX 3y ago
  The difficulty with that is trust, if there’s any chance it will miss things by accident then I’d rather eat the slow runs. Test runners that run new/changed tests first though are cool. Faster feedback on error but still comprehensive.

sophiabits 3y ago

Adding on to the other comment about CI setups, when I benchmarked removing eslint-plugin-prettier from my work’s ESLint configuration we saw a 37.5% speedup. On our fairly slow CI agents that ended up being pretty significant (~34s per run). I _wanted_ to drop the plugin and have Prettier automatically run on as a pre-commit hook (as you suggested) but in the end I lost :) We wound up keeping the plugin and eating the slower pipeline time. Of course there’s more overhead than just Prettier in this case—the ESLint plugin itself will be contributing towards the added runtime!—but depending on your codebase and CI machines, Prettier can definitely slow things down quite a bit. CI taking over a minute or two is a big drag on developer productivity imo. Ideal world is that everyone is on board with code formatting as a pre-commit task that only touches modified files so it doesn’t bloat CI unnecessarily, but it’s not always possible :(

liamfd 3y ago
What we did to avoid that bottleneck was just turn off the prettier eslint rule in CI and just run the `prettier --check` command they recommend in their docs. We also have it set to only run in changed files which helps a lot. We generally don't link pre-commit hooks for standards though, hence the CI focus. Too easy to circumvent, and I'd rather pay for an external machine to check it when it matters (pre-merge by doing it on PR commits) than block my devs when it doesn't (every time they commit to a non-main branch).
a-ungurianu 3y ago
I’ve done some research on this in the past. The reason that is so slow is because the eslint plugin runs prettier, does a diff between the result of prettier and the real file and then converts each diff entry into a eslint issues. Most of the pain comes from this remarshaling cost
cpojer 3y ago
I ran into the same issue and have been separating the prettier and eslint steps ever since: https://cpojer.net/posts/fastest-frontend-tooling-in-2022#eslint--prettier https://cpojer.net/posts/fastest-frontend-tooling-in-2022#es...

tuananh 3y ago

it's very common to have a lint/format step that make sure all files are well-formatted in Pull request.

crabmusket 3y ago
This is such a waste of electricity IMO. Just occasionally batch-fix all files for syntax. Semantic linting can run in CI, as it's about catching bugs. But formatting is for long-term consistency and doesn't need to be done on every commit.
- yawboakye 3y ago
  you may have to force approve edits by the formatter then, since they’re pretty annoying when included in changes for code review.
- dbrgn 3y ago
  Occasionally batch-fixing syntax makes "git blame" far less useful.
  - crabmusket 3y ago
    https://akrabat.com/ignoring-revisions-with-git-blame/ https://akrabat.com/ignoring-revisions-with-git-blame/
    - dbrgn 3y ago
      Yup, but you need to reference the commit ID, meaning that you always need to create two commits, one for the reformatting, and a second one for updating the .git-blame-ignore-revs file. Additionally, you may not use git rebase, or any other workflow that modifies the history, or the references will be broken. Third, the git config needs to be set per-project (can't be set globally), so all developers need to manually adjust their config, for this feature to be useful. If you value a clean commit history, then simply reformat as part of the code change. With autoformatting on save (e.g. through prettier), that's a complete non-issue anyways.

the_other 3y ago

Or, better still, just remove prettier and free yourself to write more expressive code again.

volent 3y ago

I believe you don't see the big picture. What you just described is running on thousands of machines several times a day.