Repository navigation
Discriminate jsx contextual types same as object contextual types - #27408
Wesley Wigham (weswigham) merged 5 commits into
Conversation
|
Might fix #27211 as well? |
| } | ||
|
|
||
| /** | ||
| * Woah! Do you really want to use this function? |
There was a problem hiding this comment.
Pretty sure it's spelled "Whoa" Daniel Rosenwasser (@DanielRosenwasser)
| return contextualType; | ||
| } | ||
| function discriminateContextualTypeByObjectMembers(node: ObjectLiteralExpression, contextualType: UnionType) { | ||
| // Keep the below up-to-date with the work done within `isRelatedTo` by `findMatchingDiscriminantType` |
There was a problem hiding this comment.
Since this applies to two functions now, enumerate the functions.
Actually, if you can extract the core of this algorithm and call it, that would be better. I'm not sure what you can do with that hilarious use of break propLoop in there. Maybe that could become a return?
Nathan Shively-Sanders (sandersn)
left a comment
Nathan Shively-Sanders (sandersn)
left a comment
There was a problem hiding this comment.
I like this fix, but can you see whether removal of some of the duplication is possible? Otherwise we'll now have 3 copies of this one weird algorithm.
|
Looks like there is some assertion lint. |
Nathan Shively-Sanders (sandersn)
left a comment
Nathan Shively-Sanders (sandersn)
left a comment
There was a problem hiding this comment.
Couple of minor suggestions.
| } | ||
| } | ||
| if (possiblyDiscriminant) { | ||
| const result = getDiscriminationResultForProperty(contextualType, discriminatingType!, prop.symbol.escapedName, match, isTypeAssignableTo); |
There was a problem hiding this comment.
You could get rid of the nullable assertion if you dropped possiblyDiscriminant and checked discriminatingType above. The no-initialiser case would need to change to discriminatingType = !!isDiscr... && trueType.
Not sure if it's worth the change. Probably?
|
Nathan Shively-Sanders (@sandersn) I did better; you probably wanna look again (again), since AFAIK none of your last set of comments apply anymore. |
Nathan Shively-Sanders (sandersn)
left a comment
Nathan Shively-Sanders (sandersn)
left a comment
There was a problem hiding this comment.
This version is ok, although to be honest I liked the previous one a little better. This one makes me wonder if passing tuples of closures is a bit slow, whether we should measure it, etc.
3 participants
Fixes #26004