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

Fix #8423: Remove undefined while getting the type of the first argument of then signature #8449

Merged
merged 3 commits into from
May 4, 2016

Conversation

mhegazy
Copy link
Contributor

@mhegazy mhegazy commented May 3, 2016

Fixes #8423

@@ -13443,7 +13443,7 @@ namespace ts {
}

function getTypeOfFirstParameterOfSignature(signature: Signature) {
return getTypeAtPosition(signature, 0);
return getTypeWithFacts(getTypeAtPosition(signature, 0), TypeFacts.NEUndefined);
Copy link
Contributor

Choose a reason for hiding this comment

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

this function is used in two places

  • when we get type of first parameter in the call signature of then
  • when we get type of first parameter in the onFulfilled callback.

Do we want to strip undefined in both places or just in the first one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well. i went back and forth on this, and then did not see a reason why not...
so we want to get T

then<U>(success?: (value: T) => U,...): IPromise<U>

the first takes care of the optionality of succes?, the second would remove the undefined if value was optional. the question is what is the promised type for this:

then<U>(success?: (value?: T) => U,...): IPromise<U>

is it T or T|undefined ?

Copy link
Contributor

Choose a reason for hiding this comment

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

what about cases like this:

declare function resolve1<T>(value: T): Promise<T>;
declare function resolve2<T>(value: T): Windows.Foundation.IPromise<T>;

async function f(x?: number) {
    let x1 = await resolve1(x);
    let x2 = await resolve2(x);
}

Currently type of x1 is number | undefined and x2 fails with error addressed by this PR.
With the fix type of x2 will be just number which will be inconsistent with type for x1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. fixed. thanks

@vladima
Copy link
Contributor

vladima commented May 3, 2016

👍

@mhegazy mhegazy merged commit 24aabec into master May 4, 2016
@mhegazy mhegazy deleted the Fix8423 branch May 4, 2016 04:17
@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants