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

Error on type inference inside an IF and arrow function when using --scrictNullChecks #19606

Closed
lmcarreiro opened this issue Oct 31, 2017 · 5 comments · Fixed by #56908
Closed
Labels
Duplicate An existing issue was already created

Comments

@lmcarreiro
Copy link

lmcarreiro commented Oct 31, 2017

TypeScript Version: 2.7.0-dev.20171031

Code

declare function f(a: string, c: string): void;

function f2(a?: string, b?: string[]): void {
    if (a && b) {
        b.forEach( //b: string[]
            c => f(a, c) //a: string (OK)
        );
    }
}

function f3(a: string|undefined, b: string[]|undefined): void {
    if (a && b) {
        b.forEach( //b: string[]
            c => f(a, c) //a: string (OK)
        );
    }
}

function f4(x: any): void {
    let a: string|undefined = x.getA();
    let b: string[]|undefined = x.getB();
    if (a && b) {
        b.forEach( //b: string[]
            c => f(a, c) //a: string|undefined (should be just string)
        );
    }
}

function f5(x: any): void {
    let a: string|undefined = x.getA();
    let b: string[]|undefined = x.getB();
    if (a && b) {
        f(a, b[0]) //a: string (OK)
    }
}

function f6(x: any): void {
    let a: string|undefined = x.getA();
    let b: string[]|undefined = x.getB();
    if (a) {
        let d: string = a.toLowerCase();//a: string (OK)
        if (b) {
            b.forEach( //b: string[]
                c => f(a, c) //a: string|undefined (should be just string)
            );
        }
    }
}

Expected behavior:
No error, a: string|undefined should be string inside a if (a && b)

Actual behavior:
error TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
Type 'undefined' is not assignable to type 'string'.

@lmcarreiro lmcarreiro changed the title Error on type inference inside an IF when using --scrictNullChecks Error on type inference inside an IF and arrow function when using --scrictNullChecks Oct 31, 2017
@ghost ghost added the Duplicate An existing issue was already created label Oct 31, 2017
@ghost
Copy link

ghost commented Oct 31, 2017

Looks like a duplicate of #9998

A simpler example:

declare function getA(): string | undefined;
function g() {
    let a: string | undefined = getA();
    if (a) {
        return ["1", "2"].map(x => a.toLowerCase() + x);
    }
}

The problem with closures is that we don't know whether it's called immediately or stored for later. If the closure were stored for later and a were mutated to undefined later then the code would be wrong.
In your case you don't seem to be mutating a at all, so you can use const instead of let to fix the problem.

@lmcarreiro
Copy link
Author

lmcarreiro commented Oct 31, 2017

Thanks... It would be nice if we could define in the function type parameter if it would execute sync or async:

interface JQueryStatic {
  ajax(url: string, async callback: (response: any) => void);
  each(arr: any[], sync callback: (obj: any) =>void);
}
//or
interface JQueryStatic {
  ajax(url: string, +callback: (response: any) => void);
  each(arr: any[], -callback: (obj: any) =>void);
}

@ghost
Copy link

ghost commented Oct 31, 2017

That's #11498

@lmcarreiro
Copy link
Author

so you can use const instead of let to fix the problem.

@andy-ms, in the case of readonly properties, shouldn't it work too (like const)?

class X {
    readonly a: string | undefined;
    m() {
        if (this.a) {
            this.a;         // string - OK
            () => this.a;   // string|undefined - Shouldn't be string ?
        }
    }
}

@ghost
Copy link

ghost commented Nov 18, 2017

See "Mitigating with readonly fields" in #9998

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 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

Successfully merging a pull request may close this issue.

1 participant