Skip to content

Commit 306de4d

Browse files
RyanCavanaughRyanCavanaugh
committed
CR feedback
1 parent b1a05b8 commit 306de4d

File tree

10 files changed

+145
-62
lines changed

10 files changed

+145
-62
lines changed

‎src/compiler/checker.ts

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ namespace ts {
165165
}
166166
};
167167

168-
let JsxNames = {
168+
const JsxNames = {
169169
JSX: "JSX",
170170
IntrinsicElements: "IntrinsicElements",
171171
ElementClass: "ElementClass",
@@ -5578,6 +5578,7 @@ namespace ts {
55785578
case SyntaxKind.JsxAttribute:
55795579
case SyntaxKind.JsxSpreadAttribute:
55805580
case SyntaxKind.JsxOpeningElement:
5581+
case SyntaxKind.JsxExpression:
55815582
return forEachChild(node, isAssignedIn);
55825583
}
55835584
return false;
@@ -6774,8 +6775,7 @@ namespace ts {
67746775
return false;
67756776
}
67766777
else {
6777-
let firstChar = (<Identifier>tagName).text.charAt(0);
6778-
return firstChar.toLowerCase() === firstChar;
6778+
return isIntrinsicJsxName((<Identifier>tagName).text);
67796779
}
67806780
}
67816781

@@ -6836,10 +6836,7 @@ namespace ts {
68366836
/// Returns the type JSX.IntrinsicElements. May return `unknownType` if that type is not present.
68376837
function getJsxIntrinsicElementsType() {
68386838
if (!jsxIntrinsicElementsType) {
6839-
let jsxNamespace = getGlobalSymbol(JsxNames.JSX, SymbolFlags.Namespace, undefined);
6840-
let intrinsicsSymbol = jsxNamespace && getSymbol(jsxNamespace.exports, JsxNames.IntrinsicElements, SymbolFlags.Type);
6841-
let intrinsicsType = intrinsicsSymbol && getDeclaredTypeOfSymbol(intrinsicsSymbol);
6842-
jsxIntrinsicElementsType = intrinsicsType || unknownType;
6839+
jsxIntrinsicElementsType = getExportedTypeFromNamespace(JsxNames.JSX, JsxNames.IntrinsicElements) || unknownType;
68436840
}
68446841
return jsxIntrinsicElementsType;
68456842
}
@@ -6977,18 +6974,23 @@ namespace ts {
69776974
let attribProperties = attribPropType && getPropertiesOfType(attribPropType);
69786975

69796976
if (attribProperties) {
6977+
// Element Attributes has zero properties, so the element attributes type will be the class instance type
69806978
if (attribProperties.length === 0) {
69816979
return '';
69826980
}
6981+
// Element Attributes has one property, so the element attributes type will be the type of the corresponding
6982+
// property of the class instance type
69836983
else if (attribProperties.length === 1) {
69846984
return attribProperties[0].name;
69856985
}
6986+
// More than one property on ElementAttributesProperty is an error
69866987
else {
69876988
error(attribsPropTypeSym.declarations[0], Diagnostics.The_global_type_JSX_0_may_not_have_more_than_one_property, JsxNames.ElementAttributesPropertyNameContainer);
69886989
return undefined;
69896990
}
69906991
}
69916992
else {
6993+
// No interface exists, so the element attributes type will be an implicit any
69926994
return undefined;
69936995
}
69946996
}
@@ -7044,7 +7046,7 @@ namespace ts {
70447046
return links.resolvedJsxType = getIndexTypeOfSymbol(sym, IndexKind.String);
70457047
}
70467048
else {
7047-
// Resolution failed
7049+
// Resolution failed, so we don't know
70487050
return links.resolvedJsxType = anyType;
70497051
}
70507052
}
@@ -7063,16 +7065,12 @@ namespace ts {
70637065
return prop || unknownSymbol;
70647066
}
70657067

7068+
let jsxElementClassType: Type = undefined;
70667069
function getJsxGlobalElementClassType(): Type {
7067-
let jsxNS = getGlobalSymbol(JsxNames.JSX, SymbolFlags.Namespace, /*diagnosticMessage*/ undefined);
7068-
if (jsxNS) {
7069-
let sym = getSymbol(jsxNS.exports, JsxNames.ElementClass, SymbolFlags.Type);
7070-
let elemClassType = sym && getDeclaredTypeOfSymbol(sym);
7071-
return elemClassType;
7072-
}
7073-
else {
7074-
return undefined;
7070+
if(!jsxElementClassType) {
7071+
jsxElementClassType = getExportedTypeFromNamespace(JsxNames.JSX, JsxNames.ElementClass);
70757072
}
7073+
return jsxElementClassType;
70767074
}
70777075

70787076
/// Returns all the properties of the Jsx.IntrinsicElements interface
@@ -7137,8 +7135,7 @@ namespace ts {
71377135
return checkExpression(node.expression);
71387136
}
71397137
else {
7140-
/// <Foo enabled /> is shorthand for <Foo enabled={true} />
7141-
return booleanType;
7138+
return unknownType;
71427139
}
71437140
}
71447141

‎src/compiler/emitter.ts

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,20 +1113,12 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {
11131113
/// Emit a tag name, which is either '"div"' for lower-cased names, or
11141114
/// 'Div' for upper-cased or dotted names
11151115
function emitTagName(name: Identifier|QualifiedName) {
1116-
if (name.kind === SyntaxKind.Identifier) {
1117-
var ch = (<Identifier>name).text.charAt(0);
1118-
if (ch.toUpperCase() === ch) {
1119-
emit(name);
1120-
}
1121-
else {
1122-
write('"');
1123-
emit(name);
1124-
write('"');
1125-
}
1126-
return ch.toUpperCase() !== ch;
1116+
if (name.kind === SyntaxKind.Identifier && isIntrinsicJsxName((<Identifier>name).text)) {
1117+
write('"');
1118+
emit(name);
1119+
write('"');
11271120
}
11281121
else {
1129-
Debug.assert(name.kind === SyntaxKind.QualifiedName);
11301122
emit(name);
11311123
}
11321124
}
@@ -1234,12 +1226,19 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {
12341226
}
12351227

12361228
// Don't emit empty strings
1237-
if (children[i].kind === SyntaxKind.JsxText && !shouldEmitJsxText(<JsxText>children[i])) {
1238-
continue;
1229+
if (children[i].kind === SyntaxKind.JsxText) {
1230+
let text = getTextToEmit(<JsxText>children[i]);
1231+
if(text !== undefined) {
1232+
write(', "');
1233+
write(text);
1234+
write('"');
1235+
}
1236+
}
1237+
else {
1238+
write(', ');
1239+
emit(children[i]);
12391240
}
12401241

1241-
write(', ');
1242-
emit(children[i]);
12431242
}
12441243
}
12451244

@@ -5895,12 +5894,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {
58955894
}
58965895

58975896
function trimReactWhitespace(node: JsxText): string {
5898-
// Could be empty string, do not use !node.formattedReactText
5899-
if (node.formattedReactText !== undefined) {
5900-
return node.formattedReactText;
5901-
}
5902-
5903-
let lines: string[] = [];
5897+
let result: string = undefined;
59045898
let text = getTextOfNode(node);
59055899
let firstNonWhitespace = 0;
59065900
let lastNonWhitespace = -1;
@@ -5910,9 +5904,10 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {
59105904
// on the same line as the closing tag. See examples in tests/cases/conformance/jsx/tsxReactEmitWhitespace.tsx
59115905
for (let i = 0; i < text.length; i++) {
59125906
let c = text.charCodeAt(i);
5913-
if (c === CharacterCodes.lineFeed || c === CharacterCodes.carriageReturn) {
5907+
if (isLineBreak(c)) {
59145908
if (firstNonWhitespace !== -1 && (lastNonWhitespace - firstNonWhitespace + 1 > 0)) {
5915-
lines.push(text.substr(firstNonWhitespace, lastNonWhitespace - firstNonWhitespace + 1));
5909+
let part = text.substr(firstNonWhitespace, lastNonWhitespace - firstNonWhitespace + 1);
5910+
result = (result ? result + '" + \' \' + "' : '') + part;
59165911
}
59175912
firstNonWhitespace = -1;
59185913
}
@@ -5924,19 +5919,26 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {
59245919
}
59255920
}
59265921
if (firstNonWhitespace !== -1) {
5927-
lines.push(text.substr(firstNonWhitespace));
5922+
let part = text.substr(firstNonWhitespace);
5923+
result = (result ? result + '" + \' \' + "' : '') + part;
59285924
}
59295925

5930-
return node.formattedReactText = lines.join('" + \' \' + "');
5926+
return result;
59315927
}
59325928

5933-
function shouldEmitJsxText(node: JsxText) {
5929+
function getTextToEmit(node: JsxText) {
59345930
switch (compilerOptions.jsx) {
59355931
case JsxEmit.React:
5936-
return trimReactWhitespace(node).length > 0;
5932+
let text = trimReactWhitespace(node);
5933+
if (text.length === 0) {
5934+
return undefined;
5935+
}
5936+
else {
5937+
return text;
5938+
}
59375939
case JsxEmit.Preserve:
59385940
default:
5939-
return true;
5941+
return getTextOfNode(node, true);
59405942
}
59415943
}
59425944

‎src/compiler/parser.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ namespace ts {
657657
sourceFile.languageVersion = languageVersion;
658658
sourceFile.fileName = normalizePath(fileName);
659659
sourceFile.flags = fileExtensionIs(sourceFile.fileName, ".d.ts") ? NodeFlags.DeclarationFile : 0;
660-
sourceFile.languageVariant = fileExtensionIs(sourceFile.fileName, ".tsx") ? LanguageVariant.JSX : LanguageVariant.Standard;
660+
sourceFile.languageVariant = isTsx(sourceFile.fileName) ? LanguageVariant.JSX : LanguageVariant.Standard;
661661

662662
return sourceFile;
663663
}
@@ -1298,7 +1298,6 @@ namespace ts {
12981298
case ParsingContext.HeritageClauses:
12991299
return token === SyntaxKind.OpenBraceToken || token === SyntaxKind.CloseBraceToken;
13001300
case ParsingContext.JsxAttributes:
1301-
// REMOVE -> // For error recovery, include } here (otherwise an over-braced {expr}} will close the surrounding statement block and mess up the entire file).
13021301
return token === SyntaxKind.GreaterThanToken || token === SyntaxKind.SlashToken;
13031302
case ParsingContext.JsxChildren:
13041303
return token === SyntaxKind.LessThanToken && lookAhead(nextTokenIsSlash);
@@ -3377,9 +3376,6 @@ namespace ts {
33773376
node.name = parseIdentifierName();
33783377
if (parseOptional(SyntaxKind.EqualsToken)) {
33793378
switch (token) {
3380-
case SyntaxKind.LessThanToken:
3381-
node.initializer = parseJsxElementOrSelfClosingElement();
3382-
break;
33833379
case SyntaxKind.StringLiteral:
33843380
node.initializer = parseLiteralNode();
33853381
break;

‎src/compiler/types.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -877,8 +877,6 @@ namespace ts {
877877

878878
export interface JsxText extends Node {
879879
_jsxTextExpressionBrand: any;
880-
/// Used by the emitter to avoid recomputation
881-
formattedReactText?: string;
882880
}
883881

884882
export type JsxChild = JsxText | JsxExpression | JsxElement | JsxSelfClosingElement;

‎src/compiler/utilities.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1535,6 +1535,11 @@ namespace ts {
15351535
}
15361536
}
15371537

1538+
export function isIntrinsicJsxName(name: string) {
1539+
let ch = name.substr(0, 1);
1540+
return ch.toLowerCase() === ch;
1541+
}
1542+
15381543
function get16BitUnicodeEscapeSequence(charCode: number): string {
15391544
let hexCharCode = charCode.toString(16).toUpperCase();
15401545
let paddedHexCode = ("0000" + hexCharCode).slice(-4);

‎src/services/services.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2934,7 +2934,7 @@ namespace ts {
29342934
getTypeScriptMemberSymbols();
29352935
}
29362936
else if (isRightOfOpenTag) {
2937-
let tagSymbols = typeChecker.getJsxIntrinsicTagNames();;
2937+
let tagSymbols = typeChecker.getJsxIntrinsicTagNames();
29382938
if (tryGetGlobalSymbols()) {
29392939
symbols = tagSymbols.concat(symbols.filter(s => !!(s.flags & SymbolFlags.Value)));
29402940
}

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,17): error TS1005: '{' expected.
2+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,23): error TS1005: '}' expected.
3+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,29): error TS1005: '{' expected.
4+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,57): error TS1109: Expression expected.
5+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(39,58): error TS1109: Expression expected.
6+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,1): error TS1003: Identifier expected.
7+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,6): error TS1109: Expression expected.
8+
tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx(41,12): error TS1109: Expression expected.
9+
10+
11+
==== tests/cases/conformance/jsx/jsxEsprimaFbTestSuite.tsx (8 errors) ====
12+
declare var React: any;
13+
declare var 日本語;
14+
declare var AbC_def;
15+
declare var LeftRight;
16+
declare var x;
17+
declare var a;
18+
declare var props;
19+
20+
<a />;
21+
22+
//<n:a n:v />; Namespace unsuported
23+
24+
//<a n:foo="bar"> {value} <b><c /></b></a>; Namespace unsuported
25+
26+
<a b={" "} c=" " d="&amp;" e="id=1&group=2" f="&#123456789" g="&#123*;" h="&#x;" />;
27+
28+
<a b="&notanentity;" />;
29+
<a
30+
/>;
31+
32+
<日本語></日本語>;
33+
34+
<AbC_def
35+
test="&#x0026;&#38;">
36+
bar
37+
baz
38+
</AbC_def>;
39+
40+
<a b={x ? <c /> : <d />} />;
41+
42+
<a>{}</a>;
43+
44+
<a>{/* this is a comment */}</a>;
45+
46+
<div>@test content</div>;
47+
48+
<div><br />7x invalid-js-identifier</div>;
49+
50+
<LeftRight left=<a /> right=<b>monkeys /> gorillas</b> />;
51+
~
52+
!!! error TS1005: '{' expected.
53+
~~~~~
54+
!!! error TS1005: '}' expected.
55+
~
56+
!!! error TS1005: '{' expected.
57+
~
58+
!!! error TS1109: Expression expected.
59+
~
60+
!!! error TS1109: Expression expected.
61+
62+
<a.b></a.b>;
63+
~
64+
!!! error TS1003: Identifier expected.
65+
~~
66+
!!! error TS1109: Expression expected.
67+
~
68+
!!! error TS1109: Expression expected.
69+
70+
<a.b.c></a.b.c>;
71+
72+
(<div />) < x;
73+
74+
<div {...props} />;
75+
76+
<div {...props} post="attribute" />;
77+
78+
<div pre="leading" pre2="attribute" {...props}></div>;
79+
80+
<a> </a>;
81+

‎tests/baselines/reference/jsxEsprimaFbTestSuite.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ baz
7171
<a></a>;
7272
<div>@test content</div>;
7373
<div><br />7x invalid-js-identifier</div>;
74-
<LeftRight left=<a /> right=<b>monkeys /> gorillas</b>/>;
75-
<a.b></a.b>;
74+
<LeftRight left={<a />} right={<b>monkeys /> gorillas</b> / > }/>
75+
< a.b > ;
76+
a.b > ;
7677
<a.b.c></a.b.c>;
7778
(<div />) < x;
7879
<div {...props}/>;

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,13 @@ tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(28,10): error TS2304:
6565
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(28,28): error TS1005: '>' expected.
6666
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(28,29): error TS1109: Expression expected.
6767
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(32,6): error TS1005: '{' expected.
68-
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(33,7): error TS1003: Identifier expected.
68+
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(33,6): error TS1005: '{' expected.
69+
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(33,7): error TS1109: Expression expected.
6970
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,4): error TS1003: Identifier expected.
7071
tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,21): error TS17002: Expected corresponding JSX closing tag for 'a'.
7172

7273

73-
==== tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx (70 errors) ====
74+
==== tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx (71 errors) ====
7475
declare var React: any;
7576

7677
</>;
@@ -238,8 +239,10 @@ tests/cases/conformance/jsx/jsxInvalidEsprimaTestSuite.tsx(35,21): error TS17002
238239
~
239240
!!! error TS1005: '{' expected.
240241
<a b=<}>;
242+
~
243+
!!! error TS1005: '{' expected.
241244
~
242-
!!! error TS1003: Identifier expected.
245+
!!! error TS1109: Expression expected.
243246
<a>}</a>;
244247
<a .../*hai*/asdf/>;
245248
~~~

‎tests/baselines/reference/jsxInvalidEsprimaTestSuite.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,6 @@ var x = <div>one</div> /* intervening comment */ /* intervening comment */ <div>
7676
<a>></a>;
7777
<a> ></a>;
7878
<a b=>;
79-
<a b=< />/>}>;
79+
<a b={ < }>;
8080
<a>}</a>;
81-
<a /> .../*hai*/asdf/>;</></></>;
81+
<a /> .../*hai*/asdf/>;</></></></>;

0 commit comments

Comments
 (0)