Let's get uglier

[antidecaf]

In many pull requests Prettier seems to rewrite loads of irrelevant code to satisfy it's format. For a recent example, see changes in #407. In my opinion these format changes are unnecessary noise that distracts from what is actually going on in any given pull request, and frankly in the end it's more annoying than helpful.

Can we disable Prettier alltogether?

[selbekk]

As the initial pro-Prettier person on this team, I thought I'd chime in:

I like the fact that we have automatically enforced formatting across our code base. This makes it easier for new developers to contribute, because they won't be met with a bunch of "this is great, but can you "-comments.

If unrelated format changes is the issue, that can be resolved simply by:

1. Locking our Prettier dependency (Prettier pushes style alterations as non-majors)
2. Running Prettier on our entire code-base in a single PR

These two efforts should at least alleviate some of the not-so-cool side-effects of having automatic formatting.

That being said - if I'm the only proponent of having Prettier as a tool, and it leads to more annoyance than help - I won't mind removing it either. In that case we need to come up with some new strategy about formatting and code style for FFE.

[benmatthews]

I agree with @selbekk on this one. I have seen way too many comments in PRs in the past about formatting, spaces etc and i think it distracts from the main point of the PR.

+1 for just running prettier on the entire less code in a single PR.

[kwltrs]

If prettier was a fellow developer, I would have a serious talk with them because their code contributions make pull request so much harder to review. And most often, the code style was good enough before prettier laid hand on it.

[antidecaf]

While I do agree about comments on formatting, I really don't agree with Prettier's format.

Stuff like this makes the code less readable, not more.

[selbekk]

Yep, that's the major downside - you can't really micro-manage the formatting with Prettier.

It's up to you if you want to move away from Prettier - I've made my case for why I think we should keep it. If you decide on remove it, that's fine. Perhaps slightly diverging styles or very strict lint rules is a better approach for this mix of developers :)

[ivarni]

eslint supports pretty (groan) much everything prettier does, and can be configured to not be a PITA about it. We even decided on a common ruleset at some point, though to be fair that was a pretty limited ruleset.

You can even run it with the --fix flag on a commithook and it will autofix most cosmetic rules and bark about the ones that can't be safely applied. The only new thing prettier really brought to the table was to remove the opt-in configuration file 🤷‍♂️

I haven't really seen that much noise about codestyle in PRs here either, but I haven't been paying much attention the last couple of weeks so I dunno if it suddenly became a problem.

[selbekk]

What do you think @antidecaf and @kwltrs - should we stop using prettier and introduce eslint --fix on commit instead? Or should we run Prettier on all code and decide that it's Good Enough™️?

[henrikhermansen]

While I agree with the concept of having some forced code style, I'm not a big fan of Prettier's preferences. I could, however, be fine with it if it sticks to only formatting the lines that were changed in each commit.

I thought that was the case, but it surely seems like some of the changes in #407 are purely done by Prettier. Some of the changes I'm even really unsure about, and that's surely not a good thing.

[selbekk]

Formatting is done on all parts of every staged file. I added that intentionally, as the "only format the given patch" approach we tried initially didn't work out very well.

That being said, these issues are resolved by running Prettier on the entire codebase once. That will resolve the added formatting noise in pull requests.

[kwltrs]

Prettier has caused more pain than I have had from bad code style from before we had prettier. So I won't miss automatic code formatting.

I'd prefer to see that we make our eslint rules a bit less forgiving and stick to linting as part of ci testing instead.