Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prettier #135

Closed
giuseppeg opened this issue Feb 19, 2017 · 8 comments
Closed

Add prettier #135

giuseppeg opened this issue Feb 19, 2017 · 8 comments

Comments

@giuseppeg
Copy link
Collaborator

giuseppeg commented Feb 19, 2017

Currently we use xo to lint our code.
We should replace it with standard.js

Let's just add Prettier. See https://github.com/zeit/now-cli/blob/master/package.json#L39-L52

@nkzawa
Copy link
Contributor

nkzawa commented Feb 19, 2017

I wonder if we can just format by https://github.com/jlongster/prettier instead. @rauchg

@giuseppeg
Copy link
Collaborator Author

We can have "prettier" fixtures but we still need a linter for the library code

@giuseppeg
Copy link
Collaborator Author

oh wait I thought prettier was only for JSX, awesome!

@rauchg
Copy link
Member

rauchg commented Feb 19, 2017

prettier + linting rules for mistakes / warnings

@niksurff
Copy link

niksurff commented Mar 4, 2017

I started experimenting with this.

My starting point was to lint using eslint-config-standard as base and resetting styling rules with eslint-config-prettier. The idea was then to auto-fix styling on a pre-commit hook and only lint for common mistakes. This way a dev isn't bothered at all with errors like there should be a space at line 22 column 68. Also vanilla eslint makes it easy to use editor plugins for real time feedback.

The problem: I noticed prettier adds semicolons to the output which might not be configurable any time soon (discussion here). There is a workaround for this, like piping prettier's output into standard --fix (There is a package for that). In this case code gets formatted once using prettier's advanced algorithm and then again using standard --fix. Which—due to different styling rules—does more than just removing semicolons. In my opinion this gets more complicated than it needs to be.

This might not be a big problem. But before I continue I wanted to check what the general consent is or rather what the expected developer experience is concerning linting and (auto-fixing) code style.

My suggestion is to use prettier as is or just use standard by itself (which again leaves a dev with styling errors, also fixable by using --fix somewhere in the chain).

📓 You may experiment with the changes here. After cloning (or c/p package.json) run yarn install followed by npm run lint and npm run prettier or npm run prettier-standard. Diff the output.

cc @giuseppeg @rauchg

Update: I just found prettier-eslint and the corresponding cli—both part of the prettier org—which we might just use instead of vanilla prettier and add a no semicolons eslint rule (plus other styling rules we want to keep)!? This seems relatively clean.

@niksurff
Copy link

niksurff commented Mar 5, 2017

I just realized me rambling along last night and maybe not making any sense. 🙉

Anyways I submitted a PR which I believe is a good starting point for further discussion. :octocat:

@giuseppeg
Copy link
Collaborator Author

@nikvm cool! Maybe we can just use prettier-eslint + omit semicolons?
Do we need other plugins from standard.js? It'd be nice to keep the setup minimal and unopinionated – the semicolons is the only exception I'd allow.

@niksurff
Copy link

niksurff commented Mar 8, 2017

@giuseppeg I continued the discussion in the PR thread.

@giuseppeg giuseppeg changed the title Replace xo with standard.js Add prettier May 18, 2017
@giuseppeg giuseppeg mentioned this issue May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants