Skip to content

Extend rules for merging specialized call signatures in case of interface ... extends A, B #2888

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

Closed
benjamin-hg opened this issue Apr 23, 2015 · 4 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@benjamin-hg
Copy link

Extend the rules for merging type members in case of extends as follows:
Merge interfaces like they had the same name. Afterwards, go through the merged members list from the last to the first. Remove all call signature declarations for which a type-equivalent one did already occur in the list.
At least in simple cases, like in the example below, this should yield the expected result.
Maybe there are some edge cases that still need to yield an error or that need the mentioned workaround.

Example:

declare class Super {
    get(property:string):any;
}

declare class SubA extends Super {
    get(property:"name"):string;
    get(property:string):any;
}

declare class SubB extends Super {
    get(property:"size"):number;
    get(property:string):any;
}

interface Mixin extends SubA, SubB {
    get(property:string):any; // needed workaround, see https://github.com/Microsoft/TypeScript/issues/2871
}

var subB:SubB;
subB.get("size").toFixXed(); // Desired compile error

var mixin:Mixin;
mixin.get("size").toFixXed(); // Unrecognized type error

Relates to #2871

@danquirk
Copy link
Member

For some background on why this behavior exists:

A set of function overloads is actually a single property of a type with multiple call signatures. So SubA's type actually looks like this:

interface SubA { 
   get: { (property: "name"): string; (property: string): any } 
}

SubB has a similar type with a single property named get of a type with multiple call signatures..The problem you're encountering is that function types can't be merged. At the point where you're trying to implement Mixin the compiler is checking its implementation against all the types in its heritage clause. Even if the process of doing that check involved first merging the types in the heritage clause there's no way to merge SubA and SubB's get property.

In contrast, when you merge an interface type the compiler collects all the declarations for that interface type and then processes the resulting set of members. So the compiler doesn't instantiate a get property with 2 call signatures upon seeing the first definition of SubA and then try to add on more call signatures to it later at each subsequent definition of SubA that is encountered.

While I understand the inconsistency here feels a bit odd I'm not sure it's something that's going to change.

@benjamin-hg
Copy link
Author

Tanks for the explanation, @danquirk!
As far as I understood, the issue is that merging properties types like the following is not supported:

get: { (property: "name"): string; (property: string): any } 

and

get: { (property: "size"): number; (property: string): any } 

The merging algorithm suggested above still seems applicable to me.
Using it for this example:

  • Precondition: there are two properties with the same name ("get") which only differ in their call signatures.
  • Concatenate the list of overloads of both of them. This gives us the following list of overloads
    • (property: "name"): string;
    • (property: string): any;
    • (property: "size"): number;
    • (property: string): any;
  • Go through the list from bottom to top. Memorize the visited overloads. If an overload (here: (property: string): any) already was discovered, remove it from the list.
  • Use the remaining list of overloads:
    • (property: "name"): string;
    • (property: "size"): number;
    • (property: string): any;
  • Check if all specialized call signatures are covered by a matching non-specialized call signature (which is the case here). Yield an error otherwise.

Could you give an example, where this kind of merging algorithm would break the typing concept or lead to further inconsistencies?

@RyanCavanaugh
Copy link
Member

The algorithm you describe is already the merging algorithm for bare call signatures on interfaces. The intended design is that if you want to merge call signatures, you do so on an interface type that just has call signatures. If you mix in call signatures through a property, it's less clear that you intend those to merge, especially since there's no obvious runtime parallel to that behavior.

@danquirk
Copy link
Member

If you mix in call signatures through a property, it's less clear that you intend those to merge

This is the usability portion of the problem. There're a lot of places you could imagine allowing merging or overwriting existing values/properties but we have to balance between what JS patterns are handy vs how often that behavior was actually an unintended mistake (or what the implicit intended behavior is).

especially since there's no obvious runtime parallel to that behavior.

And this is the crux of the technical issue as far as why these function typed properties don't merge.

If this was really blocking how you implement certain JS patterns we'd consider alternative options. But as it stands it's creating some pain here but also for a good reason in the cases where you didn't intend to have this collision/merge.

@danquirk danquirk added By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Suggestion An idea for TypeScript Declined The issue was declined as something which matches the TypeScript vision labels Apr 25, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants