Skip to content

Set inference result to any instead of {} for .js files if generic type parameter inference found no candidates#14492

Merged
mhegazy merged 2 commits into
masterfrom
anyInferences
Mar 9, 2017
Merged

Set inference result to any instead of {} for .js files if generic type parameter inference found no candidates#14492
mhegazy merged 2 commits into
masterfrom
anyInferences

Conversation

@mhegazy
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy commented Mar 6, 2017

…c type parameter inference found no candidates
Comment thread src/compiler/checker.ts Outdated
context.inferredTypes[index] = inferredType = instantiatedConstraint;
}
}
if (context.useAnyForNoInferences && !inferences.length && inferredType === emptyObjectType) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why this logic when we're already doing similar checks above? I would just modify the inferredType = emptyObjectType assignment above to inferredType = context.usAnyForNoInferences ? anyType : emptyObjectType.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This means you never get the constraint if one exist, always get any, since any will always be assignable to any constraint. For foo<T extends number>(): T number seemed like a better result than any.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, I thought the new code was in a different place but I see now. Anyway, one issue is that you'll change any emptyObjectType into an any regardless of how that emptyObjectType came about. It might in fact have been produced by an actual inference.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

should be updated now.

@mhegazy mhegazy merged commit 0fb415a into master Mar 9, 2017
@mhegazy mhegazy deleted the anyInferences branch March 9, 2017 00:15
@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