Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

[DO NOT MERGE] Use Prettier for auto formatting and code style rules #700

Closed
wants to merge 4 commits into from

Conversation

agubler
Copy link
Member

@agubler agubler commented Oct 2, 2017

Type: feature (do not merge)

The following has been addressed in the PR:

Description:

Example of the changes from using Prettier for code formatting and styling.

There is formatting here that I'm not keen on, but if we were to embrace Prettier we'd have to accept these changes.... (it's pretty opinionated with only a small configuration API)

@agubler agubler requested a review from kitsonk October 2, 2017 10:22
@kitsonk
Copy link
Member

kitsonk commented Oct 2, 2017

Ewww... that little OCD I have is upset:

export function Container<W extends WidgetBase>(

Cuddling... It does set out its stall as an opinionated formatter, so I guess we can't blame it too much.

@kitsonk
Copy link
Member

kitsonk commented Oct 5, 2017

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.

@agubler
Copy link
Member Author

agubler commented Oct 5, 2017

@kitsonk Thank you for your thoughts, the last item is indeed the most controversial divergence from our current styles.

I'd also like to add the flag when it became available from prettier 👍

@kitsonk
Copy link
Member

kitsonk commented Oct 9, 2017

Refs dojo/meta#206

@kitsonk kitsonk added this to the long-grass milestone Oct 9, 2017
@agubler
Copy link
Member Author

agubler commented Oct 30, 2017

Closing, this was just an example of the formatting changes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants