Consider using `Prettier` for formatting rules
agubler opened this issue · 6 comments
Since version 1.2.0 Prettier supports TypeScript as a parser. We should consider including Prettier into the Dojo CI pipeline for formatting rules to help enforce a consistent code style across the code base.
We already have a style guide for Dojo 2 but some of the rules are not enforceable with TSLint which means they need to be highlighted through code review, using Prettier's opinionated formatting styles (and updating the style guide to reflect this) in combination with some of the latest, more granular TSLint rules means that contributors can easily format their code in accordance this standard.
Using plugins such as https://github.com/ikatyang/tslint-plugin-prettier means that Prettier errors will be reported by TSLint and fail using grunt dev
/ grunt dist
.
Adding notes from dojo/widget-core#700...
Ok, I have looked at it again and actually the item I objected to was actually contrary to our style guide.
The things that I have issues with:
- Cuddles type assertions (e.g.
<any>foo
instead of<any> foo
). Though we should be avoiding that syntax and instead usefoo as any
. - Removes space from literals (e.g.
[foo, bar]
instead of[ foo, bar ]
) - Does not start certain blocks on a new line (e.g.
} else {
instead ofelse {
) - Does not brace single argument lambda/arrow functions (e.g.
foo => foo.bar
instead of(foo) => foo.bar
).
Of all of them though, it is only the last one that I really have an issue with, and it seems I am not the only one as prettier/prettier#812 is a request. The current status is that they are going to consider it because of the amount of feedback, though they have locked the issue. Since it feels like it is something they will eventually deliver, I would not object to using prettier, though I would want to adopt that flag as soon as it becomes available.
Prettier 1.9.0+ added the option to ensure params are added to single argument arrow functions, which resolves @kitsonk's biggest concern from the initial POC.
From here, I think we are at a point where we can start rolling the changes across the packages.
Packages:
dojo/cli
- Raised
- Merged
dojo/cli-build-app
- Raised
- Merged
dojo/cli-build-widget
- Raised
- Merged
dojo/cli-build-webpack
- Raised
- Merged
dojo/cli-create-app
- Raised
- Merged
dojo/cli-create-theme
- Raised
- Merged
dojo/cli-create-widget
- Raised
- Merged
dojo/cli-export-project
- Raised
- Merged
dojo/cli-test-intern
- Raised
- Merged
dojo/core
- Raised
- Merged
dojo/dgrid
- Not adding prettier to dgrid
dojo/examples
- Not adding prettier to examples
dojo/has
- Raised
- Merged
dojo/i18n
- Raised
- Merged
dojo/interfaces
- Raised
- Merged
dojo/interop
- Raised
- Merged
dojo/loader
- Raised
- Merged
dojo/routing
- Raised
- Merged
dojo/shim
- Raised
- Merged
dojo/static-optimize-plugin
- Not adding prettier to static-optimize-plugin, functionality has been moved to webpack-contrib
dojo/stores
- Raised
- Merged
dojo/streams
- Not in active development, will add specific issue for streams
dojo/widget-core
- Raised
- Merged
dojo/widgets
- Different requirements for widgets (requires ignoring render blocks as prettier doesn't format
v()
andw()
calls nicely), will add a specific issue.
- Different requirements for widgets (requires ignoring render blocks as prettier doesn't format
dojo/dojo2-package-template
- Raised
- Merged
dojo/grunt-dojo2
- Raised
- Merged
dojo/test-extras
- Raised
- Merged
dojo/webpack-contrib
- Raised
- Merged
Prettier supports adding config as prettier
to the package.json
. Do you think we should configure it in that fashion?
@kitsonk my thoughts exactly and have already added that to the widget-core conversation (just hadn't pushed)
All done, thanks everyone for making things prettier (sadly pun intended)!