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

Improve Promise type definition #10448

Merged
merged 2 commits into from
Sep 14, 2016
Merged

Improve Promise type definition #10448

merged 2 commits into from
Sep 14, 2016

Conversation

rbuckton
Copy link
Member

@rbuckton rbuckton commented Aug 20, 2016

This change modifies the overloads for then and catch of Promise<T>, and the overloads for then of PromiseLike<T> to provide better type inference.

Fixes #4903

@rbuckton
Copy link
Member Author

@sandersn, @vladima: care to take a look?

@vladima
Copy link
Contributor

vladima commented Aug 22, 2016

👍

* Creates a new Promise with the same internal state of this Promise.
* @returns A Promise.
*/
then(): Promise<T>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought overloads should be arranged longest-to-shortest to make sure that the most restrictive overloads matches first. Why change that here?

@rbuckton
Copy link
Member Author

rbuckton commented Sep 6, 2016

The change in 9d4219a fixes an existing issue with assignability between Promise<T> and PromiseLike<T>. Unfortunately, this means that some cases in this change that should result in Promise<never> retain the T of the antecedent promise. This is a result of how we erase uninstantiated generic type parameters to any and how any compares to never. A more comprehensive solution to address this is out of scope of this PR, as this still provides an improvement over our existing type definition for Promise<T>.

@@ -8,42 +8,45 @@ interface Promise<T> {
* @param onrejected The callback to execute when the Promise is rejected.
* @returns A Promise for the completion of which ever callback is executed.
*/
then<TResult1, TResult2>(onfulfilled: (value: T) => TResult1 | PromiseLike<TResult1>, onrejected: (reason: any) => TResult2 | PromiseLike<TResult2>): Promise<TResult1 | TResult2>;
then(onfulfilled?: ((value: T) => T | PromiseLike<T>) | undefined | null, onrejected?: ((reason: any) => T | PromiseLike<T>) | undefined | null): Promise<T>;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overload is meant to replace then(): Promise<T> as that overload was one of the causes of #10524.

@rbuckton
Copy link
Member Author

rbuckton commented Sep 6, 2016

@sandersn, @vladima can you take another look at these typings? The order of the overloads is intentional and very specific to get the best typings with respect to never and how undefined | null are handled depending on the value of strictNullChecks.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 13, 2016

👍

@rbuckton
Copy link
Member Author

@mhegazy, do you want to take this change for release-2.0 as well?

@mhegazy
Copy link
Contributor

mhegazy commented Sep 13, 2016

no. jsut 2.1 please.

@rbuckton rbuckton merged commit 3e1da93 into master Sep 14, 2016
@rbuckton rbuckton deleted the improvePromiseType branch September 14, 2016 18:58
@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label Nov 4, 2016
@mhegazy mhegazy modified the milestones: TypeScript 2.1.2, TypeScript 2.1 Nov 4, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
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.

5 participants