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

Switch formatting & linting to Rome #217

Closed
lgarron opened this issue Sep 10, 2022 · 1 comment
Closed

Switch formatting & linting to Rome #217

lgarron opened this issue Sep 10, 2022 · 1 comment

Comments

@lgarron
Copy link
Member

lgarron commented Sep 10, 2022

I just switched our formatting and linting to Rome: https://rome.tools/
Commit: f3be7b7

I really wanted to switch to Rome because it:

  • is much faster:
rome eslint + prettier
Commit tested 7cccdcf 9b5fe85
make lint 0.11s ⏱ 5.68s ⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱
make format 0.15s ⏱⏱ 10.85s ⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱⏱

(⏱ = 0.1s)

(Note that npm run and npx add several hundred milliseconds. This time has been removed from the testing of 9b5fe855.)

Basically, ESLint + Prettier is the ecosystem default, but it's painful and brittle to configure, and significantly slower than it "needs" to be. Further, if something breaks you have to look up a solution on your own (which usually involves installing even more non-obvious packages) instead of having a clear place to report issues.

The Rome project is not as mature, but it has pretty comprehensive rules at this point, and they aim for rough feature parity with Prettier (rome/tools#2555, rome/tools#3178) — so we're not going to lose out on a ton of important code issues.

node_modules cost

This is not the top consideration, but switching to Rome also saves us several thousand files and over 100MB in dependencies. For such core ecosystem functionality, this takes us from "ludicrous" to "reasonable".

> mkdir /tmp/eslint-prettier-test && cd /tmp/eslint-prettier-test
> npm init -y
> npm install @babel/eslint-parser @babel/preset-env @babel/preset-typescript @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier eslint-plugin-html eslint-plugin-prettier prettier
> find node_modules | wc -l # file count
    7573
> du -h node_modules | tail -n 1 # disk size
129M	node_modules
> mkdir /tmp/rome-test && cd /tmp/rome-test
> npm init -y
> npm install rome@next
> find node_modules | wc -l # file count
      31
> du -h node_modules | tail -n 1 # disk size
 16M	node_modules

I'm also really glad to be rid of Babel, which tends to require frustrating configuration maintenance compared to other tools (which support ES2020 + TypeScript out of the box).

Transition details

We're using Rome as of f3be7b7.

The main make format transition is commit 5d4256e, which we've added to .git-blame-ignore-revs. This prevents it from showing up GitHub's blame view, with similar functionality in other tools: https://github.blog/changelog/2022-03-24-ignore-commits-in-the-blame-view-beta/

Caveats

We're on the cutting edge. The VSCode Rome extension just barely picks up the rome.json config file as of v0.14.1. Rome itself is also in the middle of a rewrite and only supports linting and formatting so far (so we can't use it for more, although we've been happy with esbuild). During our transition, I even found a bug that broke our code, although hopefully this was limited one-off issue.

Miscellaneous issues:

@lgarron
Copy link
Member Author

lgarron commented Sep 10, 2022

This issue is just for the record. Closing it immediately, since the core changes are already done. For follow-ups, see:

@lgarron lgarron closed this as completed Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant