Skip to content

Do not consider empty jsx expressions semantically important children #41166

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

Merged
merged 3 commits into from
Oct 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15825,10 +15825,6 @@ namespace ts {
}
}

function getSemanticJsxChildren(children: NodeArray<JsxChild>) {
return filter(children, i => !isJsxText(i) || !i.containsOnlyTriviaWhiteSpaces);
}

function elaborateJsxComponents(
node: JsxAttributes,
source: Type,
Expand Down Expand Up @@ -24985,6 +24981,9 @@ namespace ts {
childrenTypes.push(stringType);
}
}
else if (child.kind === SyntaxKind.JsxExpression && !child.expression) {
continue; // empty jsx expressions don't *really* count as present children
}
else {
childrenTypes.push(checkExpressionForMutableLocation(child, checkMode));
}
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/transformers/jsx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ namespace ts {
}

function convertJsxChildrenToChildrenPropObject(children: readonly JsxChild[]) {
const nonWhitespaceChildren = filter(children, c => !isJsxText(c) || !c.containsOnlyTriviaWhiteSpaces);
const nonWhitespaceChildren = getSemanticJsxChildren(children);
if (length(nonWhitespaceChildren) === 1) {
const result = transformJsxChildToExpression(nonWhitespaceChildren[0]);
return result && factory.createObjectLiteralExpression([
Expand Down Expand Up @@ -244,7 +244,7 @@ namespace ts {
objectProperties = singleOrUndefined(segments) || emitHelpers().createAssignHelper(segments);
}

return visitJsxOpeningLikeElementOrFragmentJSX(tagName, objectProperties, keyAttr, length(filter(children, c => !isJsxText(c) || !c.containsOnlyTriviaWhiteSpaces)), isChild, location);
return visitJsxOpeningLikeElementOrFragmentJSX(tagName, objectProperties, keyAttr, length(getSemanticJsxChildren(children || emptyArray)), isChild, location);
}

function visitJsxOpeningLikeElementOrFragmentJSX(tagName: Expression, objectProperties: Expression, keyAttr: JsxAttribute | undefined, childrenLength: number, isChild: boolean, location: TextRange) {
Expand Down Expand Up @@ -336,7 +336,7 @@ namespace ts {
getImplicitJsxFragmentReference(),
childrenProps || factory.createObjectLiteralExpression([]),
/*keyAttr*/ undefined,
length(filter(children, c => !isJsxText(c) || !c.containsOnlyTriviaWhiteSpaces)),
length(getSemanticJsxChildren(children)),
isChild,
location
);
Expand Down
13 changes: 13 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3621,6 +3621,19 @@ namespace ts {
return -1;
}

export function getSemanticJsxChildren(children: readonly JsxChild[]) {
return filter(children, i => {
switch (i.kind) {
case SyntaxKind.JsxExpression:
return !!i.expression;
case SyntaxKind.JsxText:
return !i.containsOnlyTriviaWhiteSpaces;
default:
return true;
}
});
}

export function createDiagnosticCollection(): DiagnosticCollection {
let nonFileDiagnostics = [] as Diagnostic[] as SortedArray<Diagnostic>; // See GH#19873
const filesWithDiagnostics = [] as string[] as SortedArray<string>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [jsxEmptyExpressionNotCountedAsChild.tsx]
/// <reference path="/.lib/react16.d.ts" />
import * as React from 'react'

interface Props {
children: React.ReactElement<any>
}

function Wrapper(props: Props) {
return <div>{props.children}</div>
}

const element = (
<Wrapper>
{/* comment */}
<div>Hello</div>
</Wrapper>
)

//// [jsxEmptyExpressionNotCountedAsChild.js]
"use strict";
exports.__esModule = true;
/// <reference path="react16.d.ts" />
var React = require("react");
function Wrapper(props) {
return React.createElement("div", null, props.children);
}
var element = (React.createElement(Wrapper, null,
React.createElement("div", null, "Hello")));
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
=== tests/cases/compiler/jsxEmptyExpressionNotCountedAsChild.tsx ===
/// <reference path="react16.d.ts" />
import * as React from 'react'
>React : Symbol(React, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 6))

interface Props {
>Props : Symbol(Props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 30))

children: React.ReactElement<any>
>children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>React : Symbol(React, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 6))
>ReactElement : Symbol(React.ReactElement, Decl(react16.d.ts, 135, 9))
}

function Wrapper(props: Props) {
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))
>props : Symbol(props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 7, 17))
>Props : Symbol(Props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 30))

return <div>{props.children}</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>props.children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>props : Symbol(props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 7, 17))
>children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
}

const element = (
>element : Symbol(element, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 11, 5))

<Wrapper>
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))

{/* comment */}
<div>Hello</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))

</Wrapper>
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))

)
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
=== tests/cases/compiler/jsxEmptyExpressionNotCountedAsChild.tsx ===
/// <reference path="react16.d.ts" />
import * as React from 'react'
>React : typeof React

interface Props {
children: React.ReactElement<any>
>children : React.ReactElement<any>
>React : any
}

function Wrapper(props: Props) {
>Wrapper : (props: Props) => JSX.Element
>props : Props

return <div>{props.children}</div>
><div>{props.children}</div> : JSX.Element
>div : any
>props.children : React.ReactElement<any>
>props : Props
>children : React.ReactElement<any>
>div : any
}

