Repository navigation
Mapped types break for-in loops with intersections #13023
Description
TypeScript Version: nightly
The following used to work but is broken now.
Code
function extend<T, U>(base: T, ext: U): T & U {
const retval = <T & U> {};
for (const x in base) {
// Error: Type 'T[keyof T]' is not assignable to type '(T & U)[keyof T]'.
retval[x] = base[x];
}
for (const x in ext) {
// Error: Type 'U[keyof U]' is not assignable to type '(T & U)[keyof U]'.
retval[x] = ext[x];
}
return retval;
}
Activity
ahejlsberg commentedon Dec 19, 2016
This is working as intended, the fact that it was previously allowed was a bug. A
(T & U)[keyof T]is assignable to aT[keyof T], but not vice versa. Consider:The intersection
A & Bcorresponds to:Neither property
xfromAorBis assignable to propertyxinA & Bbecause of the deep merging.gcnew commentedon Dec 19, 2016
I agree that in a deep setting the new behaviour is the correct one, however it is also a breaking change introduced in version 2.1 and should be marked as such.
gcnew commentedon Dec 19, 2016
PS: shallow intersections seem to be more natural. Will they ever be supported? Is the deep behaviour actually used in RWC?
gcnew commentedon Dec 19, 2016
The What's new documentation on intersection types features the very same
extendexample - nothing is mentioned about a deep nature of&and the provided code doesn't handle it either. It was also used for the old definitions ofObject.assign(which is shallow). I find it a massive breaking change from a logical perspective. Maybe intersection types should be made shallow? It will be a fix for the current misleading/buggy behaviour.weswigham commentedon Dec 19, 2016
Marin Marinov (@gcnew) If you want "shallow intersections" following the logic of
Object.assign, you want spread types, and not really intersections.gcnew commentedon Dec 19, 2016
Wesley Wigham (@weswigham) Thank you!
The major problem I'm after is that basically all the information and marketing of intersection types are incomplete at best (leaning to wrong). This has created a flawed mental model for many of us.
The unfortunate state we are currently in should be very explicitly explained and fixed in the new documentation (even if the behaviour is changed to match the status-quo explanation). Also the TypeScript book and What's new should be retroactively updated.
I believe the revelation Anders Hejlsberg (@ahejlsberg) made is world breaking not only for me.
ahejlsberg commentedon Dec 20, 2016
The original PR on intersection types, #3622, has all of the semantics described in a fair amount of detail.
gcnew commentedon Dec 20, 2016
Anders Hejlsberg (@ahejlsberg) Yes, #3622 has the semantics described, however they are:
extendsexample is wrongThe
XYZexample is the only hint, except for Properties and Index Signatures.My proposal is to state the intended semantics clearly and add an appropriate notice on the pertinent resources. Otherwise people reading the official resources will still be led astray.
aluanhaddad commentedon Dec 20, 2016
I'm not sure I understand the notion of shallow intersections.
I would expect the intersection to be as deep as the deepest common path between between the types in question.
Maybe I'm missing something fundamental, but I don't see what else it could be.
gcnew commentedon Dec 20, 2016
It seems Flow has taken the shallow path with the left operand being a base type and the right one an extension. link
It is also what I had inferred form the TS documentation as nothing is said about intersections being deep and all the motivating examples being shallow. The inclusion of
extendwhich is infamously shallow (and also implemented this way) doesn't help either. I believe this has been the reason for the many issues asking why the right hand intersection type doesn't override the properties on the left one.From another perspective this is breaking the code in the official documentation. There are two routes of action, either the behaviour is changed to match the documentation, or the documentation is updated. In both cases there should be a notice clearing up the confusion and all the relevant parts updated.