-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Fix #15463: use intersection types to emulate spread in generic react components #15851
Conversation
src/compiler/checker.ts
Outdated
|
||
const attributeType = createJsxAttributesType(attributes.symbol, attributesTable); | ||
|
||
return typeToIntersect ? attributesTable.size ? getIntersectionType([typeToIntersect, attributeType]) : typeToIntersect : attributeType; |
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.
what about a flatter form?
typeToIntersect && attributesTable.size ? getIntersectionType([typeToIntersect, attributeType]) :
typeToIntersect ? typeToIntersect : attributeType
|
||
const attributeType = createJsxAttributesType(attributes.symbol, attributesTable); | ||
return typeToIntersect && attributesTable.size ? getIntersectionType([typeToIntersect, attributeType]) : | ||
typeToIntersect ? typeToIntersect : attributeType; |
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.
Consider
if (typeToIntersect) {
if (attributesTable.size) {
return getIntersectionType([typeToIntersect, attributeType]);
}
return typeToIntersect;
}
return attributeType;
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.
We use the ternary operator quite often. so do not think this is out of line here.
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.
You would save one condition and make it way easier to read in this case tho
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.
Apart from the simplifying the isValidSpreadType condition, 👍
src/compiler/checker.ts
Outdated
if (isTypeAny(exprType)) { | ||
hasSpreadAnyType = true; | ||
} | ||
spread = getSpreadType(spread, exprType); | ||
if (!isValidSpreadType(exprType)) { |
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.
Invert the condition here and flip them around 😄
Fixes #15463