const element = (
>element : JSX.Element
>( <Wrapper> {/* comment */} <div>Hello</div> </Wrapper>) : JSX.Element

<Wrapper>
><Wrapper> {/* comment */} <div>Hello</div> </Wrapper> : JSX.Element
>Wrapper : (props: Props) => JSX.Element

{/* comment */}
<div>Hello</div>
><div>Hello</div> : JSX.Element
>div : any
>div : any

</Wrapper>
>Wrapper : (props: Props) => JSX.Element

)
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//// [jsxEmptyExpressionNotCountedAsChild.tsx]
/// <reference path="/.lib/react16.d.ts" />
import * as React from 'react'

interface Props {
children: React.ReactElement<any>
}

function Wrapper(props: Props) {
return <div>{props.children}</div>
}

const element = (
<Wrapper>
{/* comment */}
<div>Hello</div>
</Wrapper>
)

//// [jsxEmptyExpressionNotCountedAsChild.js]
"use strict";
exports.__esModule = true;
var jsx_runtime_1 = require("react/jsx-runtime");
/// <reference path="react16.d.ts" />
var React = require("react");
function Wrapper(props) {
return jsx_runtime_1.jsx("div", { children: props.children }, void 0);
}
var element = (jsx_runtime_1.jsx(Wrapper, { children: jsx_runtime_1.jsx("div", { children: "Hello" }, void 0) }, void 0));
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
=== tests/cases/compiler/jsxEmptyExpressionNotCountedAsChild.tsx ===
/// <reference path="react16.d.ts" />
import * as React from 'react'
>React : Symbol(React, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 6))

interface Props {
>Props : Symbol(Props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 30))

children: React.ReactElement<any>
>children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>React : Symbol(React, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 6))
>ReactElement : Symbol(React.ReactElement, Decl(react16.d.ts, 135, 9))
}

function Wrapper(props: Props) {
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))
>props : Symbol(props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 7, 17))
>Props : Symbol(Props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 30))

return <div>{props.children}</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>props.children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>props : Symbol(props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 7, 17))
>children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
}

const element = (
>element : Symbol(element, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 11, 5))

<Wrapper>
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))

{/* comment */}
<div>Hello</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))

</Wrapper>
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))

)
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
=== tests/cases/compiler/jsxEmptyExpressionNotCountedAsChild.tsx ===
/// <reference path="react16.d.ts" />
import * as React from 'react'
>React : typeof React

interface Props {
children: React.ReactElement<any>
>children : React.ReactElement<any>
>React : any
}

function Wrapper(props: Props) {
>Wrapper : (props: Props) => JSX.Element
>props : Props

return <div>{props.children}</div>
><div>{props.children}</div> : JSX.Element
>div : any
>props.children : React.ReactElement<any>
>props : Props
>children : React.ReactElement<any>
>div : any
}

const element = (
>element : JSX.Element
>( <Wrapper> {/* comment */} <div>Hello</div> </Wrapper>) : JSX.Element

<Wrapper>
><Wrapper> {/* comment */} <div>Hello</div> </Wrapper> : JSX.Element
>Wrapper : (props: Props) => JSX.Element

{/* comment */}
<div>Hello</div>
><div>Hello</div> : JSX.Element
>div : any
>div : any

</Wrapper>
>Wrapper : (props: Props) => JSX.Element

)
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//// [jsxEmptyExpressionNotCountedAsChild.tsx]
/// <reference path="/.lib/react16.d.ts" />
import * as React from 'react'

interface Props {
children: React.ReactElement<any>
}

function Wrapper(props: Props) {
return <div>{props.children}</div>
}

const element = (
<Wrapper>
{/* comment */}
<div>Hello</div>
</Wrapper>
)

//// [jsxEmptyExpressionNotCountedAsChild.js]
"use strict";
exports.__esModule = true;
var jsx_dev_runtime_1 = require("react/jsx-dev-runtime");
var _jsxFileName = "tests/cases/compiler/jsxEmptyExpressionNotCountedAsChild.tsx";
/// <reference path="react16.d.ts" />
var React = require("react");
function Wrapper(props) {
return jsx_dev_runtime_1.jsxDEV("div", { children: props.children }, void 0, false, { fileName: _jsxFileName, lineNumber: 9, columnNumber: 11 }, this);
}
var element = (jsx_dev_runtime_1.jsxDEV(Wrapper, { children: jsx_dev_runtime_1.jsxDEV("div", { children: "Hello" }, void 0, false, { fileName: _jsxFileName, lineNumber: 15, columnNumber: 6 }, this) }, void 0, false, { fileName: _jsxFileName, lineNumber: 12, columnNumber: 18 }, this));
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
=== tests/cases/compiler/jsxEmptyExpressionNotCountedAsChild.tsx ===
/// <reference path="react16.d.ts" />
import * as React from 'react'
>React : Symbol(React, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 6))

interface Props {
>Props : Symbol(Props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 30))

children: React.ReactElement<any>
>children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>React : Symbol(React, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 6))
>ReactElement : Symbol(React.ReactElement, Decl(react16.d.ts, 135, 9))
}

function Wrapper(props: Props) {
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))
>props : Symbol(props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 7, 17))
>Props : Symbol(Props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 1, 30))

return <div>{props.children}</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>props.children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>props : Symbol(props, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 7, 17))
>children : Symbol(Props.children, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 3, 17))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
}

const element = (
>element : Symbol(element, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 11, 5))

<Wrapper>
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))

{/* comment */}
<div>Hello</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))

</Wrapper>
>Wrapper : Symbol(Wrapper, Decl(jsxEmptyExpressionNotCountedAsChild.tsx, 5, 1))

)
Loading