Pretty printed code

[nickbalestra]

Initial PR to check out prettier.

The following PR use prettier-eslint to pipe code through prettier->eslint--fix to format code. Meaning that eslint will be run last allowing for a final override of the prettier output.

I see we have quite a big eslintrc file, I guess it could be a bit more minimal perhaps? What do you think @elboletaire ?

Following steps could be:

• use lint-staged together with husky for a totally invisible/painless workflow. I've been using it with prettier as suggested and its pretty awesome.

@MicheleBertoli would love to hear your thoughts in hooking prettier with eslint in here

cc @mattiaerre @matteofigus

[matteofigus]

Looks good, I just was thinking, should we make what you recommend (make the format command run as pre-commit) part of this PR?

[nickbalestra]

@matteofigus yes, just wanted to make sure we all liked prettier output first :)

TODO:

• husky + lint-staged

[nickbalestra]

@mattiaerre @matteofigus one thing that we should also remove is running eslint before testing now, as we will automatically run it with --fix together with prettier. Thoughts?

[matteofigus]

eslint in CI checks with a clean npm install, which is something I can't guarantee the user has when doing the commit. I think we should leave it here, as it's cheap and free to use.

[nickbalestra]

Ideal workflow:

1. write code
2. make tests greens (a ; shouldn't be the blocker for the test as this get into your way) <- we could remove the eslint check here
3. commit and push (the 'magic' happens on pre-commit, where all the .js file staged get piped through prettier --write -> eslint--fix and are re git added before commit)

The user shouldn't worry to much about formatting the code according to your opinions, and you want to be sure that everything is commited as you wanted. As you are running eslint --fix, testing against it shouldn't be necessary anymore. That's why I think we could remove it from the grunt test task.

@kentcdodds am I missing something or does this make sense ? I'm trying to understand how we could add prettier in our workflow here...

[kentcdodds]

Sounds right to me. Important to note that if you want to do a partial commit of a file you'll want to run that with --no-verify, otherwise the whole file will get added via this git hook. But that's often not a problem in my work personally so I don't really care much :)

[nickbalestra]

Indeed! thank you so much for the feedback and for prettier-eslint

[elboletaire]

Hey sorry for the delay, I've been out these days. Answering what @nickbalestra asked when opening the PR, of course it can be reduced. Remember that I used the eslint generator for that first step and, as I said then, many of the rules should be verified. Specially now that you're adding prettier, as many of that rules where related to coding style.

Moreover, eslint rules can extend other rules. So, another way to significantly reduce the eslint file size could be search for a configuration that fits our needs (or the majority of them) and extend from it.

[nickbalestra]

Thank you for the input @elboletaire! Feel free to suggest any thing you might consider we want to clean from them as w moving forward with this.

[nickbalestra]

Once this land in master, would be nice to add OpenTable to this -> prettier/prettier#1351

[elboletaire]

I'd encourage to add the dangling comma, as it's a good practice when using source control (adding it won't show a modified line every time we add a line to an array/object).

But this is something should be discussed, I guess (that's why I'm not setting this as a review comment).

http://eslint.org/docs/rules/comma-dangle

[nickbalestra]

That was already like that, so didn't want to modify any of our rules with this PR.
But totally, we could definitely discuss all of those one-by-one so please feel free to lead the effort with issues/pr in this direction so that we can discuss each of those improvements. Thanks for that!

[matteofigus]

@nickbalestra now that we have prettier-eslint-cli and removed eslint directly from the grunt task, shouldn't we remove grunt-eslint as dev dependency, including its own /tasks file?

[nickbalestra]

Yes!

[matteofigus]

Unless we want to leave it as utility for development

[nickbalestra]

I think make sense to remove it (also help a little toward moving to a gruntless world).

You could now run prettier-eslint instead if you want for developement

[nickbalestra]

@matteofigus @elboletaire should be good to go now.