Skip to content

For JSX Attributes, map over unions of props for contextual types#17790

Merged
weswigham merged 5 commits into
microsoft:masterfrom
weswigham:fix-jsx-union-contextual-type
Aug 22, 2017
Merged

For JSX Attributes, map over unions of props for contextual types#17790
weswigham merged 5 commits into
microsoft:masterfrom
weswigham:fix-jsx-union-contextual-type

Conversation

@weswigham
Copy link
Copy Markdown
Member

This way an individual attribute can fulfill any of them, rather than failing to get a contextual type unless it fulfills all of them.

Fixes #17427

@weswigham weswigham requested review from a user and sandersn August 14, 2017 23:45
Comment thread src/compiler/checker.ts Outdated
const attributeName = node.parent.name.escapedText;
if (attributesType.flags & TypeFlags.Union) {
// The attribute may fulfill any of the members of the union
return getUnionType(compact(map((attributesType as UnionType).types, t => getTypeOfPropertyOfType(t, attributeName))));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use mapDefined instead of compact(map()).

@ghost
Copy link
Copy Markdown

ghost commented Aug 15, 2017

Do you know why getPropertyOfType wasn't working for that? It appears to already have a special case for union types.

@weswigham
Copy link
Copy Markdown
Member Author

@Andy-MS Since it's a union and the property only exists on one option of the union, the property gets marked as CheckFlags.Partial, then omitted from the getPropertyOfType result. Acually, looking into it, I can just use getTypeOfSymbol on getUnionOrIntersectionProperty rather than what I've got here, which should also enable some cacheing of cacheable results.

@ghost
Copy link
Copy Markdown

ghost commented Aug 16, 2017

Is there any reason why JSX should differ from object literals here?

@weswigham
Copy link
Copy Markdown
Member Author

@Andy-MS Thanks for the prompt - I hadn't realized we were doing something slightly differently for non-jsx-related contextual types. Elsewhere we were using getTypeOfPropertyOfContextualType instead of getTypeOfPropertyOfType, which fully explains the difference in behavior (getTypeOfPropertyOfContextualType does the mapping I had inlined here). Now for all JSX contextual typing, we use getTypeOfPropertyOfContextualType, as we should.

InferenceShouldNotProduceAny({ data: 2, convert: n => "" + n });


const f1 = <InferenceShouldNotProduceAny data={"1"} />;
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.

This is tiny, but a positive name like InferenceShouldProduceNumber would be better.

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.

ShouldInferFromData, maybe

@weswigham weswigham merged commit 009d9b4 into microsoft:master Aug 22, 2017
@weswigham weswigham deleted the fix-jsx-union-contextual-type branch August 22, 2017 21:14
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 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