-
Notifications
You must be signed in to change notification settings - Fork 60
Improve Promise and remove --noStrictGenericChecks #830
Conversation
+ " ) => " | ||
+ classTemplatizedType | ||
+ " | RESULT ) | null , " | ||
+ " ) => PromiseLike < RESULT > | RESULT ) | null , " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a clever solution. In the past we just expected all promise implementations to be structurally compatible, so one can say they expect then((...): ಠ_ಠ.clutz.goog.Thenable<T>, ...)
and expected that ಠ_ಠ.clutz.goog.Thenable<T>
is mutually assignable with PromiseLike<T>
.
However, we ran into issues when strictGenericChecks
was on - the two types were not mutually assignable. That's why is is off here and we have it off in the google codebase. I think with your change the types are getting more obviously assignable, so this might no longer be an issue.
Looks good, can you rebase and I will merge it. |
This is the issue that I was referring to. I am not sure if this is a TS bug or a proper feature, but notice that with You change doesn't affect this behavior because you are still keeping the return of the then as |
289cdbb
to
cee3c4a
Compare
merged |
--noStrictGenericChecks
in DeclarationSyntaxTest was introduced temporary in #689.This PR is minimum improvement of Promise to remove
--noStrictGenericChecks
fromDeclarationSyntaxTest
and the users environment.Both
goog.Thenable.prototype.then
andgoog.Promise.prototype.then
can receiveIThenable
(that is mapped toPromiseLike
in TypeScript) and this fix resolves the strict generic check error.https://github.com/google/closure-library/blob/e656c0807353188f42b67722d56724a5afc13228/closure/goog/promise/thenable.js#L47-L75
I can improve more, but I can not understand the meaning of that TODO comment, so I will leave other code as it is.