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

Wrong type narrowing when types are compatibles #11664

Closed
akarzazi opened this issue Oct 15, 2016 · 7 comments
Closed

Wrong type narrowing when types are compatibles #11664

akarzazi opened this issue Oct 15, 2016 · 7 comments
Labels
Fixed A PR has been merged for this issue

Comments

@akarzazi
Copy link

akarzazi commented Oct 15, 2016

TypeScript Version: 2.0.3

Code

class Result1 {
    readonly value1: number;

    constructor(value1: number) {
        this.value1 = value1;
    }
}

class Result2 {
    readonly value1: number;
    readonly value2: number;

    constructor(value1: number, value2: number) {
        this.value1 = value1;
        this.value2 = value2;
    }
}

function someFunction(result: Result1 | Result2) {
    if (result instanceof Result1) {
        // result is still Result1 | Result2
        console.log("1");
    }
    else {
        // result is never ! this is wrong !
        console.log("2");
    }
}

// this will output 2 !
someFunction(new Result2(9,9))

Expected behavior:
instance of type guard should not care about type compatibility
Actual behavior:
Wrong assertion by the compiler.

@yortus
Copy link
Contributor

yortus commented Oct 16, 2016

Essentially a duplicate of #10934. See also design notes about instanceof in #8503.

@akarzazi
Copy link
Author

Indeed, this is a duplicate :(
From #7271, the issue will be fixed in 2.1.

@yortus
Copy link
Contributor

yortus commented Oct 16, 2016

@akarzazi note that the fix for #7271 only affects narrowing structurally identical types, but your issue here (like #10934) is with narrowing structural subtype/supertypes. This hasn't been solved yet and is on the roadmap for future investigation at an unspecified time.

@akarzazi
Copy link
Author

akarzazi commented Oct 16, 2016

@yortus I thought the fix will cover type compatibly as well.
Since mhegazy said : As noted in #8503, classes should be treated definitely for instanceof checks.

@mikelehen
Copy link

FYI- This still reproduces with TypeScript 2.3.2

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@nodefish
Copy link

Really simplistic example of what's wrong with instanceof:

const model = this.opts.resources.model.store.get(id);
if (!model || model instanceof Placeholder)
    return null;
const data: any = model.get();

TSC gives the following error:
Property 'get' does not exist on type 'never'.

A workaround was proposed in #13325 but I can't see an equivalent in imperative code sections.

@RyanCavanaugh RyanCavanaugh added Duplicate An existing issue was already created and removed Needs Investigation This issue needs a team member to investigate its status. labels Nov 17, 2017
@RyanCavanaugh
Copy link
Member

Fixed in #19671

@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed Duplicate An existing issue was already created labels Nov 20, 2017
@mhegazy mhegazy added this to the TypeScript 2.7 milestone Nov 20, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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
Projects
None yet
Development

No branches or pull requests

6 participants