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

Mapped types break for-in loops with intersections #13023

Closed
gcnew opened this issue Dec 19, 2016 · 10 comments
Closed

Mapped types break for-in loops with intersections #13023

gcnew opened this issue Dec 19, 2016 · 10 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@gcnew
Copy link
Contributor

gcnew commented Dec 19, 2016

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;
}
@ahejlsberg
Copy link
Member

This is working as intended, the fact that it was previously allowed was a bug. A (T & U)[keyof T] is assignable to a T[keyof T], but not vice versa. Consider:

type A = {
    a: string;
    x: { s: string };
}
type B = {
    b: string;
    x: { t: string };
}

The intersection A & B corresponds to:

type AB = {
    a: string;
    b: string;
    x: { s: string, t: string };
}

Neither property x from A or B is assignable to property x in A & B because of the deep merging.

@ahejlsberg ahejlsberg added the Working as Intended The behavior described is the intended behavior; this is not a bug label Dec 19, 2016
@gcnew
Copy link
Contributor Author

gcnew commented 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
Copy link
Contributor Author

gcnew commented 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
Copy link
Contributor Author

gcnew commented Dec 19, 2016

The What's new documentation on intersection types features the very same extend example - 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 of Object.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
Copy link
Member

@gcnew If you want "shallow intersections" following the logic of Object.assign, you want spread types, and not really intersections.

@gcnew
Copy link
Contributor Author

gcnew commented Dec 19, 2016

@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 @ahejlsberg made is world breaking not only for me.

@ahejlsberg
Copy link
Member

The original PR on intersection types, #3622, has all of the semantics described in a fair amount of detail.

@gcnew
Copy link
Contributor Author

gcnew commented Dec 20, 2016

@ahejlsberg Yes, #3622 has the semantics described, however they are:

  1. not really intelligible for average programmers
  2. they are not covered in What's new / the TS book
  3. even there the extends example is wrong
  4. the TS team continuously used intersections in shallow situations

The XYZ example 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
Copy link
Contributor

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
Copy link
Contributor Author

gcnew commented 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 extend which 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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants