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

[prefer-interface] Consider removing @typescript-eslint/prefer-interface from the recommended list #433

Closed
mauricedb opened this issue Apr 16, 2019 · 13 comments · Fixed by #729
Labels
has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin recommended-rules Discussion about recommended rule sets

Comments

@mauricedb
Copy link

Currently the @typescript-eslint/prefer-interface is part of the recommended rules as an error. I believe the reasons are dated and in most cases types are just as capable and more flexible. There are a few cases where interfaces are better but I would not recommend them as a default. Therefor I would recommend removing this rule as a recommendation and just allowing users to configure it if they really prefer to.

See also the discussion in issue #142 and point 19 in https://medium.com/@martin_hotell/10-typescript-pro-tips-patterns-with-or-without-react-5799488d6680

@mauricedb mauricedb added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Apr 16, 2019
@bradzacher bradzacher added question Questions! (i.e. not a bug / enhancment / documentation) and removed triage Waiting for maintainers to take a look labels Apr 17, 2019
@bradzacher
Copy link
Member

I disagree with a lot of what's in that medium post. There's a decent chunk of misinformation as well as suggestions that go against the point typescript.
Suggesting things like:

  • Object.freeze instead of just using Readonly<T> types (esp when Object.freeze has significant read time perf impacts (~60% slower reads))
  • Object.freeze over enum because it generates "extra boilerplate" (again, Object.freeze has significant perf impact, and chances are your enum is probably regularly used means you'll take that perf hit a lot).
  • always using import('x') syntax for types because in the off chance you write a TS project with tsconfig module set to none, it'll keep your code consistent.

The specifically pointed out point 19 is only in there because they suggest deriving Props/State types from your default props / initial state definitions (which is really the wrong thing to be doing, esp when most components won't have default props, your state / props could have optional fields, etc).
If you use that pattern, then it makes sense because it reduces diff churn, but apart from that...


We have added it as a recommended because:

  • the typescript docs use them by default.
  • many of the existing community configs had the rule turned on.
    • we didn't have a recommended config until late last year, even though the plugin has existed for ~3 years.

I'm happy to change it if there is significant demand from the community.

As an aside - your point of removing it because those that want it could just turn it on works in both directions though - if you don't like using interfaces, you can configure it if you really prefer to.

@bradzacher bradzacher added recommended-rules Discussion about recommended rule sets and removed question Questions! (i.e. not a bug / enhancment / documentation) labels Apr 17, 2019
@mauricedb
Copy link
Author

@bradzacher,

I completely agree with your point about Object.freeze(). I do use it in addition to Readonly<T> but only in a development build, not at runtime.

Regarding the prefer-interface rule. I have in fact turned it off for our configuration, not hard at all. Lets see what other people think about this.

@senhongo
Copy link

There is also an older discussion on the tslint repo advocating for its removal, with a comment by @lukescott showing how the rule causes breakage, something we've just encountered while trying out this ruleset.

palantir/tslint#3248

@kamok
Copy link

kamok commented Apr 26, 2019

From my understand the difference between type and interface is that...

An interface can be named in an extends or implements clause, but a type alias for an object type literal cannot.
An interface can have multiple merged declarations, but a type alias for an object type literal cannot.

From this post.

Other's have written in many web pages that interface simply has more features.

In my case, I do not want my type to be extended or merged. I want it to be used as-is. This rule seems to encourage people to use a more feature-full version of something that they might not even need or want. I agree that it should be removed as a recommended. The explanation given on the official rule page isn't good enough,

Interfaces are generally preferred over type literals because interfaces can be implemented, extended and merged.

That's assuming that everyone needs to implement, extend, or merge types.

I believe that if someone tries to use type, and find out they need to implement, extend, or merge, then they can use an interface. Always start with something bare minimum and then pull in the right tool when you need to do more. No?

@mauricedb
Copy link
Author

@kamok It seems the TypeScript team don't do a good job of updating the docs when they update language features (or accepting PR's when others do so). A Type is far more capable then it originally was. The following is all valid code and some of it, like an interface extending from a type, would not be valid according to the docs:

const point = {
  x: 1,
  y: 1
};

type PointType = typeof point

interface PointInterface {
    x: number;
    y: number;
}

class PointOne implements PointInterface {
  x: 1
  y: 2
}


class PointTwo implements PointType {
  x: 1
  y: 2
}

interface PointThree extends PointType
{
  z: number
}

@bradzacher
Copy link
Member

bradzacher commented Apr 26, 2019

Correct, there is actually very little difference between the two in the latest versions of typescript.

The difference I know of are:

  • types can be used to create strictly keyed mapped types:
type StrEnum = 'a' | 'b';
type FooType = {
  [k in StrEnum]: number
}
interface FooInterface {
  [k in StrEnum]: number // all manner of compile-time errors
}

repl

  • types can be implicitly cast to an index signature type:
type FooType = {
  prop: 1;
}
interface FooInterface = {
  prop: 1;
}

interface MappedType {
  [k: string]: number
}
declare function test(arg: MappedType): void;

// implicit cast
test({ prop: 1 } as FooType)
test({ prop: 1 } as FooInterface) // Index signature is missing in type 'FooInterface'.

// note that an explicit cast works fine for both cases
const fooInterface: FooInterface = { prop: 1 }
const castInterface = fooInterface as MappedType;

const fooType: FooType = { prop: 1 }
const castType = fooType as MappedType;

repl

  • interfaces are subject to declaration merging:
interface FooInterface {
  a: 1
}

interface FooInterface {
  b: 2
}

const x: FooInterface = {
  a: 1,
  b: 2,
}

type FooType = { // Duplicate identifier 'FooType'.
  a: 1
}
type FooType = { // Duplicate identifier 'FooType'.
  b: 2
}

repl

@iFwu
Copy link

iFwu commented May 8, 2019

Correct, there is actually very little difference between the two in the latest versions of typescript.

That's right. I also meet the error and have to disable this rule.

@leoyli
Copy link

leoyli commented May 10, 2019

I will also suggest the remove this rule due to the above reason. The merging behavior in interface is a weird design but I agree we don't need to use it. On the other hand. type is easier and have no such caveat. Also, when typing in function, especially in higher order function, I'd like to use type alias to separate type declaration and implementation, which interface is not helpful here. That is, type is actually more powerful and useful than interface, IMHO. So, that leaves no reason to use an interface since we can just use type all the time.

@bradzacher
Copy link
Member

bradzacher commented May 10, 2019

Also, when typing in function, especially in higher order function, I'd like to use type alias to separate type declaration and implementation, which interface is not helpful here

What can't you do with an interface that you can do with types in this regard?
You can declare an interface function type, and you can declare overloads in an interface with function types. You can also declare a constructor in an interface.

interface OverloadInterface {
  (): void
  (arg: string): number
  (arg: number): string
}

type OverloadType =
  & (() => void)
  & ((arg: string) => number)
  & ((arg: number) => string);

They should be feature compatible in this regard.
Sure a type lets you do nicely it in one line, but that's a small thing and a moot point for any complex definition.

Also important to note that this rule doesn't force you to use interfaces for function types and doesn't ban type definitions. It only enforces that when you define an object type, that you use an interface instead of a type.

@leoyli
Copy link

leoyli commented May 16, 2019

As you've mentioned "an object type"; won't you think the type write up is more close to idiomatic Javascript? i.e.:

const ComponentAProps = {
   name: 'John',
   address: 'Mars'
};

// I have similar shape as the instance
type ComponentAProps = {
   name: string;
   address: string;
};

// I looks a bit more exotic from JS wonderland
interface ComponentAProps {
   name: string;
   address: string;
}

Also, Maybe I were wrong, but here is interface can not do:

For example:

type ComponentAProps = {
   name: string;
   address: string;
};

// 👍 
type ComponentBProps = Omit<ComponentAProps, 'address'> & {
   isPreme: boolean;
};

// 👎 I will get a typing complain from TS
interface ComponentBProps extends Omit<ComponentAProps, 'address'> {
   isPreme: boolean;
}

I think interface is more OOP oriented, it goest well with class for sure. But if I sometimes have to use type just like the above to do union or interception, then why not just use type all the time?! This brings more consistency too.

The overloads in interface sometimes brings more headaches since it is not intuitive from a set theory perspective IMHO. And that is why we even got a lint rule to force them to be loaded side-by-side, this rule indicates something 💩, which leads me believe type was added to fix this helpless defeat in interface from the early TS. Sure, it add no harm, but it seems not pointing a good practice.

@bradzacher
Copy link
Member

bradzacher commented May 16, 2019

I don't agree that there's any difference in how they look.
Ultimately there's a total of 4 characters of difference between the two:

type ComponentAProps = {
interface ComponentAProps {

Whilst yes, the root of the term interface does come from OOP, in typescript it is almost entirely analogous to type when it comes to object types.
Apart from the few differences I mentioned above #433 (comment), the two are pretty much compatible when it comes to object types.

But if I sometimes have to use type just like the above to do union or interception, then why not just use type all the time?!

I mean you could say the same thing for let vs var vs const. Or using funciton declarations function x() {} vs variable function declarations const x = function () {} vs using arrow functions const x = () => {}. Or using classes vs using old school functions and prototypes. Or using switch/case vs if/else.

Each of these features has their own uses. Each one has benefits. Some work in certain situations that other ones don't.
In some situtations things are analogous. Some people prefer to use them in certain cases, whilst others don't. It's a stylistic preference about which one to use and when.

The overloads in interface sometimes brings more headaches since it is not intuitive from a set theory perspective IMHO.

I don't use them for that. Nor do I use types for overloads definitions.
I just declare overloads as overloads, because it's much cleaner IN MY OPINION.

function x(): void
function x(arg: string): number
function x(arg: number): string

Using a lint rule to ensure they're next to one another isn't a sign of a code-smell or bad language design. It's a flexible structure and people might want to write their code however they like. For example someone might like to do something like:

function x(): void
function x(arg: string): number
type OptArg = {
  prop: string
}
function x(arg: OptArg): string

Some would prefer that OptArg sits outside the list of overloads, some might like the declaration to be next to the usage. There's nothing 💩 about either of those choices.


Maybe I were wrong, but here is interface can not do

In that instance, you are wrong.
Your omit example works fine with either types or with interfaces: playground example

@leoyli
Copy link

leoyli commented May 16, 2019

@bradzacher, oh right my bad, it do working. And most arguments are pretty valid too. So it seems like interface and type really don't have anything that fundamentally different?!

I see unless TS official website updated that resulted in a practice shift, then this recommendation would be valid. I have turned it off anyway because I more in favour assignment or expressional styles. I guess this issue then can be closed?!

@bradzacher
Copy link
Member

We'll be reviewing the recommended set and making it less "opinionated". Changes to the recommended set are considered breaking changes though, so it'll be done as part of the 2.0.0 (whenever the time comes for that).

I'll be leaving this open till we take care of that, so that it doesn't get lost.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has pr there is a PR raised to close this package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin recommended-rules Discussion about recommended rule sets
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants