Skip to content
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

[Master] Type checking JSX children #15160

Merged
merged 24 commits into from
Apr 21, 2017
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 112 additions & 64 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -2099,6 +2099,10 @@
"category": "Error",
"code": 2707
},
"props.children are specified twice. The attribute named 'children' will be overwritten.": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name for children should be variable, and i would not include props here.

"category": "Error",
"code": 2708
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2457,7 +2457,7 @@ namespace ts {
let indentation: number;
for (const line of lines) {
for (let i = 0; i < line.length && (indentation === undefined || i < indentation); i++) {
if (!isWhiteSpace(line.charCodeAt(i))) {
if (!isWhiteSpaceLike(line.charCodeAt(i))) {
if (indentation === undefined || i < indentation) {
indentation = i;
break;
Expand Down
7 changes: 6 additions & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3826,13 +3826,15 @@ namespace ts {

function parseJsxText(): JsxText {
const node = <JsxText>createNode(SyntaxKind.JsxText, scanner.getStartPos());
node.containsOnlyWhiteSpaces = currentToken === SyntaxKind.JsxTextAllWhiteSpaces;
currentToken = scanner.scanJsxToken();
return finishNode(node);
}

function parseJsxChild(): JsxChild {
switch (token()) {
case SyntaxKind.JsxText:
case SyntaxKind.JsxTextAllWhiteSpaces:
return parseJsxText();
case SyntaxKind.OpenBraceToken:
return parseJsxExpression(/*inExpressionContext*/ false);
Expand Down Expand Up @@ -3862,7 +3864,10 @@ namespace ts {
else if (token() === SyntaxKind.ConflictMarkerTrivia) {
break;
}
result.push(parseJsxChild());
const child = parseJsxChild();
if (child) {
result.push(child);
}
}

result.end = scanner.getTokenPos();
Expand Down
29 changes: 24 additions & 5 deletions src/compiler/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ namespace ts {
return computeLineAndCharacterOfPosition(getLineStarts(sourceFile), position);
}

export function isWhiteSpace(ch: number): boolean {
export function isWhiteSpaceLike(ch: number): boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the rename?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it is not just WhiteSpace but Whitespace and empty line.

return isWhiteSpaceSingleLine(ch) || isLineBreak(ch);
}

Expand Down Expand Up @@ -510,7 +510,7 @@ namespace ts {
break;

default:
if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpace(ch))) {
if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpaceLike(ch))) {
pos++;
continue;
}
Expand Down Expand Up @@ -691,7 +691,7 @@ namespace ts {
}
break scan;
default:
if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpace(ch))) {
if (ch > CharacterCodes.maxAsciiCharacter && (isWhiteSpaceLike(ch))) {
if (hasPendingCommentRange && isLineBreak(ch)) {
pendingHasTrailingNewLine = true;
}
Expand Down Expand Up @@ -1723,8 +1723,12 @@ namespace ts {
return token = SyntaxKind.OpenBraceToken;
}

// First non-whitespace character on this line.
let firstNonWhitespace = 0;
// These initial values are special because the first line is:
// firstNonWhitespace = 0 to indicate that we want leading whitspace,

while (pos < end) {
pos++;
char = text.charCodeAt(pos);
if (char === CharacterCodes.openBrace) {
break;
Expand All @@ -1736,8 +1740,23 @@ namespace ts {
}
break;
}

// FirstNonWhitespace is 0, then we only see whitespaces so far. If we see a linebreak, we want to ignore that whitespaces.
// i.e (- : whitespace)
// <div>----
// </div> becomes <div></div>
//
// <div>----</div> becomes <div>----</div>
if (isLineBreak(char) && firstNonWhitespace === 0) {
firstNonWhitespace = -1;
}
else if (!isWhiteSpaceLike(char)) {
firstNonWhitespace = pos;
}
pos++;
}
return token = SyntaxKind.JsxText;

return firstNonWhitespace === -1 ? SyntaxKind.JsxTextAllWhiteSpaces : SyntaxKind.JsxText;
}

// Scans a JSX identifier; these differ from normal identifiers in that
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace ts {

/** ES6 Map interface. */
export interface Map<T> {
get(key: string): T;
get(key: string): T | undefined;
has(key: string): boolean;
set(key: string, value: T): this;
delete(key: string): boolean;
Expand Down Expand Up @@ -65,6 +65,7 @@ namespace ts {
NumericLiteral,
StringLiteral,
JsxText,
JsxTextAllWhiteSpaces,
RegularExpressionLiteral,
NoSubstitutionTemplateLiteral,
// Pseudo-literals
Expand Down Expand Up @@ -1572,6 +1573,7 @@ namespace ts {

export interface JsxText extends Node {
kind: SyntaxKind.JsxText;
containsOnlyWhiteSpaces: boolean;
parent?: JsxElement;
}

Expand Down Expand Up @@ -2975,6 +2977,8 @@ namespace ts {
NonPrimitive = 1 << 24, // intrinsic object type
/* @internal */
JsxAttributes = 1 << 25, // Jsx attributes type
/* @internal */
ContainsSynthesizedJsxChildren = 1 << 26, // Jsx attributes type contains synthesized children property from Jsx element's children

/* @internal */
Nullable = Undefined | Null,
Expand Down
2 changes: 1 addition & 1 deletion src/services/formatting/smartIndenter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ namespace ts.formatting {
let current = position;
while (current > 0) {
const char = sourceFile.text.charCodeAt(current);
if (!isWhiteSpace(char)) {
if (!isWhiteSpaceLike(char)) {
break;
}
current--;
Expand Down
2 changes: 1 addition & 1 deletion src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ namespace ts.textChanges {
if (force || !isTrivia(s)) {
this.lastNonTriviaPosition = this.writer.getTextPos();
let i = 0;
while (isWhiteSpace(s.charCodeAt(s.length - i - 1))) {
while (isWhiteSpaceLike(s.charCodeAt(s.length - i - 1))) {
i++;
}
// trim trailing whitespaces
Expand Down
2 changes: 1 addition & 1 deletion src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1381,7 +1381,7 @@ namespace ts {
}

export function getFirstNonSpaceCharacterPosition(text: string, position: number) {
while (isWhiteSpace(text.charCodeAt(position))) {
while (isWhiteSpaceLike(text.charCodeAt(position))) {
position += 1;
}
return position;
Expand Down
39 changes: 39 additions & 0 deletions tests/baselines/reference/checkJsxChildrenProperty1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//// [file.tsx]
import React = require('react');

interface Prop {
a: number,
b: string,
children: string | JSX.Element
}

function Comp(p: Prop) {
return <div>{p.b}</div>;
}

// OK
let k = <Comp a={10} b="hi" children ="lol" />;
let k1 =
<Comp a={10} b="hi">
hi hi hi!
</Comp>;
let k2 =
<Comp a={10} b="hi">
<div>hi hi hi!</div>
</Comp>;

//// [file.jsx]
"use strict";
exports.__esModule = true;
var React = require("react");
function Comp(p) {
return <div>{p.b}</div>;
}
// OK
var k = <Comp a={10} b="hi" children="lol"/>;
var k1 = <Comp a={10} b="hi">
hi hi hi!
</Comp>;
var k2 = <Comp a={10} b="hi">
<div>hi hi hi!</div>
</Comp>;
67 changes: 67 additions & 0 deletions tests/baselines/reference/checkJsxChildrenProperty1.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
=== tests/cases/conformance/jsx/file.tsx ===
import React = require('react');
>React : Symbol(React, Decl(file.tsx, 0, 0))

interface Prop {
>Prop : Symbol(Prop, Decl(file.tsx, 0, 32))

a: number,
>a : Symbol(Prop.a, Decl(file.tsx, 2, 16))

b: string,
>b : Symbol(Prop.b, Decl(file.tsx, 3, 14))

children: string | JSX.Element
>children : Symbol(Prop.children, Decl(file.tsx, 4, 14))
>JSX : Symbol(JSX, Decl(react.d.ts, 2352, 1))
>Element : Symbol(JSX.Element, Decl(react.d.ts, 2355, 27))
}

function Comp(p: Prop) {
>Comp : Symbol(Comp, Decl(file.tsx, 6, 1))
>p : Symbol(p, Decl(file.tsx, 8, 14))
>Prop : Symbol(Prop, Decl(file.tsx, 0, 32))

return <div>{p.b}</div>;
>div : Symbol(JSX.IntrinsicElements.div, Decl(react.d.ts, 2397, 45))
>p.b : Symbol(Prop.b, Decl(file.tsx, 3, 14))
>p : Symbol(p, Decl(file.tsx, 8, 14))
>b : Symbol(Prop.b, Decl(file.tsx, 3, 14))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react.d.ts, 2397, 45))
}

// OK
let k = <Comp a={10} b="hi" children ="lol" />;
>k : Symbol(k, Decl(file.tsx, 13, 3))
>Comp : Symbol(Comp, Decl(file.tsx, 6, 1))
>a : Symbol(a, Decl(file.tsx, 13, 13))
>b : Symbol(b, Decl(file.tsx, 13, 20))
>children : Symbol(children, Decl(file.tsx, 13, 27))

let k1 =
>k1 : Symbol(k1, Decl(file.tsx, 14, 3))

<Comp a={10} b="hi">
>Comp : Symbol(Comp, Decl(file.tsx, 6, 1))
>a : Symbol(a, Decl(file.tsx, 15, 9))
>b : Symbol(b, Decl(file.tsx, 15, 16))

hi hi hi!
</Comp>;
>Comp : Symbol(Comp, Decl(file.tsx, 6, 1))

let k2 =
>k2 : Symbol(k2, Decl(file.tsx, 18, 3))

<Comp a={10} b="hi">
>Comp : Symbol(Comp, Decl(file.tsx, 6, 1))
>a : Symbol(a, Decl(file.tsx, 19, 9))
>b : Symbol(b, Decl(file.tsx, 19, 16))

<div>hi hi hi!</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react.d.ts, 2397, 45))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react.d.ts, 2397, 45))

</Comp>;
>Comp : Symbol(Comp, Decl(file.tsx, 6, 1))

75 changes: 75 additions & 0 deletions tests/baselines/reference/checkJsxChildrenProperty1.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
=== tests/cases/conformance/jsx/file.tsx ===
import React = require('react');
>React : typeof React

interface Prop {
>Prop : Prop

a: number,
>a : number

b: string,
>b : string

children: string | JSX.Element
>children : string | JSX.Element
>JSX : any
>Element : JSX.Element
}

function Comp(p: Prop) {
>Comp : (p: Prop) => JSX.Element
>p : Prop
>Prop : Prop

return <div>{p.b}</div>;
><div>{p.b}</div> : JSX.Element
>div : any
>p.b : string
>p : Prop
>b : string
>div : any
}

// OK
let k = <Comp a={10} b="hi" children ="lol" />;
>k : JSX.Element
><Comp a={10} b="hi" children ="lol" /> : JSX.Element
>Comp : (p: Prop) => JSX.Element
>a : number
>10 : 10
>b : string
>children : string

let k1 =
>k1 : JSX.Element

<Comp a={10} b="hi">
><Comp a={10} b="hi"> hi hi hi! </Comp> : JSX.Element
>Comp : (p: Prop) => JSX.Element
>a : number
>10 : 10
>b : string

hi hi hi!
</Comp>;
>Comp : (p: Prop) => JSX.Element

let k2 =
>k2 : JSX.Element

<Comp a={10} b="hi">
><Comp a={10} b="hi"> <div>hi hi hi!</div> </Comp> : JSX.Element
>Comp : (p: Prop) => JSX.Element
>a : number
>10 : 10
>b : string

<div>hi hi hi!</div>
><div>hi hi hi!</div> : JSX.Element
>div : any
>div : any

</Comp>;
>Comp : (p: Prop) => JSX.Element

Loading