use prettier to format all of js,json files #1883
Conversation
|
Nice work. I'll check it out ASAP and report back. |
There was a problem hiding this comment.
Finally, if you could change bracketSpacing to false I think it would help to cut down the number of lines touched by this PR.
Still, I'm not entirely convinced I prefer this over Standard.js, so can you explain its advantages and what the development workflow looks like with this? Do I have to install any IDE plugins? Run a script before committing?
.prettierrc
Outdated
| @@ -0,0 +1,5 @@ | |||
| { | |||
| "singleQuote": true, | |||
| "trailingComma": "es5", | |||
There was a problem hiding this comment.
I'd prefer if this was set to "none". Also, can you set semi: false?
lib/base/collection.js
Outdated
| iterator, | ||
| initialValue, | ||
| context | ||
| ) { |
There was a problem hiding this comment.
Wow. I really hate how the function arguments are split into individual lines like this. Makes no sense for such a small number of arguments. Is it possible to avoid this?
lib/base/events.js
Outdated
| const args = Array.from(arguments); | ||
|
|
||
| return Promise.mapSeries(listeners, listener => listener.apply(this, args.slice(1))); | ||
| return Promise.mapSeries(listeners, (listener) => | ||
| listener.apply(this, args.slice(1)) |
There was a problem hiding this comment.
This is weird. Splitting a single line fat arrow function into multiple lines makes its intention a bit ambiguous: e.g. was it supposed to be returning something or was it supposed to return undefined? Did the author made a mistake?
lib/base/model.js
Outdated
| throw new TypeError(`Cannot accept incompatible options: method=insert, patch=${options.patch}`); | ||
| throw new TypeError( | ||
| `Cannot accept incompatible options: method=insert, patch=${ | ||
| options.patch |
There was a problem hiding this comment.
This is definitely super weird 😕 Why is the options.patch in a single line by itself? Is there like a 80 column max line length limit or something?
There was a problem hiding this comment.
Yes, it wraps per default at 80 columns however you can set it via printWidth to a more reasonable amount. I suggest 120 columns. This should fix most of your aforementioned problems. How many columns do you suggest @ricardograca
There was a problem hiding this comment.
👍 120 sounds good to me as well. 80 is definitely too narrow.
There was a problem hiding this comment.
@chentsulin Can you please set printWidth: 120 and run prettier on the codebase again?
chentsulin
force-pushed
the
prettier
branch
2 times, most recently
from
c66d786
to
e17aea3
Compare
|
Added those config: |
|
@chentsulin Can you address the last remaining comment above? I can do that after merging this, but I'd like to avoid several multi-thousand lines PRs if possible. |
|
I set Before:
After
|
|
Many thanks! |
Introduction
Use prettier and husky + lint-staged precommit hook to auto format all of js, json files
Proposed solution
Follow @tgriesser's
.prettierrcconfig and npm scripts in knex/knex#2697