Skip to content

No common supertype between {optional, optional} and {required, absent} #1446

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
JsonFreeman opened this issue Dec 11, 2014 · 9 comments · Fixed by #1795
Closed

No common supertype between {optional, optional} and {required, absent} #1446

JsonFreeman opened this issue Dec 11, 2014 · 9 comments · Fixed by #1795
Assignees
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Fixed A PR has been merged for this issue Question An issue which isn't directly actionable in code Spec Issues related to the TypeScript language specification

Comments

@JsonFreeman
Copy link
Contributor

We made the change that an object type with a missing property cannot be a subtype of an object with an optional property. This made subtype choose better results in some cases, because it fixed cases of nonantisymmetry in the subtype relation. But now there are cases where you get an error in type argument inference because a pair of type has no common supertype.

interface I {
    a?: string;
    b?: string;
}

var i: I;

function foo<T>(p1: T, p2: T) { }

foo(i, { a: "" }); // Error because neither type is a supertype of the other

One interpretation is that in this case I should be the common supertype. To make this happen, we would say that instead of a missing property being a supertype of an optional, it is a subtype. In this view, missing and required are both specific cases of optional, and optional is a supertype of both.

Another fix is a change in type argument inference. If finding a common supertype does not yield a result, we can have another round where we use assignability. In the above example, this would not produce an error, since the object literal is assignable to I.

The issue here is that it may lead to results that depend on the order of the arguments. Both would select the first type in the following case:

interface I1 {
   a?: string
}
interface I2 {
   b?: string
}
var i1: I1, i2: I2;
foo(i1, i2);

Both proposed fixes would choose I1 because it is first, whereas now we at least get an error consistently no matter the argument order.

@JsonFreeman JsonFreeman added Question An issue which isn't directly actionable in code Breaking Change Would introduce errors in existing code Spec Issues related to the TypeScript language specification Bug A bug in TypeScript labels Dec 11, 2014
@sophiajt sophiajt added this to the TypeScript 1.5 milestone Jan 12, 2015
@ahejlsberg
Copy link
Member

@JsonFreeman The thing that gets us in trouble here is that we regard an object literal as having a complete type when in reality we only know about the properties that were actually specified.

Another way to think of it is that an object literal's type has a known set of required properties (those that were specified) and an infinite set of optional properties with any spelling and the value undefined. In that world view, the object literal { a: "" } has a type that is a subtype of { a?: string; b?: string; } because it has an phantom optional property b with the value undefined. An object literal's phantom optional properties would go away during widening (the same process that converts undefined and null types to the any type), so assigning { a: "" } to a variable would produce an inferred type of { a: string }.

This same process could be used as an alternative to #1774. We wouldn't need to copy missing properties from the contextual type as we could just rely on the phantom optional properties.

@JsonFreeman
Copy link
Contributor Author

If I understand you correctly, your point could be alternatively stated like the following. Given two types S and T, if p is an optional property in T that is missing in S, then:

  1. If S originated as an object literal, then S is a subtype of T (from the perspective of p)
  2. If S did not originate as an object literal, then T is a subtype of S (from the perspective of p)

The observation is that an object literal cannot express whether a property is optional or required, only if it is present or absent. So as long as it is present with the right type, or absent, it is fit to be a subtype of the optional.

The one thing I am concerned about is that in this scheme: Whether or not the type originated in an object literal can completely flip the relation. So consider the following:

interface WithOptional {
    x?: string;
}
var withOpt: WithOptional = {};
var withoutOpt1: {} = {};
var withoutOpt2 = {};
function foo<T>(p1: T, p2: T) { }
foo(withOpt, withoutOpt1).x; // error
foo(withOpt, withoutOpt2).x; // no error

withoutOpt1 has no properties, but withoutOpt2 has infinitely many optional properties of type undefined.

@JsonFreeman
Copy link
Contributor Author

Actually, reading your explanation again, I neglected the part about widening. So withoutOpt1 and withoutOpt2 actually would have the same type.

So the key difference between this and #1774 is that #1774 only happens when you contextually type, whereas this proposal happens when you don't widen. The latter happens in strictly more places I believe.

@JsonFreeman
Copy link
Contributor Author

One thing that occurs to me, is that the case where there are no inference candidates would give a different result with the new scheme:

interface WithOptional<T> {
    x?: T;
}
declare function foo<T>(p1: WithOptional<T>): T;
foo({}); // Used to infer {}, now we would infer any

interface WithOptionalAndUnion<T> {
     x?: T // primary
     y: string | T; // secondary
}
declare function bar<T>(p1: WithOptionalAndUnion<T>): T;
bar({ y: "" }); // Used to infer string, now we would infer any

@JsonFreeman
Copy link
Contributor Author

But all in all, I do think this is a good idea.

@JsonFreeman
Copy link
Contributor Author

There is a case I can think of that the contextual typing idea #1774 fixes, but this proposal (by itself) does not:

declare function foo(p: () => { x?: number }): number;
declare function foo(p: any): any;

var result = foo(() => { return {}; }); //#1774 gives number, this proposal gives any

@JsonFreeman
Copy link
Contributor Author

I thought about this some more, and discovered something very strange about this idea:

declare function foo<T>(p1: T, p2: T): T;
foo({ a: "" }, { });

Intuitively, this would infer { }. However, it sounds like your scheme would actually make this an error, because neither argument is a subtype of the other. This is because the object literal { } would actually have a type like { a?: undefined }. So it is not a supertype of { a: string }, because undefined is not a supertype of string. But it's not a subtype either because an optional cannot be a subtype of a required property.

@ahejlsberg
Copy link
Member

@JsonFreeman No, your example ends up inferring {} as you would expect. Forget about the "phantom" properties and think of it this way instead: An object literal type is a subtype of of a type T even if the object literal type is missing some of the optional properties in T, but once the object literal type is widened to a regular type this is no longer the case. In your example. It is effectively as if object literal types play by the old rules (pre-#919) before widening.

@ahejlsberg
Copy link
Member

@JsonFreeman I've got a fix for this in #1795.

@ahejlsberg ahejlsberg added Fixed A PR has been merged for this issue and removed Fixed A PR has been merged for this issue labels Jan 27, 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
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Fixed A PR has been merged for this issue Question An issue which isn't directly actionable in code Spec Issues related to the TypeScript language specification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants