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

Intersection types and string literals #9410

Closed
Quantumplation opened this issue Jun 29, 2016 · 9 comments
Closed

Intersection types and string literals #9410

Quantumplation opened this issue Jun 29, 2016 · 9 comments
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@Quantumplation
Copy link
Contributor

TypeScript Version: 1.8.0

Code

// A *self-contained* demonstration of the problem follows...
class MyClass<T> {
  value: T & string
}

var myInstance: MyClass<"A" | "B">
myInstance.value = "A" // Case 1
myInstance.value = "B" // Case 2
myInstance.value = "other value" // Case 3

Expected behavior:
(prefix: I'm not sure if this was intentional or not, or whether it's a known issue or not, and I looked through other issues looking for mention of it and didn't see anything)

Case 1 and 2 should be valid, while case 3 should be an error.

Actual behavior:
All three cases are errors, as the intersection results in an empty type, rather than the type "A" | "B"
From one perspective, the type "string" is a super set of type "A" | "B".

This might also impact the recent work done in #9407

If this was intentional behavior, i'll edit this issue into a question as to why that decision was made, so it can serve as future documentation for anyone else searching for this issue.

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jun 29, 2016
@DanielRosenwasser
Copy link
Member

There are two things that you might consider issues.

The first is that "A" | "B" does not undergo subtype reduction in the intersection of ("A" | "B") & string. @ahejlsberg can answer better than I can here.

Another is that the way that we check the contextual type for the strings "A" and "B". We basically check if the contextual type for a string literal expression is a string literal type, or if it is a union where any constituent type is itself a string literal type.

In this case, you don't have a union with string literal types as constituents. You have an intersection type (with a constituent that is a union which has a string literal types as constituents). You could make the argument that we should be digging deeper into the contextual type, but I don't really think it makes sense. You can never satisfy a string literal type that is intersected with anything else, so really I consider subtype reduction to be the source of the issue.

@Quantumplation
Copy link
Contributor Author

Quantumplation commented Jun 29, 2016

You can never satisfy a string literal type that is intersected with anything else,

This definitely makes sense for intersecting it with anything other than other string literals,but what about this case?

var myVal: ("A" | "B") & ("A" | "C")
myVal = "A"

This also currently fails typechecking.

(Also, I should make a note that I fully acknowledge that these are rather niche corner cases of the type system :) the first one I pointed out does actually come up in a real world use case though.)

@ahejlsberg
Copy link
Member

I'm not quite sure what the intent of T & string is the original example since it will only produce a meaningful type when T itself is string. An intersection type X & Y conceptually represents a value that is both an X and a Y. Unlike union types, intersection types aren't always meaningful (e.g. a value can't be both a string and a number) and sometimes serve no other purpose than to reveal an illogical combination of types for diagnostic purposes. I think we may have a case of that here.

You could argue that we should perform supertype reduction for intersection types just like we perform subtype reduction for union types. Specifically, that we should reduce (A | B) & string to just A | B because string is a supertype of both. But I'm not really sure it would serve any meaningful purpose.

@Quantumplation
Copy link
Contributor Author

Quantumplation commented Jun 29, 2016

I'll elaborate a bit on our use case (which is, admittedly, a bit of type system abuse...)

For many of our types, they come in two varieties: design time view and runtime view. Thus, we might have the following two types:

class DesignTimeTuple<T> {
  _value: T
  _previousValue: T
  _isChanged: boolean
}

class Person {
  name: string
  salary: number
}
class DesignTimePerson {
  name: DesignTimeTuple<string>
  salary: DesignTimeTuple<number>
}

This pattern happens over and over, and in every case the properties on the type, and the underlying type that they take are identical, meaning we have two places to maintain the signature of this type.

One way to encode this to unify the two types would be:

class Person {
  name: string | DesignTimeTuple<string>
  salary: number | DesignTimeTuple<number>
}

However, this doesn't enforce that all properties are either runtime values or design time values (i.e. you can mix and match). Similarly, a function can't indicate that it plans to return the design time view.

The trick we came up with is to have something like the following:

/* Runtime needs to be restricted to a set of primitive types we support so that DesignTimeTuple below isn't considered a valid runtime type */
type Runtime = string | number | boolean
class DesignTime {
  private token: "token" // To make DesignTime only apply to things that extend DesignTime
}

class DesignTimeTuple<T extends Runtime> extends DesignTime {
  _value: T
  _previousValue: T
  _isChanged: boolean
}
type Value<T,U> = (T | DesignTimeTuple<T>) & U // <--

class Person<U extends Runtime | DesignTime> {
   name: Value<string, U>
   salary: Value<number, U>
}

This means that U effectively restricts the type (T | DesignTimeTuple<T>) to either the left or right type.

This works great, up until we have something like Value<"A" | "B", U>, in which case ("A" | "B") & Runtime doesn't restrict to ("A" | "B")

We're still debating, as a team, whether this abuse of the type system is worth the safety it provides, or whether it just overcomplicates things; however, it was something that surprised us, so we decided to raise it here in case it was actually a case that had been overlooked.

@jcalz
Copy link
Contributor

jcalz commented Apr 24, 2018

The particular case in the OP seems to have been addressed a while ago. Now:

declare var myInstance: MyClass<"A" | "B">;
myInstance.value = "A"; // okay as desired
myInstance.value = "B"; // okay as desired
myInstance.value = "other value"; // error as desired

@mhegazy
Copy link
Contributor

mhegazy commented Apr 24, 2018

Though it has no effect on the scenario in the OP, the type of myInstance.value is still "A" & string. it should be just "A".

@jcalz
Copy link
Contributor

jcalz commented Apr 24, 2018

I agree. I have asked for such things before, and the reply was something like it's not a big deal, as long as concrete values behave properly. And I see the point of that. If "A" & string acts like "A" everywhere, then not doing the reduction is more of a cosmetic issue than a bug. Of course, in #23651 I've found what looks like a problem, since "A" & string demonstrably acts differently from "A" there.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 24, 2018

I have asked for such things before,

We have come around on this issue a bit since you first filed it :D
We have been evolving our definition of intersection as we add more type operators. but we are trying to tread lightly here to avoid breaking existing use cases. I think literal & strings as well as literal & enum Literal should be safe to reduce.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 28, 2018

Should be fixed by #23751

@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed In Discussion Not yet reached consensus labels Apr 28, 2018
@mhegazy mhegazy closed this as completed Apr 28, 2018
@mhegazy mhegazy added this to the TypeScript 2.9 milestone Apr 28, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants