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

Inconsistent ending for type #7719

Closed
vjeux opened this issue Mar 5, 2020 · 10 comments
Closed

Inconsistent ending for type #7719

vjeux opened this issue Mar 5, 2020 · 10 comments
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:needs discussion Issues needing discussion and a decision to be made before action can be taken

Comments

@vjeux
Copy link
Contributor

vjeux commented Mar 5, 2020

I believe that the one we should have is what flow is outputting. Does someone have more context around why there's a difference here?

Flow

--parser babel
export type Block = {
  format: BlockFormat,
  lang: BlockLanguage
}

Typescript

--parser typescript
type Block = {
  format: BlockFormat;
  lang: BlockLanguage;
};
@bradzacher
Copy link

TS and flow are different systems, so I don't think you should necessarily aim for consistency between the two.

In flow you pretty much only ever use type Obj = { }; types, so using a , makes sense, because it's only one step away from an actual js object.

OTOH, in TS the majority of the community uses interface Obj { } types, which is much closer to class Obj { }, in which you delimit properties with ;.

The question then becomes "in TS, should prettier use the same delimiter for interface and object types, or should they use different characters?"
IMO consistency is better, as it causes less diff churn, so I think that ; makes sense as the character for all type members in TS.

@bolinfest
Copy link
Contributor

@bradzacher I feel like I am going to lose this battle, but nevertheless, I favor the consistency of "objects looking like objects" over "objects looking like interface". In the case of type Obj, using ; within the curlies makes it look like a statement, which it is not. So from my perspective, the status quo is the inconsistency!

@sosukesuzuki
Copy link
Member

As mentioned in #6655 (comment), the official guide of TS uses semicolons.(The comment mentioned about only interface, but the guide also uses semicolon for Type Alias(https://www.typescriptlang.org/docs/handbook/advanced-types.html#type-aliases))

@sosukesuzuki sosukesuzuki added the lang:typescript Issues affecting TypeScript-specific constructs (not general JS issues) label Mar 6, 2020
@thorn0
Copy link
Member

thorn0 commented Mar 6, 2020

Object types in TS are anonymous interfaces. They can contain everything named interfaces can, e.g. method signatures, which look really strange with commas:

let a: {
    foo(): number,
    bar(): string,
    bar(baz: string): void,
    (): void,
};

The relationship between object types and interfaces closely resembles the one between function expressions and function declarations. A function expression can be assigned to a variable (let f = ...) to achieve almost the same effect a function declaration would have, but still function declarations have some extra behavior, namely hoisting. Same with object types and interfaces. If you assign an object type to an alias (type t = ...), it will be almost the same thing as an interface declaration, but it won't have the extra behavior interface declarations have, namely declaration merging.

The point I'm making is that using different formatting for object types and interfaces is very much like using different formatting for function expressions and function declarations. Not only inconsistent, it'd be also misleading, making object types and interfaces look more different than they actually are.

@alexander-akait alexander-akait added the status:needs discussion Issues needing discussion and a decision to be made before action can be taken label Mar 6, 2020
@bradzacher
Copy link

interestingly, I just found out that prettier actually treats types and interfaces in flow differently:

interface Test {
  semi: string;
  comma: string,
}

type Test = {
  semi: string;
  comma: string,
};

is formatted to the following with babel/babel-flow:

interface Test {
  semi: string;
  comma: string;
}

type Test = {
  semi: string,
  comma: string,
};

Whereas it's formatted to the following with typescript:

interface Test {
  semi: string;
  comma: string;
}

type Test = {
  semi: string;
  comma: string;
};

repl

@seahindeniz
Copy link

seahindeniz commented May 4, 2020

@thorn0 Please see this @typescript-eslint/member-delimiter-style

{
  "rules": {
    "@typescript-eslint/member-delimiter-style": [
      "error",
      {
        "multiline": {
          "delimiter": "comma",
          "requireLast": true
        },
        "singleline": {
          "delimiter": "comma",
          "requireLast": true
        }
      }
    ]
  }
}

Having this configuration allows eslint to use comma instead of semicolon

Eslint before the rule
image

Eslint after adding the rule
image

After format
image

@thorn0
Copy link
Member

thorn0 commented May 7, 2020

@seahindeniz And that's what that page says: "Semicolon style (default, preferred in TypeScript)". To avoid conflicts with Prettier, such formatting-related rules should be disabled. See https://prettier.io/docs/en/integrating-with-linters.html#eslint


I'm closing this issue, see the justification above: #7719 (comment), #7719 (comment)

@thorn0 thorn0 closed this as completed May 7, 2020
@seahindeniz
Copy link

@thorn0 yes, I have seen that before but there is also no restriction on the Typescript that enforces to use semicolons. The comma support has added in Typescript 1.5 see #1971
Can't we can agree semicolon usage shouldn't be also enforced by Prettier and should be an optional choice?
Like default as semicolon and user can change delimiter as comma?

This is kind of look like the argument between tab and 2 or 4 spaces usage, isn't it? 😕

@thorn0
Copy link
Member

thorn0 commented May 7, 2020

@seahindeniz I guess you've seen the option philosophy page and #40 too, right? This isn't going to happen. Tab width is a much more divisive issue than this.

@seahindeniz
Copy link

seahindeniz commented May 7, 2020

I didn't read that before and now did.

Everybody wants to add just a tiny piece of configuration to satisfy their use case. As a result we end up with inconsistently designed tools, with unexpected combinations that don’t work correctly, with bugs and regressions that, when fixed in one configuration, break something else with another configuration.

I can say and confirm that this isn't directly related to my use case. I'm simply referring is that Typescript has support for comma delimiter since 2015 and there is no enforcement that restricts us to use semicolon in type/interface definitions. We can agree there isn't going to be a bug in both usages, am I right?

Btw. the tab width example is just to show that you there is no restriction between both usages, therefore, there shouldn't be for delimiters.

@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 Aug 6, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 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:needs discussion Issues needing discussion and a decision to be made before action can be taken
Projects
None yet
Development

No branches or pull requests

7 participants