-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[prefer-interface] Consider removing @typescript-eslint/prefer-interface from the recommended list #433
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
Comments
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.
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). We have added it as a recommended because:
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. |
I completely agree with your point about Regarding the |
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. |
From my understand the difference between
From this post. Other's have written in many web pages that 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,
That's assuming that everyone needs to implement, extend, or merge types. I believe that if someone tries to use |
@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
} |
Correct, there is actually very little difference between the two in the latest versions of typescript. The difference I know of are:
type StrEnum = 'a' | 'b';
type FooType = {
[k in StrEnum]: number
}
interface FooInterface {
[k in StrEnum]: number // all manner of compile-time errors
}
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;
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
} |
That's right. I also meet the error and have to disable this rule. |
I will also suggest the remove this rule due to the above reason. The merging behavior in |
What can't you do with an 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. Also important to note that this rule doesn't force you to use interfaces for function types and doesn't ban |
As you've mentioned " 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 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 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 |
I don't agree that there's any difference in how they look.
Whilst yes, the root of the term
I mean you could say the same thing for Each of these features has their own uses. Each one has benefits. Some work in certain situations that other ones don't.
I don't use them for that. Nor do I use types for overloads definitions. 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
In that instance, you are wrong. |
@bradzacher, oh right my bad, it do working. And most arguments are pretty valid too. So it seems like 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?! |
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. |
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
The text was updated successfully, but these errors were encountered: