Skip to content

Can't partial a generic type with extension constraint #13442

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
markboyall opened this issue Jan 12, 2017 · 7 comments
Closed

Can't partial a generic type with extension constraint #13442

markboyall opened this issue Jan 12, 2017 · 7 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@markboyall
Copy link

TypeScript Version: 2.1.5

export function apply<T>(existing: T) {
    return {
        changes: (changes: Partial<T>): T => {
            return null;
        }
    };
}

function applyVisibility<SectionVisibility, Document extends { sectionVisibility: SectionVisibility }>(document: Document, visibilityUpdates: SectionVisibility): Document {
    const sectionVisibility = apply(document.sectionVisibility).changes(visibilityUpdates);
    return apply(document).changes({ sectionVisibility: sectionVisibility });
}

Expected behavior:

Compiles.

Actual behavior:

Doesn't compile. The type { sectionVisibility: SectionVisibility } is not assignable to Partial, despite the fact that we clearly constrained Document to include this property, and it's Partial so we know that every other property is optional.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jan 12, 2017
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 12, 2017

The error is correct. We know that Document doesn't require more than sectionVisibility, but there's no guarantee that a given instantiation of applyVisibility (i.e. an actual call site) doesn't require the type of sectionVisibility to be a subtype of the declared type due to Document being a more-specific type.

@markboyall
Copy link
Author

markboyall commented Jan 12, 2017

I'm not wholly sure that I understand. Presumably for the specific type that Document eventually is, that mandates the selection of SelectionVisibility to be that property's type. So the visibilityUpdates parameter must be of that same type (or extending it), which should be legal. Could you show an example of the error case you're describing?

@RyanCavanaugh
Copy link
Member

A simpler example is helpful, since Partial isn't actually required to understand the behavior:

interface Point2d { x: number, y: number };
interface Point3d { x: number, y: number, z: number };

function foo<T, U extends { prop: T }>(a: T, b: U): U {
    return { prop: a }; // Disallowed, see next comment for why
}
var p2: Point2d, p3: Point3d;
// m: { prop: Point3d } m.prop.z does not actually exist
let m = foo(p2, { prop: p3 });

@markboyall
Copy link
Author

Yeah, OK, that is nasty.

@markboyall
Copy link
Author

markboyall commented Jan 23, 2017

Wait, doesn't this present a massive hole in the structural typing in general, regardless of any generics?

interface Point2d { x: number, y: number };
interface Point3d { x: number, y: number, z: number };

function foo(a: Point2d, b: { prop: Point2d }): void {
  b.prop = a;
}
var p2: Point2d, p3: Point3d;
var original = { prop: p3 };
foo(p2, original);
// original: { prop: Point3d } original.prop.z does not actually exist

Or to put it another way, your example can be trivially modified to compile and still break the type system

interface Point2d { x: number, y: number };
interface Point3d { x: number, y: number, z: number };

function foo<T, U extends { prop: T }>(a: T, b: U): U {
  b.prop = a;
  return b;
}
var p2: Point2d, p3: Point3d;
// m: { prop: Point3d } m.prop.z does not actually exist
let m = foo(p2, { prop: p3 });

Seems to me like unless the property is readonly, you must assume that the function could write to it, and so can't permit covariance here.

@RyanCavanaugh
Copy link
Member

Absolutely; TypeScript is not 100% sound in that regard.

@markboyall
Copy link
Author

markboyall commented Jan 25, 2017

I don't suppose there are any plans to change this in the future? I'm suddenly becoming aware that there would be a lot of breakage going on if this were fixed.

I guess that although my original example is admittedly unsafe, I'm not sure that it's any more unsafe than the permitted examples. It seems odd to refuse to compile in this case to favour correctness over convenience when the correctness horse left the stable a long, long time ago.

If it's not going to be fixed and we favour convenience, then I'd say that the lack of convenience in this case is a bug.

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

3 participants