Elaborate jsx children elementwise #29264
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11196,7 +11196,7 @@ namespace ts { | |
| case SyntaxKind.ArrayLiteralExpression: | ||
| return elaborateArrayLiteral(node as ArrayLiteralExpression, source, target, relation); | ||
| case SyntaxKind.JsxAttributes: | ||
| return elaborateJsxComponents(node as JsxAttributes, source, target, relation); | ||
| case SyntaxKind.ArrowFunction: | ||
| return elaborateArrowFunction(node as ArrowFunction, source, target, relation); | ||
| } | ||
|
|
@@ -11336,8 +11336,113 @@ namespace ts { | |
| } | ||
| } | ||
|
|
||
| function *generateJsxChildren(node: JsxElement, getInvalidTextDiagnostic: () => DiagnosticMessage): ElaborationIterator { | ||
| if (!length(node.children)) return; | ||
| let memberOffset = 0; | ||
| for (let i = 0; i < node.children.length; i++) { | ||
| const child = node.children[i]; | ||
| const nameType = getLiteralType(i - memberOffset); | ||
| const elem = getElaborationElementForJsxChild(child, nameType, getInvalidTextDiagnostic); | ||
| if (elem) { | ||
| yield elem; | ||
| } | ||
| else { | ||
| memberOffset++; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function getElaborationElementForJsxChild(child: JsxChild, nameType: LiteralType, getInvalidTextDiagnostic: () => DiagnosticMessage) { | ||
| switch (child.kind) { | ||
| case SyntaxKind.JsxExpression: | ||
| // child is of the type of the expression | ||
| return { errorNode: child, innerExpression: child.expression, nameType }; | ||
| case SyntaxKind.JsxText: | ||
| if (child.containsOnlyWhiteSpaces) { | ||
| break; // Whitespace only jsx text isn't real jsx text | ||
| } | ||
|
There was a problem hiding this comment. This is hardcoding React-specific knowledge (preact makes no distinction between 1 or many children, it's always array), but it's still better than how it is right now. This does mean that "The fact that a single-element-child isn't assignable to an array target (because it's not inserted into an array) is real strange." is actually a correct implementation of React at runtime. There was a problem hiding this comment. ick, react-specific jsx checker code. This and the (now incorrect) sfc return type check. 🤕 There was a problem hiding this comment. This is why React tells you to always use React.Children when dealing with children, but nobody listens... 😥 There was a problem hiding this comment.
OK. |
||
| // child is a string | ||
| return { errorNode: child, innerExpression: undefined, nameType, errorMessage: getInvalidTextDiagnostic() }; | ||
|
There was a problem hiding this comment. Do you think this fixes There was a problem hiding this comment. The react const Tag = (x: {}) => <div></div>;
// OK
const k1 = <Tag />;
const k2 = <Tag></Tag>;
// Not OK (excess children)
const k3 = <Tag children={<div></div>} />;
const k4 = <Tag><div></div></Tag>;
const k5 = <Tag><div></div><div></div></Tag>;it seems to work OK. There was a problem hiding this comment. That's not dependent on this PR, however - that's how it currently behaves. (This PR only adjusts error reporting mostly, after all) There was a problem hiding this comment. Ick, works less OK when you have props in common: const k4 = <Tag key="1"><div></div></Tag>; // should error but doesn't
const k5 = <Tag key="1"><div></div><div></div></Tag>; // should error but doesn'tThere was a problem hiding this comment. Hahahahahahaha the children prop thing is defs a bug on our part. It's excluded from excess property checks because it's missing a There was a problem hiding this comment. Yep, this is that bug I found that time I did try to stop the unconditional mixing in of children :) |
||
| case SyntaxKind.JsxElement: | ||
| case SyntaxKind.JsxSelfClosingElement: | ||
| case SyntaxKind.JsxFragment: | ||
| // child is of type JSX.Element | ||
| return { errorNode: child, innerExpression: child, nameType }; | ||
| default: | ||
| return Debug.assertNever(child, "Found invalid jsx child"); | ||
| } | ||
| } | ||
|
|
||
| function elaborateJsxComponents(node: JsxAttributes, source: Type, target: Type, relation: Map<RelationComparisonResult>) { | ||
| let result = elaborateElementwise(generateJsxAttributes(node), source, target, relation); | ||
| let invalidTextDiagnostic: DiagnosticMessage | undefined; | ||
| if (isJsxOpeningElement(node.parent) && isJsxElement(node.parent.parent)) { | ||
| const containingElement = node.parent.parent; | ||
| const childPropName = getJsxElementChildrenPropertyName(getJsxNamespaceAt(node)); | ||
| const childrenPropName = childPropName === undefined ? "children" : unescapeLeadingUnderscores(childPropName); | ||
| const childrenNameType = getLiteralType(childrenPropName); | ||
| const childrenTargetType = getIndexedAccessType(target, childrenNameType); | ||
| const validChildren = filter(containingElement.children, i => !isJsxText(i) || !i.containsOnlyWhiteSpaces); | ||
| if (!length(validChildren)) { | ||
| return result; | ||
| } | ||
| const moreThanOneRealChildren = length(validChildren) > 1; | ||
| const arrayLikeTargetParts = filterType(childrenTargetType, isArrayOrTupleLikeType); | ||
| const nonArrayLikeTargetParts = filterType(childrenTargetType, t => !isArrayOrTupleLikeType(t)); | ||
| if (moreThanOneRealChildren) { | ||
| if (arrayLikeTargetParts !== neverType) { | ||
| const realSource = createTupleType(checkJsxChildren(containingElement, CheckMode.Normal)); | ||
| result = elaborateElementwise(generateJsxChildren(containingElement, getInvalidTextualChildDiagnostic), realSource, arrayLikeTargetParts, relation) || result; | ||
| } | ||
| else if (!isTypeRelatedTo(getIndexedAccessType(source, childrenNameType), childrenTargetType, relation)) { | ||
| // arity mismatch | ||
| result = true; | ||
| error( | ||
| containingElement.openingElement.tagName, | ||
| Diagnostics.This_JSX_tag_s_0_prop_expects_a_single_child_of_type_1_but_multiple_children_were_provided, | ||
| childrenPropName, | ||
| typeToString(childrenTargetType) | ||
| ); | ||
| } | ||
| } | ||
| else { | ||
| if (nonArrayLikeTargetParts !== neverType) { | ||
| const child = validChildren[0]; | ||
| const elem = getElaborationElementForJsxChild(child, childrenNameType, getInvalidTextualChildDiagnostic); | ||
| if (elem) { | ||
| result = elaborateElementwise( | ||
| (function*() { yield elem; })(), | ||
| source, | ||
| target, | ||
| relation | ||
| ) || result; | ||
| } | ||
| } | ||
| else if (!isTypeRelatedTo(getIndexedAccessType(source, childrenNameType), childrenTargetType, relation)) { | ||
| // arity mismatch | ||
| result = true; | ||
| error( | ||
| containingElement.openingElement.tagName, | ||
| Diagnostics.This_JSX_tag_s_0_prop_expects_type_1_which_requires_multiple_children_but_only_a_single_child_was_provided, | ||
| childrenPropName, | ||
| typeToString(childrenTargetType) | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| return result; | ||
|
|
||
| function getInvalidTextualChildDiagnostic() { | ||
| if (!invalidTextDiagnostic) { | ||
| const tagNameText = getTextOfNode(node.parent.tagName); | ||
| const childPropName = getJsxElementChildrenPropertyName(getJsxNamespaceAt(node)); | ||
| const childrenPropName = childPropName === undefined ? "children" : unescapeLeadingUnderscores(childPropName); | ||
| const childrenTargetType = getIndexedAccessType(target, getLiteralType(childrenPropName)); | ||
| const diagnostic = Diagnostics._0_components_don_t_accept_text_as_child_elements_Text_in_JSX_has_the_type_string_but_the_expected_type_of_1_is_2; | ||
| invalidTextDiagnostic = { ...diagnostic, key: "!!ALREADY FORMATTED!!", message: formatMessage(/*_dummy*/ undefined, diagnostic, tagNameText, childrenPropName, typeToString(childrenTargetType)) }; | ||
| } | ||
| return invalidTextDiagnostic; | ||
| } | ||
| } | ||
|
|
||
| function *generateLimitedTupleElements(node: ArrayLiteralExpression, target: Type): ElaborationIterator { | ||
|
|
@@ -13478,6 +13583,10 @@ namespace ts { | |
| return isTupleType(type) || !!getPropertyOfType(type, "0" as __String); | ||
| } | ||
|
|
||
| function isArrayOrTupleLikeType(type: Type): boolean { | ||
| return isArrayLikeType(type) || isTupleLikeType(type); | ||
| } | ||
|
|
||
| function getTupleElementType(type: Type, index: number) { | ||
| const propType = getTypeOfPropertyOfType(type, "" + index as __String); | ||
| if (propType) { | ||
|
|
@@ -17493,19 +17602,31 @@ namespace ts { | |
| return node === conditional.whenTrue || node === conditional.whenFalse ? getContextualType(conditional) : undefined; | ||
| } | ||
|
|
||
| function getContextualTypeForChildJsxExpression(node: JsxElement, child: JsxChild) { | ||
| const attributesType = getApparentTypeOfContextualType(node.openingElement.tagName); | ||
| // JSX expression is in children of JSX Element, we will look for an "children" atttribute (we get the name from JSX.ElementAttributesProperty) | ||
| const jsxChildrenPropertyName = getJsxElementChildrenPropertyName(getJsxNamespaceAt(node)); | ||
| if (!(attributesType && !isTypeAny(attributesType) && jsxChildrenPropertyName && jsxChildrenPropertyName !== "")) { | ||
| return undefined; | ||
| } | ||
| const childIndex = node.children.indexOf(child); | ||
| const childFieldType = getTypeOfPropertyOfContextualType(attributesType, jsxChildrenPropertyName); | ||
| return childFieldType && mapType(childFieldType, t => { | ||
| if (isArrayLikeType(t)) { | ||
| return getIndexedAccessType(t, getLiteralType(childIndex)); | ||
| } | ||
| else { | ||
| return t; | ||
| } | ||
| }, /*noReductions*/ true); | ||
| } | ||
|
|
||
| function getContextualTypeForJsxExpression(node: JsxExpression): Type | undefined { | ||
| const exprParent = node.parent; | ||
| return isJsxAttributeLike(exprParent) | ||
| ? getContextualType(node) | ||
| : isJsxElement(exprParent) | ||
| ? getContextualTypeForChildJsxExpression(exprParent, node) | ||
| : undefined; | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
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.
will have a
JsxTextof'\n'but it'll get eliminated before it's converted to the JSX factory call. It won't become a call toReact.createElement(Foo, {}, '\n'), but ratherReact.createElement(Foo).It will be real text if it's placed between two other JSX elements and doesn't look like newline + indent (
<><Foo/> <Bar/></>has 3 children), but I think this is only checking the single child case.