1507172 - Use Prettier for formatting JS/JSX

Status: RESOLVED FIXED

(Tree Management :: Treeherder: Frontend, enhancement, P1)

Description

[Ed Morley [:emorley]]

We're currently using the AirBnb style guide, with a number of rules disabled for things that we haven't yet had a chance to fix.

A good proportion of those disabled rules are for code formatting such as indentation, bracket spacing etc, and now that we're on newest ESLint (v5) thanks to bug 1364894 I was hoping that the improved rules/auto-fixing would make mass-fixing the formatting deviations much simpler.

However it seems that a number of the ESLint style auto-fixers (particularly for JSX) aren't entirely enough on their own, which means plenty of manual fixing afterwards.

I tried using Prettier instead (which we've previously said we should try out) and it was much more reliable.

As such I think we should start using Prettier for formatting of JS/JSX (it supports other file types too such as JSON and CSS, but seems best to explore those later).

There are a few different ways to use Prettier:
1) Run Prettier via its CLI separately from ESLint:
   -> https://prettier.io/docs/en/cli.html
2) Run Prettier inside ESLint using `eslint-plugin-prettier`:
   -> https://prettier.io/docs/en/eslint.html#use-eslint-to-run-prettier
3) Via some other less common workflows:
   -> https://prettier.io/docs/en/related-projects.html#eslint-integrations

Regardless of the approach taken, it's necessary to disable the parts of the AirBnB ruleset that relate to style (vs code correctness/best practices) so they don't conflict with Prettier. Thankfully there is `eslint-config-prettier` which does that for us.

Having tested both (1) and (2), I'm leaning towards (2) since:
* it's faster (twice as fast)
* it's closer to what people are used to (just running ESLint)
* it's still possible to use Prettier integrations in IDEs etc (either instead of, or in combination with ESLint IDE integrations)

Since Prettier is more strict about code formatting, it will mean that it's more likely people hit linter errors after changes that would require a `yarn lint --fix` - however we can encourage people to use the "format on save" IDE integrations in our docs, which can prevent unnecessary fixups+repushing to Travis.

Cameron/Sarah, does this switch sound ok to you? (I'll open a PR shortly so you can see the code style)

Comment 1

[Ed Morley [:emorley]]

See also:
* "What is Prettier": https://prettier.io/docs/en/index.html
* https://www.youtube.com/watch?v=hkfBvpEfWdA

Comment 2

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4276

Comment 3

[Sarah Clements [:sclements]]

Sounds awesome to me (and the code looks great)! Formatting is tedious :)

Comment 4

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/65b7f4ab451fcb8bba49f5e41963c81be2fa9cdb
Bug 1507172 - Use Prettier for formatting JS/JSX (#4276)

Since it's more reliable (and strict) at code formatting than ESLint.
We use it via an ESLint plugin, and so disable the style-related AirBnB
preset rules, leaving the AirBnB guide to handle only correctness and
best practices rules.

It's highly encouraged to use an IDE integration or Git commit hook
to run Prettier (or `yarn lint --fix`) automatically. See:
* https://prettier.io/docs/en/editors.html
* https://prettier.io/docs/en/precommit.html

We may consider enabling a git commit hook out of the box (using eg
Husky) in the future, however they have previously been known to
interfere with partial-staging workflows, so would need to test the
fixes they have made for them thoroughly first.

In future PRs we may also want to start formatting JSON/CSS/Markdown
using Prettier too.

Comment 5

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4288

Comment 6

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/c3567a2a0c399f38d3e45eab50cdd7224f29658e
Bug 1507172 - Add additional .prettierignore entries (#4288)

For:
* generated directories such as `build/`
* filetypes that are supported by Prettier, but that we haven't yet
  converted to its style (eg CSS, HTML, JSON, YAML)

These entries are not needed when running Prettier via ESLint, but
help prevent the "format on save" feature of IDE prettier plugins
from auto-formatting files when making unrelated changes to them.

As and when we use Prettier with more filetypes, these can be removed.