-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Fix generic return type inference #31680
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
Conversation
@typescript-bot run dt |
Heya @weswigham, I've started to run the extended test suite on this PR at c3ef035. You can monitor the build here. It should now contribute to this PR's status checks. |
Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at c3ef035. You can monitor the build here. It should now contribute to this PR's status checks. |
context.returnMapper = getMapperFromContext(cloneInferredPartOfContext(context)); | ||
const returnContext = createInferenceContext(signature.typeParameters!, signature, context.flags); | ||
const returnSourceType = instantiateType(contextualType, outerContext && outerContext.returnMapper); | ||
inferTypes(returnContext.inferences, returnSourceType, inferenceTargetType); |
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.
The extra invocation of inference here probably warrants a comment as to why it's needed, especially seeing as its removal affects only the one test just added. 😄
@typescript-bot test this again - afaik none of the auth tokens need to be refreshed for months, so maybe it was just a hiccup in the auth service? |
Heya @weswigham, I've started to run the extended test suite on this PR at c3ef035. You can monitor the build here. It should now contribute to this PR's status checks. |
@typescript-bot perf test |
Heya @ahejlsberg, I've started to run the perf test suite on this PR at c3ef035. You can monitor the build here. It should now contribute to this PR's status checks. Update: The results are in! |
@typescript-bot test this but now I've refreshed the credentials so if you don't work I'll know something's wrong. |
Heya @weswigham, I've started to run the extended test suite on this PR at c3ef035. You can monitor the build here. It should now contribute to this PR's status checks. |
@ahejlsberg Here they are:Comparison Report - master..31680
System
Hosts
Scenarios
|
@rbuckton this one platform/solution combination is consistently an outlier - do we know what's up with it yet? Its emit times (and only emit) are always mega high variance, comparatively speaking. |
Fixes #31443.