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

Covariant checking for callback parameters #15104

Merged
merged 9 commits into from
Apr 18, 2017
Merged

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Apr 10, 2017

In order to ensure that any generic type Foo<T> is at least co-variant with respect to T no matter how Foo uses T, TypeScript relates parameters bi-variantly (given that parameters are input positions, they naturally relate only contra-variantly). However, when source and target parameters both have function types with a single call signature, we know we are relating two callback parameters. In that case it is sufficient to only relate the parameters of the signatures co-variantly because, similar to return values, callback parameters are output positions. With this PR we introduce that change. This means that a Promise<T> or Observable<T>, where T is used only in callback parameter positions, will be co-variant (as opposed to bi-variant) with respect to T, which solves a commonly reported issue.

This change finds several issues in our real world code suites that all appear to be legitimate inconsistencies.

Fixes #11022.
Fixes #14770.

@DanielRosenwasser DanielRosenwasser changed the title Covariant callbacks Covariant checking for callback parameters Apr 10, 2017
@ahejlsberg
Copy link
Member Author

@mhegazy Want to take a look?

@acutmore
Copy link
Contributor

acutmore commented May 5, 2017

@ShawnTalbert If not clear. This PR does not introduce any new syntax. Instead changes type assignability of generics where the type is only user in a function parameter position e.g Promise / Observable

interface Box<T> {
   foo(cb: (t: T => void)): void;
}

Before:

declare const bA: Box<Animal>;
const bC: Box<Cat> = bA; // no error 😿 

After:

declare const bA: Box<Animal>;
const bC: Box<Cat> = bA; // error 😺 

@ShawnTalbert
Copy link

Yes, wrong issue, sorry

@bmcbarron
Copy link

It appears that the solution was amended to ignore union with null/undefined. Should that be dependent on --strictNullChecks? At the moment, this does not seem to address #13513.

@ahejlsberg
Copy link
Member Author

ahejlsberg commented May 8, 2017

@bmcbarron #13513 is fixed by this PR in --strictNullChecks mode. Without --strictNullChecks, undefined is assignable to any type and is effectively ignored in union types, but that has always been the case and isn't going to change.

@schotime
Copy link

schotime commented Aug 17, 2017

This doesn't fix this problem unfortunately and this is usually the case if the callbacks are stored in different files and imported.

class Animal {}
class Cat extends Animal {
    public meow() {}
}

let promise: Promise<Animal> = null;
promise.then((cat: Cat) => { //Should error here but does not
  cat.meow();
});

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants