Skip to content

Commit db0d8e0

Browse files
yuityuit
authored
Fix 8549: Using variable as Jsx tagname (#9337)
* Parse JSXElement's name as property access instead of just entity name. So when one accesses property of the class through this, checker will check correctly * wip - just resolve to any type for now * Resolve string type to anytype and look up property in intrinsicElementsType of Jsx * Add tests and update baselines * Remove unneccessary comment * wip-address PR * Address PR * Add tets and update baselines * Fix linting error
1 parent 2aa1d71 commit db0d8e0

40 files changed

+836
-24
lines changed

‎src/compiler/checker.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9657,8 +9657,9 @@ namespace ts {
96579657
/**
96589658
* Returns true iff React would emit this tag name as a string rather than an identifier or qualified name
96599659
*/
9660-
function isJsxIntrinsicIdentifier(tagName: Identifier | QualifiedName) {
9661-
if (tagName.kind === SyntaxKind.QualifiedName) {
9660+
function isJsxIntrinsicIdentifier(tagName: JsxTagNameExpression) {
9661+
// TODO (yuisu): comment
9662+
if (tagName.kind === SyntaxKind.PropertyAccessExpression || tagName.kind === SyntaxKind.ThisKeyword) {
96629663
return false;
96639664
}
96649665
else {
@@ -9854,6 +9855,29 @@ namespace ts {
98549855
}));
98559856
}
98569857

9858+
// If the elemType is a string type, we have to return anyType to prevent an error downstream as we will try to find construct or call signature of the type
9859+
if (elemType.flags & TypeFlags.String) {
9860+
return anyType;
9861+
}
9862+
else if (elemType.flags & TypeFlags.StringLiteral) {
9863+
// If the elemType is a stringLiteral type, we can then provide a check to make sure that the string literal type is one of the Jsx intrinsic element type
9864+
const intrinsicElementsType = getJsxType(JsxNames.IntrinsicElements);
9865+
if (intrinsicElementsType !== unknownType) {
9866+
const stringLiteralTypeName = (<StringLiteralType>elemType).text;
9867+
const intrinsicProp = getPropertyOfType(intrinsicElementsType, stringLiteralTypeName);
9868+
if (intrinsicProp) {
9869+
return getTypeOfSymbol(intrinsicProp);
9870+
}
9871+
const indexSignatureType = getIndexTypeOfType(intrinsicElementsType, IndexKind.String);
9872+
if (indexSignatureType) {
9873+
return indexSignatureType;
9874+
}
9875+
error(node, Diagnostics.Property_0_does_not_exist_on_type_1, stringLiteralTypeName, "JSX." + JsxNames.IntrinsicElements);
9876+
}
9877+
// If we need to report an error, we already done so here. So just return any to prevent any more error downstream
9878+
return anyType;
9879+
}
9880+
98579881
// Get the element instance type (the result of newing or invoking this tag)
98589882
const elemInstanceType = getJsxElementInstanceType(node, elemType);
98599883

‎src/compiler/emitter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1219,7 +1219,7 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge
12191219
function jsxEmitReact(node: JsxElement | JsxSelfClosingElement) {
12201220
/// Emit a tag name, which is either '"div"' for lower-cased names, or
12211221
/// 'Div' for upper-cased or dotted names
1222-
function emitTagName(name: Identifier | QualifiedName) {
1222+
function emitTagName(name: LeftHandSideExpression) {
12231223
if (name.kind === SyntaxKind.Identifier && isIntrinsicJsxName((<Identifier>name).text)) {
12241224
write('"');
12251225
emit(name);

‎src/compiler/parser.ts‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3576,7 +3576,7 @@ namespace ts {
35763576
return finishNode(node);
35773577
}
35783578

3579-
function tagNamesAreEquivalent(lhs: EntityName, rhs: EntityName): boolean {
3579+
function tagNamesAreEquivalent(lhs: JsxTagNameExpression, rhs: JsxTagNameExpression): boolean {
35803580
if (lhs.kind !== rhs.kind) {
35813581
return false;
35823582
}
@@ -3585,8 +3585,15 @@ namespace ts {
35853585
return (<Identifier>lhs).text === (<Identifier>rhs).text;
35863586
}
35873587

3588-
return (<QualifiedName>lhs).right.text === (<QualifiedName>rhs).right.text &&
3589-
tagNamesAreEquivalent((<QualifiedName>lhs).left, (<QualifiedName>rhs).left);
3588+
if (lhs.kind === SyntaxKind.ThisKeyword) {
3589+
return true;
3590+
}
3591+
3592+
// If we are at this statement then we must have PropertyAccessExpression and because tag name in Jsx element can only
3593+
// take forms of JsxTagNameExpression which includes an identifier, "this" expression, or another propertyAccessExpression
3594+
// it is safe to case the expression property as such. See parseJsxElementName for how we parse tag name in Jsx element
3595+
return (<PropertyAccessExpression>lhs).name.text === (<PropertyAccessExpression>rhs).name.text &&
3596+
tagNamesAreEquivalent((<PropertyAccessExpression>lhs).expression as JsxTagNameExpression, (<PropertyAccessExpression>rhs).expression as JsxTagNameExpression);
35903597
}
35913598

35923599

@@ -3654,7 +3661,7 @@ namespace ts {
36543661
Debug.fail("Unknown JSX child kind " + token);
36553662
}
36563663

3657-
function parseJsxChildren(openingTagName: EntityName): NodeArray<JsxChild> {
3664+
function parseJsxChildren(openingTagName: LeftHandSideExpression): NodeArray<JsxChild> {
36583665
const result = <NodeArray<JsxChild>>[];
36593666
result.pos = scanner.getStartPos();
36603667
const saveParsingContext = parsingContext;
@@ -3717,17 +3724,22 @@ namespace ts {
37173724
return finishNode(node);
37183725
}
37193726

3720-
function parseJsxElementName(): EntityName {
3727+
function parseJsxElementName(): JsxTagNameExpression {
37213728
scanJsxIdentifier();
3722-
let elementName: EntityName = parseIdentifierName();
3729+
// JsxElement can have name in the form of
3730+
// propertyAccessExpression
3731+
// primaryExpression in the form of an identifier and "this" keyword
3732+
// We can't just simply use parseLeftHandSideExpressionOrHigher because then we will start consider class,function etc as a keyword
3733+
// We only want to consider "this" as a primaryExpression
3734+
let expression: JsxTagNameExpression = token === SyntaxKind.ThisKeyword ?
3735+
parseTokenNode<PrimaryExpression>() : parseIdentifierName();
37233736
while (parseOptional(SyntaxKind.DotToken)) {
3724-
scanJsxIdentifier();
3725-
const node: QualifiedName = <QualifiedName>createNode(SyntaxKind.QualifiedName, elementName.pos); // !!!
3726-
node.left = elementName;
3727-
node.right = parseIdentifierName();
3728-
elementName = finishNode(node);
3737+
const propertyAccess: PropertyAccessExpression = <PropertyAccessExpression>createNode(SyntaxKind.PropertyAccessExpression, expression.pos);
3738+
propertyAccess.expression = expression;
3739+
propertyAccess.name = parseRightSideOfDot(/*allowIdentifierNames*/ true);
3740+
expression = finishNode(propertyAccess);
37293741
}
3730-
return elementName;
3742+
return expression;
37313743
}
37323744

37333745
function parseJsxExpression(inExpressionContext: boolean): JsxExpression {

‎src/compiler/types.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,11 +1042,13 @@ namespace ts {
10421042
closingElement: JsxClosingElement;
10431043
}
10441044

1045+
export type JsxTagNameExpression = PrimaryExpression | PropertyAccessExpression;
1046+
10451047
/// The opening element of a <Tag>...</Tag> JsxElement
10461048
// @kind(SyntaxKind.JsxOpeningElement)
10471049
export interface JsxOpeningElement extends Expression {
10481050
_openingElementBrand?: any;
1049-
tagName: EntityName;
1051+
tagName: JsxTagNameExpression;
10501052
attributes: NodeArray<JsxAttribute | JsxSpreadAttribute>;
10511053
}
10521054

@@ -1073,7 +1075,7 @@ namespace ts {
10731075

10741076
// @kind(SyntaxKind.JsxClosingElement)
10751077
export interface JsxClosingElement extends Node {
1076-
tagName: EntityName;
1078+
tagName: JsxTagNameExpression;
10771079
}
10781080

10791081
// @kind(SyntaxKind.JsxExpression)

‎tests/baselines/reference/tsxDynamicTagName1.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//// [tsxDynamicTagName1.tsx]
2+
3+
var CustomTag = "h1";
4+
<CustomTag> Hello World </CustomTag> // No error
5+
6+
//// [tsxDynamicTagName1.jsx]
7+
var CustomTag = "h1";
8+
<CustomTag> Hello World </CustomTag>; // No error

‎tests/baselines/reference/tsxDynamicTagName1.symbols‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
=== tests/cases/conformance/jsx/tsxDynamicTagName1.tsx ===
2+
3+
var CustomTag = "h1";
4+
>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName1.tsx, 1, 3))
5+
6+
<CustomTag> Hello World </CustomTag> // No error
7+
>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName1.tsx, 1, 3))
8+
>CustomTag : Symbol(CustomTag, Decl(tsxDynamicTagName1.tsx, 1, 3))
9+

‎tests/baselines/reference/tsxDynamicTagName1.types‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
=== tests/cases/conformance/jsx/tsxDynamicTagName1.tsx ===
2+
3+
var CustomTag = "h1";
4+
>CustomTag : string
5+
>"h1" : string
6+
7+
<CustomTag> Hello World </CustomTag> // No error
8+
><CustomTag> Hello World </CustomTag> : any
9+
>CustomTag : string
10+
>CustomTag : string
11+

‎tests/baselines/reference/tsxDynamicTagName2.errors.txt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
tests/cases/conformance/jsx/tsxDynamicTagName2.tsx(10,1): error TS2339: Property 'customTag' does not exist on type 'JSX.IntrinsicElements'.
2+
tests/cases/conformance/jsx/tsxDynamicTagName2.tsx(10,25): error TS2339: Property 'customTag' does not exist on type 'JSX.IntrinsicElements'.
3+
4+
5+
==== tests/cases/conformance/jsx/tsxDynamicTagName2.tsx (2 errors) ====
6+
7+
declare module JSX {
8+
interface Element { }
9+
interface IntrinsicElements {
10+
div: any
11+
}
12+
}
13+
14+
var customTag = "h1";
15+
<customTag> Hello World </customTag> // This should be an error. The lower-case is look up as an intrinsic element name
16+
~~~~~~~~~~~
17+
!!! error TS2339: Property 'customTag' does not exist on type 'JSX.IntrinsicElements'.
18+
~~~~~~~~~~~~
19+
!!! error TS2339: Property 'customTag' does not exist on type 'JSX.IntrinsicElements'.

‎tests/baselines/reference/tsxDynamicTagName2.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//// [tsxDynamicTagName2.tsx]
2+
3+
declare module JSX {
4+
interface Element { }
5+
interface IntrinsicElements {
6+
div: any
7+
}
8+
}
9+
10+
var customTag = "h1";
11+
<customTag> Hello World </customTag> // This should be an error. The lower-case is look up as an intrinsic element name
12+
13+
//// [tsxDynamicTagName2.jsx]
14+
var customTag = "h1";
15+
<customTag> Hello World </customTag>; // This should be an error. The lower-case is look up as an intrinsic element name

‎tests/baselines/reference/tsxDynamicTagName3.errors.txt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
tests/cases/conformance/jsx/tsxDynamicTagName3.tsx(10,1): error TS2339: Property 'h1' does not exist on type 'JSX.IntrinsicElements'.
2+
3+
4+
==== tests/cases/conformance/jsx/tsxDynamicTagName3.tsx (1 errors) ====
5+
6+
declare module JSX {
7+
interface Element { }
8+
interface IntrinsicElements {
9+
div: any
10+
}
11+
}
12+
13+
var CustomTag: "h1" = "h1";
14+
<CustomTag> Hello World </CustomTag> // This should be an error. we will try look up string literal type in JSX.IntrinsicElements
15+
~~~~~~~~~~~
16+
!!! error TS2339: Property 'h1' does not exist on type 'JSX.IntrinsicElements'.

0 commit comments

Comments
 (0)