Switch to Prettier for TypeScript Code formatting #12294
Conversation
listochkin
changed the title
[Prettier][1] is an up-to date code formatter for JavaScript ecosystem.
For settings we rely on [EditorConfig][2] for things like tab style and
size (with added bonus that the code editor with an EditorConfig plugin
does some automated code formatting on file save for you). Unfortunately,
Prettier's Glob handling isn't great:
1. `*.{ts,js,json}` has no effect
2. Similarly, in a list of globs `*.ts,*.js,*.json` only the first glob
has an effect, the rest are ignored.
That's why the file looks the way it does.
The only other setting we change is line width. [Lukas][3] suggested we
use 100 instead of 80, because that's what Rustfmt is using.
[1]: https://prettier.io
[2]: https://editorconfig.org
[3]: https://github.com/Veykril
Per [bjorn3][https://github.com/bjorn3] suggestion resolves cases where an early return is moved to a separate line due to line width formatting. This setting changes ``` if (a very long condition) return; ``` to ``` if (a very long condition) { return; } ``` while keeping ``` if (short) return; ``` as is. In pathological cases this may cause `npm run fix` not to fix formatting in one go and may require running it twice.
| @@ -0,0 +1,19 @@ | |||
| # https://EditorConfig.org | |||
There was a problem hiding this comment.
I'm not sure if we want to add this to the repo (first time I'm hearing about this file tbh).
Would it be good enough for now to add .editorconfig to .gitignore assuming you use this personally for configurations?
There was a problem hiding this comment.
The whole point is to not have this file in .gitignore. It's a way for the repo to force some settings on the editor in a cross-editor manner. So, we want to have this file present when the user clones the repo.
You write code in VSCode, I type it in Vim, someone else is using CLion, etc. Yet, our editors agree to use the same character encoding, insert a final new line at the end of every file, not to leave whitespace at the end of the line, etc. It is language agnostic and editor agnostic. It doesn't give you any guidelines other than that. It does require a plugin for some editors (notably both Vim and VSCode), but without a plugin this file doesn't do any harm either.
EditorConfig files are very common, if you look around GitHub you'll see them everywhere. In fact, they are so popular some tools like Prettier decided to use them as a substitution for some of their own setting.
I could put all prettier settings in it's own file, and this way the only time, say, the final new line would be added to a file is during the npm run fix. But if I split these basic things into .editorconfig then my editor would do that cleanup on file save, too. It's a question of what we prefer.
There was a problem hiding this comment.
Alright. It's mainly that I am a bit wary of adding new files I've never heard of to the toplevel. But sure, let's add this and see.
EditorConfig files are very common, if you look around GitHub you'll see them everywhere.
This one surprises me though since as I've said I've never seen this before. (Not that I am doubting you here)
|
@bors r+ |
|
📌 Commit 00a9727 has been approved by |
|
☀️ Test successful - checks-actions |
Summary of changes:
.editorconfigfile to dictate general hygienic stuff like character encoding, no trailing whitespace, new line symbols etc. for all files (e.g. Markdown). Install an editor plugin to get this rudimentary formatting assistance automatically. Prettier can read this file and, for example, use it for indentation style and size.Rustfmtuses.package.jsonto use Prettier instead oftsfmtfor code formatting..git-blame-ignore-revsfile. For it to work you need to add a configuration to your git installation:typescript-formatterfrom the list of dependencies.What follows below is summary of the discussion we had on Zulip about the formatter switch:
Background
For the context, there are three reasons why we went with
tsfmtoriginally:tsfmt)And the meta reason here was that we didn't have anyone familiar with frontend, so went for the simplest option, at the expense of features and convenience.
Meanwhile, Prettier became a formatter project that JS community consolidated on a few years ago. It's similar to
go fmt/cargo fmtin spirit: minimal to no configuration to promote general uniformity in the ecosystem. There are some options, that were needed early on to make sure the project gained momentum, but by no means it's a customizable formatter that is easy to adjust to reduce the number of changes for adoption.Overview of changes performed by Prettier
Some of the changes are acceptable. Prettier dictates a unified string quoting style, and as a result half of our imports at the top are changed. No one would mind that. Some one-line changes are due to string quotes, too, and although these a re numerous, the surrounding lines aren't changed, and git blame / GitLens will still show relevant context.
Some are toss ups.
trailingCommaoption - set it tonone, and get a bunch of meaningless changes in half of the code. set it toalland get a bunch of changes in the other half of the code. Same with using parentheses around single parameters in arrow functions:x => x + 1vs(x) => x + 1. Perrier forces one style or the other, but we use both in our code.Like I said, the changes above are Ok - they take a single line, don't disrupt GitLens / git blame much. The big one is line width. Prettier wants you to choose one and stick to it. The default is 80 and it forces some reformatting to squish deeply nested code or long function type declarations. If I set it to 100-120, then Prettier finds other parts of code where a multi-line expression can be smashed into a single long line. The problem is that in both cases some of the lines that get changed are interesting, they contain somewhat non-trivial logic, and if I were to work on them in future I would love to see the commit annotations that tell me something relevant. Alas, we use some of that.
Project impact
Though Prettier is a mainstream JS project it has no dependencies. We add another package so that it and ESLint work together nicely, and that's it.