Return types on Promise #806

Activity

1. teppeis commented on Dec 10, 2018

   

   teppeis

   on Dec 10, 2018

   Contributor

   More actions

   #689 introduced the type definition.

   // TODO(lucassloan): goog.Promise has bad types that are coerced to any, so explicitly emit any
   // and change to the proper type (value: googPromise< T , any > | T): googPromise<T, any>
   // when the callers have been fixed.

   I don't know why it has to return any and what the "when the callers have been fixed" means. (about Google internal code?)
   I think goog.Promise and goog.Thenable should be fixed like:

   declare namespace ಠ_ಠ.clutz.goog {
     class Promise < TYPE , RESOLVER_CONTEXT = void > implements ಠ_ಠ.clutz.goog.Thenable < TYPE > {
       constructor (resolver : (this : RESOLVER_CONTEXT , a : (a ? : TYPE | PromiseLike < TYPE > | null | { then : any } ) => any , b : (a ? : any ) => any ) => void , opt_context ? : RESOLVER_CONTEXT ) ;
       then < RESULT > (opt_onFulfilled ? : ( (a : TYPE ) => PromiseLike < RESULT > | RESULT ) | null , opt_onRejected ? : ( (a : any ) => any ) | null) : ಠ_ಠ.clutz.goog.Promise < RESULT > ;
       static resolve < T > (value : PromiseLike < T > | T ): ಠ_ಠ.clutz.goog.Promise < T > ;
     }
   }

   sample code in playground

   @rkirov @LucasSloan How about this? Can I send PR to fix it?
2. teppeis commented on Jan 9, 2019

   

   teppeis

   on Jan 9, 2019

   Contributor

   More actions

   The externs for IThenable::then, Promise::catch and Promise::then in Closure Compiler were changed, adding REJECTED_VALUE to the template.
   google/closure-compiler@b3df223
3. rkirov commented on Jan 16, 2019

   

   rkirov

   on Jan 16, 2019

   Contributor

   More actions

   @teppeis Thanks for looking into this. Yes, the comment refers to google internal code, which is the primary consumer of clutz generated .d.ts files. Having 'any's was a temporary solution as we were working through getting --partial working and indeed there are many possible improvements.

   Unfortunately, each improvement has to be weighted against how much current violations we have in our giant codebase and how much work it is to roll it out.

   Always feel free to send me a PR and I will do that evaluation and let you know. Sometimes the answer would be that it would be prohibitively hard. I just did that for #830 and I can tell you that it is fine. Likely changing the return from ':any' to a proper type would not be, but again if you send me a PR I would follow up with describing common failure patterns. Often people forget that in real world codebases, users can mix up to 4 types of promises:

   • TS Promises
   • JS promises (translated through clutz)
   • AngularJS promises described by angularjs.d.ts
   • AngularJS promises (described by externs translated through clutz).

   They are mostly runtime compatible (at least uses expect them to), so any change that makes them type-checking incompatible runs afoul of lots of existing code.