Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

Consider using prettier to format JS (and also CSS and JSON files) #1229

Closed
jonathaningram opened this issue Sep 14, 2017 · 5 comments
Closed

Comments

@jonathaningram
Copy link
Contributor

Prettier (https://github.com/prettier/prettier) is the defacto frontend code formatter with support for:

  • JavaScript, including ES2017
  • JSX
  • Flow
  • TypeScript
  • CSS, LESS, and SCSS
  • JSON
  • GraphQL

It is essentially the equivalent of gofmt and removes a significant amount of cognitive load when writing and reading a codebase. Prettier has excellent editor support (e.g. format on save), and is also super easy to run over a codebase where there is no editor support. There are many other benefits that we could discuss, but I won't list them here.

Instead, I'll just ask: would you (and your project) be interested in adopting prettier? This would require some changes to your eslint settings, but style and formatting should (arguably) not be part of linting anyway (since you can just resolve it automatically by just running a formatter over the codebase).

Note: the aim is to replace/remove the parts of Airbnb Javascript that are related to formatting and styling, not the other "best practice" parts of it, for example "use const", "use the object spread operator over Object.assign"

Background: as Adam mentioned in #1225 (comment) we're considering using this codebase as a frontend to one of our CF environments within cloud.gov.au.

@jcscottiii
Copy link
Contributor

@jonathaningram sorry for the delay. I've been asking around 18F about people's thoughts on this tool. (The frontend world isn't my expertise)

Overall, people said no to prettier. (it's relatively new in the JS world, worrying about how well it catches issues that are not just formatting, etc.)

Also, I did some research and saw things like this:
airbnb/javascript#1307
airbnb/javascript#1548

With all that said, I'm all for increasing dev productivity. I would open to seeing a PR. I would share it around internally and get thoughts on it. If all looks well, we could merge it! But warning, there's no guarantee that it would be accepted.
If you plan on trying it out, we can keep this issue open to track it.

@jonathaningram
Copy link
Contributor Author

jonathaningram commented Oct 3, 2017

@jcscottiii thanks for your response. As a semi aside: I have a PR for a frontend feature that we're currently reviewing internally. Will hopefully be PRing it soon. It uses prettier on all the new JS code written in it.

Prettier 1.0 has been out for a while and is ready for production. As for who is using it - most of the libs/tools in this cf-dashboard JS stack are using it:

image

With regards to "catches issues that are not just formatting" - think of prettier as exactly the same as gofmt: all we want to do as devs is press "cmd+s" in our editor and have the code format the same. It's not meant to tell me about things like "remove unused var x". As a dev I don't want to think about eslint warnings that are not actually problems with my code like "there should be a space between that opening brace". This topic can go on forever, with opinions one way or another so I don't want to go into it. However, here's an example snippet of the current code I'm writing and how it is unhelpful as a developer:

image

And the problem that eslint reports (with a scary red line):

image

So, I just need that space between the : and the var...

Using prettier in my editor (or however you run it), I just cmd+s and this goes away - meaning it was never actually a problem I should have had to care about :)

Edit: Another example where I've made 3 formatting issues that prettier would just fix on save:

image

And the problems are all related to formatting that literally instantly go away as soon as I press cmd+s (and if you're like me, you probably press cmd+s at least every 10s when writing code).

image

Edit 2: but yep I will do the prettier PR - not sure if I'll run prettier on the codebase for the PR though as it will just make a massive diff of changed JS files which is not that useful to see. If it gets approved we can run it over the codebase once the PR is approved.

@afeld
Copy link
Contributor

afeld commented Jul 26, 2018

cc 18F/frontend#167

@mogul
Copy link
Contributor

mogul commented Jul 26, 2018

@afeld Note we're planning to deprecate the cg-dashboard in favor of https://github.com/cloudfoundry-incubator/stratos so we're unlikely to do anything here.

@afeld
Copy link
Contributor

afeld commented Jul 26, 2018

No problem! Just found this issue searching around @18F repos for "Prettier".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants