iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
620 stars 210 forks source link

Add dprint code formatter #7263

Open wgoehrig opened 1 month ago

wgoehrig commented 1 month ago

The primary reason for choosing dprint was performance - check out the video at https://dprint.dev/overview/ to see just how fast format on save can be.

This PR adds a new rush fmt command that will run dprint fmt against the entire monorepo, but that's probably the slowest way to run dprint, since rush and node add a lot of unnecessary startup overhead. In fact, on my machine running rush fmt is at least 10x slower than just running ./tools/build/node_modules/dprint/dprint fmt from the root of the repo. In fact, dprint fmt was able format every file in the repo in under a second!

Either way, I recommend just setting up your editor of choice to run dprint fmt on save and never worry about formatting again. I've added dprint as a recommended VSCode extension and setup format-on-save in the VSCode workspace settings, although you'll want to make sure editor.defaultFormatter is not set differently in your user settings as that will override the workspace.

As for the actual formatting, I've tried to configure dprint to match our existing formatting as much as possible - we can always choose more opinionated settings if folks are OK with more initial churn. Let's get all the arguing about style rules out of our system now and not look back.

Still to do/open questions:

pmconne commented 1 month ago

Have you checked yet to see if the dprint formatted code conflicts with any of our formatting-related lint rules? Ideally we could remove all such rules and let the formatter worry about formatting and the linter worry about code.

wgoehrig commented 1 month ago

Ideally we could remove all such rules and let the formatter worry about formatting and the linter worry about code.

Many of those rules were removed in the latest version of typescript-eslint (stylistic eslint rules overall are now discouraged by typescript-eslint), so I do plan on removing all stylistic lint rules, which should hopefully also speed up lint times.

wgoehrig commented 2 weeks ago

This is blocked by https://github.com/swc-project/swc/issues/9688