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

[TS] Separate type alias entries using commas instead of semicolons #6655

Closed
mgtitimoli opened this issue Oct 15, 2019 · 8 comments
Closed
Labels
lang:typescript Issues affecting TypeScript-specific constructs (not general JS issues) locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:wontfix This will not be worked on

Comments

@mgtitimoli
Copy link

mgtitimoli commented Oct 15, 2019

Hi there,

Time has passed and new conventions (luckily) have arisen, @calebmer opened an issue some time ago asking for this and @vjeux replied that it was done and then it was rolled back.

A couple of comments later @mafredi said semicolon was used in official docs, and here is when I jump to say this is no longer true.

As you can see in the new handbook, type aliases are written using commas (thankfully), so it could make sense to open this debate again to collect opinions.

I will use the same technique @azz used in this comment

Vote here!

👍 Use commas
👎 Continue using semicolons

@mgtitimoli mgtitimoli changed the title [TS] Separate types aliases entries using commas instead of semicolons [TS] Separate type alias entries using commas instead of semicolons Oct 15, 2019
@lydell lydell added lang:typescript Issues affecting TypeScript-specific constructs (not general JS issues) status:needs discussion Issues needing discussion and a decision to be made before action can be taken labels Oct 15, 2019
@bakkot
Copy link
Collaborator

bakkot commented Oct 16, 2019

(I am pretty sure this is about object types, not type aliases.)

As a data point, it looks like tslint still enforces semicolons. tslint is theoretically being deprecated, but at 2 million downloads per week it's obviously still in pretty wide use.

@mgtitimoli
Copy link
Author

mgtitimoli commented Oct 16, 2019

I would suggest not to take tslint into account, it's already deprecated and it hasn't been updated with the latest standards.

One example of this, is the fact that it recommends the use of an interface over an object type alias.

image

@c32hedge
Copy link

c32hedge commented Feb 5, 2020

Can this be made a configuration option? If I'm not mistaken this would be analogous to the @typescript-eslint/member-delimiter-style rule available in typescript-eslint, which is more relevant than tslint now that the latter is deprecated.

(I happen to support commas instead of semicolons as a default value, but given that the tool's opinion seems to keep going back and forth, I question whether that opinion should trump all other opinions on this particular topic.)

From the object types section that was linked in a previous comment:

Here, we annotated the parameter with a type with two properties - x and y - which are both of type number. You can use , or ; to separate the properties, and the last separator is optional either way.

That, combined with the existing trailing commas option that covers the last part of that same sentence, seem to make it a good candidate for being configurable, IMO.

As things currently stand, it's a bit ironic that the name of the existing option is "trailing commas", yet Prettier enforces semicolons.

@sosukesuzuki
Copy link
Member

sosukesuzuki commented Feb 6, 2020

@c32hedge If you don't know https://prettier.io/docs/en/option-philosophy.html, please see.
I personally prefer using commas as member delimiter. But, I disagree adding configuration options for this.
However, according to the voting on this PR, using commas seem to be more popular. I think we should consider make member delimiter commas.

@c32hedge
Copy link

c32hedge commented Feb 6, 2020

@sosukesuzuki Thanks, hadn't seen that page. Fair enough. The semicolons instead of commas seemed to be the main thing that jumped out as jarring when I just started trying Prettier, so this change would make me a lot happier if not 100% satisfied.

My main point above was that since Prettier's decision on this topic has already gone back and forth at least once, I just want to be able to "lock in" on one style.

@alexander-akait
Copy link
Member

alexander-akait commented Feb 6, 2020

ts uses semicolons in official guides https://www.typescriptlang.org/docs/handbook/interfaces.html, so we don't need change something or introduce the option for that, you can setup eslint rule and use prettier-eslint if you want change something

Honestly, I would close this issue

сс @thorn0

@mgtitimoli
Copy link
Author

mgtitimoli commented Apr 16, 2020

I'm back to TS again on a Project and I still believe it would be better to go with commas and limit the usage of semicolons to separate statements, if this is really controversial we could explore the possibility of going with this approach only for type aliases (using commas), and keep interfaces the way they are now (with semicolons).

What do you think?

@thorn0
Copy link
Member

thorn0 commented Apr 16, 2020

@mgtitimoli This isn't going to change. See the reasoning here: #7719 (comment) and #7719 (comment).

@thorn0 thorn0 closed this as completed Apr 16, 2020
@thorn0 thorn0 added status:wontfix This will not be worked on and removed status:needs discussion Issues needing discussion and a decision to be made before action can be taken labels Apr 16, 2020
@github-actions github-actions bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 16, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang:typescript Issues affecting TypeScript-specific constructs (not general JS issues) locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

7 participants