Skip to content

Flow control resets guarded variables in sub function scope #8367

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
kitsonk opened this issue Apr 29, 2016 · 3 comments
Closed

Flow control resets guarded variables in sub function scope #8367

kitsonk opened this issue Apr 29, 2016 · 3 comments
Labels
Duplicate An existing issue was already created

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Apr 29, 2016

TypeScript Version:

nightly (1.9.0-dev.20160429)

Code

function isFoo(value: any): value is Foo {
    return value && typeof value.foo === 'function';
}

interface Foo {
    foo(): string;
}

interface Bar {
    bar(): string;
}

function convertBar(input: Foo | Bar): () => string {
    return isFoo(input) ? function () {
        return input.foo(); // <-- Property 'foo' does not exist on type 'Foo | Bar'.
    } : input.bar;
}

Expected behavior:

Receive no errors (as per 1.8)

Actual behavior:

Receive the error Property 'foo' does not exist on type 'Foo | Bar'.

@weswigham
Copy link
Member

Technically, this dupes the first half of #8270 but since that was closed as the second issue the user in that thread mentioned was fixed (and he got a workaround for this issue), this issue may be appropriate.

@ahejlsberg 's prior comment on this:

Regarding the first issue, control flow analysis doesn't consider a type guard to hold across a callback (because the callback could happen at any time, including outside the guarded block). This is different and more conservative from what we've previously done, and we're certainly open to feedback about whether it is too conservative. Meanwhile, you probably want to capture the guarded variable in a local and then reference that in the callback:

@kitsonk
Copy link
Contributor Author

kitsonk commented Apr 29, 2016

Thanks. Since this is "clean" of the other issue, maybe it makes sense to keep this open, and for now I will feedback here.

From a feedback perspective it was surprising to me that the guard didn't hold (and a breaking change in behaviour). While it is true that the callback can occur at any point, in the above use case, any conservatism is "wasted". Also, technically input could be anything by the time the return function gets called, including not being type Foo | Bar. The workaround based on the above would be something like (though obviously there are many ways):

function convertBar(input: Foo | Bar): () => string {
    let foo: () => string;
    if isFoo(input) {
        foo = input.foo;
    }
    return foo ? function () {
        return foo();
    } : input.bar;
}

To me that just seems awkward and intent a whole lot less clear. What I have done for now (which it always pains me to strong arm the compiler):

function convertBar(input: Foo | Bar): () => string {
    return isFoo(input) ? function () {
        return (<Foo> input).foo(); // <-- I am sure I made a angel weep for doing this
    } : input.bar;
}

@kitsonk kitsonk changed the title Flow control issue when using new scope regression Flow control resets guarded variables in sub function scope Apr 29, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Apr 29, 2016

this is already covered in #7662.

@mhegazy mhegazy closed this as completed Apr 29, 2016
@mhegazy mhegazy added the Duplicate An existing issue was already created label Apr 29, 2016
@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
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants