Description

Cliff Koh (cliffkoh)

opened on Sep 11, 2017

Issue body actions

TypeScript Version: 2.4.2 / Typescript playground

Code

export interface Storage {
    getItem<T>(onComplete?: OnComplete<T>): Promise<T>;
}

export type OnComplete<T> = (err?: any, result?: T) => any;

let storage: Storage = {
    getItem: (onComplete: OnComplete<string>) => {
        return Promise.resolve("Yay");
    }
};

Typescript playground link:
https://www.typescriptlang.org/play/#src=export%20interface%20Storage%20%7B%0D%0A%20%20%20%20getItem%3CT%3E(onComplete%3F%3A%20OnComplete%3CT%3E)%3A%20Promise%3CT%3E%3B%0D%0A%7D%0D%0A%0D%0Aexport%20type%20OnComplete%3CT%3E%20%3D%20(err%3F%3A%20any%2C%20result%3F%3A%20T)%20%3D%3E%20any%3B%0D%0A%0D%0Alet%20storage%3A%20Storage%20%3D%20%7B%0D%0A%20%20%20%20getItem%3A%20(onComplete%3A%20OnComplete%3Cstring%3E)%20%3D%3E%20%7B%0D%0A%20%20%20%20%20%20%20%20return%20Promise.resolve(%22Yay%22)%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%3B%0D%0A

Expected behavior:
Code snippet above works

Actual behavior:
Ultimately, string is not assignable to type T on Typescript playground. On 2.4.2, I get string is not assignable to T | undefined. Both makes no sense...

Activity

1. aluanhaddad commented on Sep 11, 2017

   

   Aluan Haddad (aluanhaddad)

   on Sep 11, 2017

   Contributor

   More actions

   That is the correct behavior. The interface declares a generic method getItem<T> method which returns a different type of value depending on what you pass to it. The object you are attempting to implement that interface with does not meet that requirement.
2. ahejlsberg commented on Sep 11, 2017

   

   Anders Hejlsberg (ahejlsberg)

   on Sep 11, 2017

   Member

   More actions

   This is working as intended and is an effect of #16368. The type checker is pointing out that it is not safe to treat the type { getItem: (onComplete: OnComplete<string>) => Promise<string>; } as the more general type { getItem<T>(onComplete?: OnComplete<T>): Promise<T>; }. Basically, you can't treat a method that operates on the specific type string as a method that operates on any type T.

   I suspect you probably want to write the code this way:

   export interface Storage<T> {
       getItem(onComplete?: OnComplete<T>): Promise<T>;
   }
   
   export type OnComplete<T> = (err?: any, result?: T) => any;
   
   let storage: Storage<string> = {
       getItem: (onComplete: OnComplete<string>) => {
           return Promise.resolve("Yay");
       }
   };

   Reacted by Aluan Haddad
3. Anders Hejlsberg (ahejlsberg)

   added

   Working as IntendedThe behavior described is the intended behavior; this is not a bugThe behavior described is the intended behavior; this is not a bug

   on Sep 11, 2017
4. cliffkoh commented on Sep 11, 2017

   

   Cliff Koh (cliffkoh)

   on Sep 11, 2017

   Author

   More actions

   Thank you for the explanation. I hit this problem with the definition of Storage as defined here: https://github.com/rt2zz/redux-persist/blob/master/type-definitions/index.d.ts

   I will submit a PR for them to fix per your suggestion Anders Hejlsberg (@ahejlsberg).

   Also, for the longest time, I've been confused about what it means when <T> is on a method but not on the top-level interface. Seeing the amount of similar mistakes including for typings on React-Redux in the past, perhaps the team could consider augmenting the Typescript documentation on generics to explain this particular case as well?

   https://www.typescriptlang.org/docs/handbook/generics.html

   Thank you again for the explanation.
5. Cliff Koh (cliffkoh)

   closed this as completedon Sep 11, 2017
6. ahejlsberg commented on Sep 11, 2017

   

   Anders Hejlsberg (ahejlsberg)

   on Sep 11, 2017

   Member

   More actions

   Cliff Koh (@cliffkoh) Just FYI, the Storage type in that definition file is a mess. It has type parameters that are never used at all (which are pointless) and type parameters that never occur in a parameter position (which means you must always explicitly specify them).

   Reacted by Aluan Haddad
7. cliffkoh commented on Sep 13, 2017

   

   Cliff Koh (cliffkoh)

   on Sep 13, 2017

   Author

   More actions

   Anders Hejlsberg (@ahejlsberg) Thanks for the FYI. Because of your FYI I took a deeper look and have updated the PR to hopefully addressed the concerns you expressed above (link to PR above), in the interests of the entire Typescript community having better typings to work with.

   Mostly removed the use of generics bar the generics expressed in Mozilla's localForage library for their Storage API (under LocalForageDbMethods): https://github.com/localForage/localForage/blob/master/typings/localforage.d.ts
8. microsoft

   locked and limited conversation to collaborators on Jun 14, 2018

Issue actions