-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
update contextual discrimination to include omitted members #43633
update contextual discrimination to include omitted members #43633
Conversation
This diff extends the types checked by discriminateContextualTypeByObjectMembers and discriminateContextualTypeByJSXAttributes to also include any optional components in the type union. fixes #41759 although it doesn't address the better error reporting for their last repro, which I'm not sure how to address.
@weswigham that check makes sense. It should be resolved, although I had trouble accepting the changes through gh. Thanks for the pointers of where to start btw, probably wouldn't have thrown this together without them. |
Under the TS 4.3 beta, this change appears to have introduced a stack overflow in my codebase:
|
@schmod can you provide more context? Ideally a minimum working example, and even more ideally a bisect showing it was this commit? The call stack seems to indicate that it's causing a stack overflow at |
This diff extends the types checked by
discriminateContextualTypeByObjectMembers
anddiscriminateContextualTypeByJSXAttributes
to also include any optional components in the type union.fixes #41759 although it doesn't address the better error reporting for their last repro, which I'm not sure how to address.