Skip to content

"is not assignable to" even after all necessary discriminated union checks are made #32390

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
angelo-mollame opened this issue Jul 13, 2019 · 5 comments
Labels
Duplicate An existing issue was already created

Comments

@angelo-mollame
Copy link

TypeScript Version: 3.5.1

Search Terms: is not assignable to, discriminated union

Code

export const enum PatternKind {
    Known = "Known",
    Unknown = "Unknown"
}

export type Pattern1 = {
    kind: PatternKind.Known;
    name: "Pattern1";
    pattern1_attribute: string;
};

export type Pattern2 = {
    kind: PatternKind.Known;
    name: "Pattern2";
    pattern2_attribute: string;
};

export type KnownPattern = Pattern1 | Pattern2;

export type UnknownPattern = {
    kind: PatternKind.Unknown;
    name: string;
};

export type Pattern = KnownPattern | UnknownPattern;

function getPattern(): Pattern {
    // TODO: implement
    throw new Error("");
}

const pattern: Pattern = getPattern();

if (pattern.kind === PatternKind.Known) {
    if (pattern.name === "Pattern1") {
        // Error, pattern1_attribute is not accessible
        pattern.pattern1_attribute = "foo";

        // Error, pattern is not assignable to Pattern1
        let pattern1: Pattern1 = pattern;
        pattern1 = pattern1;
    }
}

if (pattern.kind === PatternKind.Known) {
    // But for some reason, storing pattern in a KnownPattern variable first makes it work, even
    // though after the "pattern.kind === PatternKind.Known" above, the compiler is already
    // recognizing pattern to be a KnownPattern (which is why pattern can be assigned to the
    // KnownPattern variable by the way)
    const knownPattern: KnownPattern = pattern;

    if (knownPattern.name === "Pattern1") {
        // No error, pattern1_attribute is accessible
        knownPattern.pattern1_attribute = "foo";

        // No error, knownPattern is assignable to Pattern1
        let pattern1: Pattern1 = knownPattern;
        pattern1 = pattern1;
    }
}

Expected behavior: no compiler errors

Actual behavior: compiler errors

Playground Link:
http://www.typescriptlang.org/play/#code/KYDwDg9gTgLgBAYwgOwM72MgrgWzgBQEMYZgpkBpAS2QBM4BvAKDlbguQgHdk4BeOACIO3ZIIA0LNgFVkAa049+Q2QtGCmAXyZNQkWHBgBPMMALFS5AIzLmbOHJq0AXOZJlKTgHQieAbilWZEIcYFdBIndrQQD7MAsPKwB9CygqACMsUld0NOQAcwDNAN1waHhjUzdLZAAmW0CHJ1dImuo6H0VkWLZg0PDWj1qYxvioupSSNMzsuFyaQq0SvXLDEzNfZEHyZW3kGwAfaqHlsoNKs1UuvYb7RzoWhPJ22i8r0R6gkLC5mDzF4o6FbndbHHYCTY3I7vHh7EoAMywyAQMCoKDg+WAMD2AAoAJSPcaMRoAehJcAAKgB5AAiVNcVBwYAANsBQsgYI0YAALKDcODIYBcOAAUSgfKgOMEgjxRR0SDQ8DGNUJNWUmOxT2Q+JKVHhcBxyo8Xnu9D45rBng6mzxxPseoNRvIXj6ZnNAgiWqsMrt9jYZNF4ug4jgTv2kz+GSyZioqAFEHghAQCGAqFQGVZjTiWq8YeSqSjpGUgnhEAgIyz-vJYolIbDcFj8cTaao+WC6VZhgglqsldYrKVXtViWUYc+frzo694+02iYDsNOdN-AtexenVEtrsVbgACEsnBS1A5hBQnAoMBCKgUCH0NAFqGtQ3eIR2NcnwA3QhpQgdszwqgoHQOAcEIORUwbeAuGgOQQ2AD9MFJckeQgLB8m5OBCHhSxDG5MxBDDE0nBXAQ128TZBEw9IIAQkMeTMJAmSoVlj0bQhmQvQhaCMJDz2AJA2yoAAvB96xgbt0jMV9ISfHEuG5KgEAwxt5KMR8iQQQheEkzCWzbYB6HE3DgF4mSiS-H8-zgdI1PouAuEIIw8UaBVgLUWEtVcMy1QEMcdHtfUcXcrYc1dEihD2b0tz7OAAwAOW7Mha3Ump8ymQsYzjJMUxbP8YuCvZcy9CNpmjYtS3LEo-R3BK4CS4MHHfIk2L09tOyMyKYoHFLEmHaxlAKrVx2zcYbF86dGlnIA

@angelo-mollame angelo-mollame changed the title Invalid assignments even after all necessary discriminated union checks are made "is not assignable to" even after all necessary discriminated union checks are made Jul 13, 2019
@jack-williams
Copy link
Collaborator

This is a design limitation caused by the fact that discriminant narrowing only considers the declared type, rather than the current type of the identifier.

In your erroneous example you are relying on the composition of two discriminant narrowing's (pattern.kind and then pattern.name). In the other example you store the assigned the narrowed variable to a new identifier such that its declared type is the narrowed intermediate type (which is why that works).

Ideally this would just work as expected but I think the perf regressions would be significant.

@SamB
Copy link

SamB commented Jul 20, 2019

And we don't even need the type annotation on the new variable:

if (pattern.kind === PatternKind.Known) {
    // And this works even if we don't declare the type
    const knownPattern = pattern;

    if (knownPattern.name === "Pattern1") {
        // No error, pattern1_attribute is accessible
        knownPattern.pattern1_attribute = "foo";

        // No error, knownPattern is assignable to Pattern1
        let pattern1: Pattern1 = knownPattern;
        pattern1 = pattern1;
    }
}

Also, could you give a short description of exactly what would cause the performance regressions?

Like, would it lead to a lot of second-, third-, etc. guessing during type inference? What of and/or where would it happen?

@jack-williams
Copy link
Collaborator

Synthesising the type for a property on a union, such as pattern.kind, is expensive and happens at each discriminant check. Caching makes this bearable, but you need to make sure that you consistently hit the cache - TypeScript does this by always narrowing from the declared type.

@RyanCavanaugh
Copy link
Member

Tracked by #30557

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jul 31, 2019
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants