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

build: replace prettier with biome #61397

Closed
wants to merge 1 commit into from
Closed

build: replace prettier with biome #61397

wants to merge 1 commit into from

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Dec 7, 2023

Summary

This PR replaces Prettier and several Eslint rules with Biome for reduced CI times for faster linting & formatting. Linting and formatting now takes 44 seconds, compared to master branch with 95 seconds.

Here's a list of caveats with the proposed changes:

Why Biome?

I recommend reading the Philosophy behind Biome.

Impact

⚠️ The following results are taken on my work M2 machine. The results will be a lot different on shared VM such as GitHub Actions. Please use this as a baseline and execute the following scripts on your own machine.

Before

Duration

It takes 95 seconds to run yarn lint which includes prettier with eslint. In order

yarn lint  95.66s user 4.21s system 159% cpu 1:02.74 total

Time consuming Eslint rules

The following shows the top 10 most time spent rules in Eslint. In order to reproduce the following on your local, please run TIMING=10 yarn lint

Rule                                                          | Time (ms) | Relative
:-------------------------------------------------------------|----------:|--------:
prettier/prettier                                             | 21383.555 |    45.6%
import/default                                                |  6131.662 |    13.1%
import/no-unresolved                                          |  3002.880 |     6.4%
@typescript-eslint/naming-convention                          |  2618.324 |     5.6%
react/no-direct-mutation-state                                |  1669.827 |     3.6%
no-lookahead-lookbehind-regexp/no-lookahead-lookbehind-regexp |  1287.728 |     2.7%
import/no-nodejs-modules                                      |  1058.123 |     2.3%
@typescript-eslint/no-redeclare                               |   889.496 |     1.9%
react/sort-comp                                               |   599.328 |     1.3%
react/no-typos                                                |   578.446 |     1.2%

After

Duration

It takes 44 seconds to run yarn lint command, and only 531ms is spent on Biome. Ideally moving more linting rules to Biome will reduce the linting speed.

yarn lint  44.17s user 2.54s system 151% cpu 30.738 total

Remaining Eslint rules

The following shows the top 10 most time spent rules in Eslint. In order to reproduce the following on your local, please run TIMING=10 yarn lint:js

Rule                                                          | Time (ms) | Relative
:-------------------------------------------------------------|----------:|--------:
import/no-duplicates                                          |  3262.210 |    23.3%
@typescript-eslint/naming-convention                          |  2395.588 |    17.1%
react/no-direct-mutation-state                                |  1504.342 |    10.8%
no-lookahead-lookbehind-regexp/no-lookahead-lookbehind-regexp |  1127.794 |     8.1%
react/sort-comp                                               |   608.894 |     4.4%
react/function-component-definition                           |   491.970 |     3.5%
simple-import-sort/imports                                    |   455.786 |     3.3%
react/jsx-fragments                                           |   297.780 |     2.1%
jest/no-large-snapshots                                       |   252.458 |     1.8%
react/no-deprecated                                           |   187.937 |     1.3%

@anonrig anonrig requested review from a team as code owners December 7, 2023 22:14
@anonrig anonrig requested a review from a team December 7, 2023 22:14
@anonrig anonrig requested review from a team as code owners December 7, 2023 22:14
@github-actions github-actions bot added Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Dec 7, 2023
Copy link
Contributor

github-actions bot commented Dec 7, 2023

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

.eslintrc.js Show resolved Hide resolved
Copy link
Member

@asottile-sentry asottile-sentry left a comment

Choose a reason for hiding this comment

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

@getsantry
Copy link
Contributor

getsantry bot commented Jan 10, 2024

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Jan 10, 2024
@getsantry getsantry bot closed this Jan 18, 2024
@mitsuhiko
Copy link
Member

mitsuhiko commented Jan 18, 2024

Is there a reason we did not merge this? (Or the other one)

@anonrig
Copy link
Member Author

anonrig commented Jan 18, 2024

Is there a reason we did not merge this? (Or the other one)

I couldn't find anybody to land this and later help me track the health of the release, in case there is a bug.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants