NoIssue - Proposal: allow prettier before eslint

This is an attempt to move the discussion forward as suggested by Felix in the recent, public, thread.

Before opening a new thread with findings though, I'd like to champion the idea in ABP UI, which is the code base I know the most.

Only once we understand if we're happy, and we can focus more on important matter, we could create such Discourse post presenting facts, maybe not measurable as data, but as overall happiness, or even increased frustration, if that'll be the case.

Background

The Prettier way of doing things, is to disable eslint completely, and agree on whatever output prettier produce.

This would result into massive discussions and paranoia due full rewrites of our code base. I have already witnessed errors produced by passing prettier through our eslint as it is, and broken code is not exactly where we want to aim.

However, letting prettier define its own output a part, rewriting a clean version of the file, and then use eslint with our rules to fix such output, seems to produce the least practical differences, but a consistent, cleaner, result.

Proposal

In order to have our IDEs also able to enjoy such result, we need to make at least one change: "brace-style": ["error", "stroustrup"].

Even the latest eslint is indeed incapable of understanding curly brackets on a new line, putting these always at the beginning of the next line, the only way to produce a readable output is to have curly brackets opening in the same line.

Since most of the programming community already has a preference, which is curly brackets starting on the same line, and since every tool is familiar with such preference, and there is no difference whatsoever in performance, or logic, it would be not pragmatic to go against this common choice to gain pretty much zero (but effort is required to eventually fix this issue mainstream).

Accordingly, if we don't agree on trying out how this output looks, only, and optionally, in new files we modify, so that we can have a painless incremental migration, we're basically stuck with current decision unless we put effort fixing the new line issue in eslint.

Since I'm proposing this, and every Open Source project I've ever contributed uses the same convention, it's needless to say I'm in favor of dropping new line curly braces from now on.

Stretch Goal

The recent discussion was born after me nitpicking for no real gain or reason a single line if.

It seemed that, at the end of the day, we all agreed that if we have braces, it's always better.

Accordingly, using also "curly": ["error"] would automatically convert any inline, or single line if, else if, or else, into a block with curly braces: we don't need to write it like that, but prettier + eslint can optionally fix that for us.

How To

For the time being, without adding any specific git commit hook, we can decide to npm run fix js/any-file.js before submitting it.

The patched file will always contain, at its end, last time it's been auto-modified, so that it's easy for reviewers to both know upfront the file has been fixed through tools, and when such fix was ultimately applied (i.e. check after asking for changes too)

One Single Rule: First Push - No Changes

We must push the first version of any file we want to change after a first pass to fix, so that we re sure we'll review changes, later on, over some already modified output.

Reviewing changes together with the prettified / eslinted version of the file might be too confusing for the reviewer (but also for the code author).

Example

npm run fix js/file-to-change.js
git add js/file-to-change.js
git commit -m 'Issue XXX - Change the file'

# ... now work on the fixed file, and then
git add js/file-to-change.js
git commit -m 'after fix'

# ... now keep working and either amend or commit over

Alternative: Merge Request of files first

We could also agree if we need to change files we might want to land their fixed by tools counterpart first, and then work on those a part, so it's clear where we've patched in our git logs.

Having a branch over the patched version will also help us not being blocked by patched files reviews.

I am actually probably happy moving forward this way.

As Summary

We have the opportunity to start producing really consistent output if we're flexible enough to accept the combination of prettier, and eslint through almost all our rules after.

This should uniform our style in the long term, and it could be used as proof that reducing nitpicking, even if in a small percentage, is possible, thanks to modern tools.

This approach might also give us the opportunity to strengthen even more our eslint rules, if we find cases we don't really like much, so that we could propose these changes also for the eslint-config-eyeo module, aiming at cross team JS code consistency.