FullstackAcademy / boilermaker

Code scaffold for projects
https://www.youtube.com/watch?v=7bLSuTHH4Ag&list=PLx0iOsdUOUmn7D5XL4mRUftn8hvAJGs8H
MIT License
215 stars 706 forks source link

F/prettier #87

Closed glebec closed 6 years ago

glebec commented 6 years ago

Assignee Tasks


This PR adds automatic Prettier support.

queerviolet commented 6 years ago

I'm torn.

On the one hand, I'm lukewarm on automatic code formatters (because they have no respect for poetry), blended with a concern about student confusion that there's something rewriting their source from under them.

On the other hand, I see a lot of code that makes me cry.

@tmkelly28

glebec commented 6 years ago

I feel similarly. If I was working with people who put care into their formatting choices I’d not bother. But I’ve gradually found myself reflexively running Prettier on every student-written module I review because until I do the structure is buried under a mess of random indentations and bracket locations.

An alternative might be to make the repo set up for prettification but stop short of automatically running it on every commit. Make it an easy opt-in choice. EDIT: but on the other hand, it's also easy to opt out of, and in general it's the students who most need this kind of watchdog that are the least likely to opt into it.

tmkelly28 commented 6 years ago

I would much rather this be an opt-in feature. I'd rather tell students who need it to opt-in rather than annoy students who don't.

Somewhat related note, I'm not a huge fan of some of the style decisions happening with this pull request (I like to keep my curly braces tight, dislike trailing commas, and keep a space after function keyword), but we can maybe bikeshed over that stuff later.

glebec commented 6 years ago

Hi @tmkelly28. OK, I'm happy to adjust it to be an opt-in feature.

Re: style decisions, Prettier by design affords very little in the way of customization. The expectation is that all engineers have small differences of opinion when it comes to style choices, but that the time is better spent just writing code however each individual author wants and passively allowing a system to normalize it, rather than forcing people to actively format their code according to a linter. I also do not agree with every single thing Prettier does, but for me the benefit is in completely letting go of such considerations and focusing entirely on logic. That being said, here is this PR's .prettierrc, which lists practically all relevant possible choices for configuration (plus their corresponding defaults):

# printWidth: 80 # 80
# tabWidth: 2 # 2
# useTabs: false # false
semi: false # true
singleQuote: true # false
trailingComma: all # none | es5 | all
# bracketSpacing: true # true
# jsxBracketSameLine: false # false
# arrowParens: avoid # avoid | always

As you can see, I've turned on semi: false, singleQuote: true, and trailingComma: all (as opposed to none) – the latter because it leads to cleaner git diffs. But I'd be happy to discuss changing any or all of these settings.

I do not know whether bracketSpacing (which is designed to affect objects and arrays) also affects ES2015 import statements. I'll try it out and report back.


Edit: yes, it does. Here is a before / after:

import React from 'react'
-import { connect } from 'react-redux'
+import {connect} from 'react-redux'

const AuthForm = props => {
-  const { name, displayName, handleSubmit, error } = props
+  const {name, displayName, handleSubmit, error} = props
glebec commented 6 years ago

Updated to be opt-in. Also re-formatted using stated preference of no space in brackets and no trailing commas.

collin commented 6 years ago

👍

glebec commented 6 years ago

Lockfile sync'd.

The open questions with this PR are as follows: