dojo/meta

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 use foo 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 of else {)
  • 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:

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)!