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

Typescript type checks passing a callback with non-optional parameter for optional one, but throws error at runtime #18466

Closed
afroozeh opened this issue Sep 14, 2017 · 7 comments
Labels
Fixed A PR has been merged for this issue Question An issue which isn't directly actionable in code

Comments

@afroozeh
Copy link

TypeScript Version: 2.4.1

Code

function f(callback: (a: string, b?: string) => void) {
    callback("a", undefined);
}

f((a: string, b: string) => {
    console.log(a.length, b.length);
});

Expected behavior:
Compile error with when compiled with the strict flag.
(a: string, b: string) => void is not a subtype of (a: string, b?: string) => void as string is not a supertype of string | undefined, I expected a compile error.

Actual behavior:
Compiles, but throws error at runtime

console.log(a.length, b.length);
                       ^

TypeError: Cannot read property 'length' of undefined
    at /Users/Ali/test.js:11:28
    at f (/Users/Ali/test.js:3:5)
    at Object.<anonymous> (/Users/Ali/test.js:10:1)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:389:7)
@kitsonk
Copy link
Contributor

kitsonk commented Sep 14, 2017

When you include the type annotations, TypeScript evaluates the assignability of the parameter types. string is a subtype string | undefined, therefore b is assignable to b. If you omit the parameter type annotations, type inference works and the last example errors as expected with strictNullCheck:

function f(callback: (a: string, b?: string) => void) {
    callback("a", undefined);
}

const cb = (a: string, b: string) => {
    console.log(a.length, b.length);
}

f(cb);

f((a, b) => {
    console.log(a.length, b.length);
});

@afroozeh
Copy link
Author

I'm new to TypeScript, but I expected that when determining if a function is a substitute for another one, its parameters should be more general (contra-variant on input parameters and co-variant on the output). Why TypeScript does not adhere to this rule?

@kitsonk
Copy link
Contributor

kitsonk commented Sep 14, 2017

TypeScript contravariance and covariance of arguments is likely the biggest issue with TypeScript that doesn't have a clean solution. There are several issues open for this and the conversations is rather extensive and exhaustive. Needless to say, it isn't like other type systems.

The FAQ does a decent job of covering off how the type system behaves. This sort of falls under Why are functions with fewer parameters assignable to functions that take more parameters.

I will admit I only have semi-good grasp of covariance/contravariance. I am sure some of the core team though can explain it better than I and why it is the way it is.

@acutmore
Copy link
Contributor

Following on from @kitsonk - The part of the FAQ which explains this behaviour is why-are-function-parameters-bivariant

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Sep 14, 2017
@afroozeh
Copy link
Author

afroozeh commented Sep 15, 2017

Thanks for the link to the FAQ. I was a bit confused, I guess structural typing adds another level of complexity.

For return values of functions, do you consider them only co-variant. Is it true?

@acutmore
Copy link
Contributor

@afroozeh correct. Return values are treated as co-variant.

interface Pet {
    feed(): void;
}

interface Cat extends Pet {
    purr(): void;
}

declare function getPet(): Pet;
declare function getCat(): Cat;

const p : Pet       = getCat(); // type 'Cat' is assignable to type 'Pet'
const gC: () => Pet = getCat;   // type '() => Cat' is assignable to type '() => Pet'

const c : Cat       = getPet(); // ERROR: Type 'Pet' is not assignable to type 'Cat'
const gp: () => Cat = getPet;   // ERROR: Type '() => Pet' is not assignable to type '() => Cat'

@lihz6
Copy link

lihz6 commented Sep 16, 2017

@afroozeh I stand for your point. And @kitsonk , I think the issue here has nothing to with the length of the parameters, but with the type of the parameters. I will show you how the tsc confuses itself when it comes function type assignments.

First, let's see when it does the right thing:

// the wrapper function ensures tsc doesn't infer the real type of strOrUndefined beforehand.
function wrapper(strOnly: string, strOrUndefined: string | undefined) {
    strOnly = strOrUndefined; // error, because undefined is not assignable to string
    strOrUndefined = strOnly; // fine, because string is subtype of string | undefined
}

But there is a way to trick the compiler failing to detect the error of strOnly = strOrUndefined;. But the compiler should not fall for the trick:

// the wrapper function ensures tsc doesn't infer the real type of strOrUndefined beforehand.
function wrapper(strOnly: string, strOrUndefined: string | undefined) {
    // strOnly = strOrUndefined; // error, because undefined is not assignable to string
    const func1 = function (str: string): void {
        strOnly = str;// NO ERROR! but str actualy points to strOrUndefined!
    }
    const func2: (str: string | undefined) => void = func1;

    func2(strOrUndefined);// what actually got executed is func1(strOrUndefined);
}

Obviously, there is a problem here. And the problem lies on const func2: (str: string | undefined) => void = func1;. How come?

When we assign (str: string) => void type to (str: string | undefined) => void type, it seems we are assigning string type to string | undefined, but it is not the case at all. We are doing the opposite, we are assigning string | undefined type to string type! This is why things go wrong.

Why are we doing the opposite by this manner? Because when it comes functions, the real assignment doesn't take place until the functions get executed. Let's see what really happened when func2 got executed. func2 got the parameter of string | undefined type BUT passing it as string type to func1! This is equal to assign string | undefined to string. This is the real problem.

This is absolutely a bug of the compiler, a really misleading one.

@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Sep 21, 2017
@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
Fixed A PR has been merged for this issue Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

6 participants