Skip to content

Commit

Permalink
Fix react-jsx spread children invalid emit (#46565)
Browse files Browse the repository at this point in the history 
* Fix react-jsx spread children invalid emit

* Update Baselines and/or Applied Lint Fixes

* Change childrenLength parameter -> isStaticChildren

Co-authored-by: TypeScript Bot <typescriptbot@microsoft.com>
  • Loading branch information
@jablko @typescript-bot
jablko and typescript-bot committed Oct 29, 2021
1 parent 3254358 commit 9b1ba8f
Show file tree
Hide file tree
Showing 18 changed files with 653 additions and 11 deletions.
40 changes: 30 additions & 10 deletions src/compiler/transformers/jsx.ts
View file
Expand Up @@ -26,12 +26,12 @@ namespace ts {
return currentFileState.filenameDeclaration.name;
}

function getJsxFactoryCalleePrimitive(childrenLength: number): "jsx" | "jsxs" | "jsxDEV" {
return compilerOptions.jsx === JsxEmit.ReactJSXDev ? "jsxDEV" : childrenLength > 1 ? "jsxs" : "jsx";
function getJsxFactoryCalleePrimitive(isStaticChildren: boolean): "jsx" | "jsxs" | "jsxDEV" {
return compilerOptions.jsx === JsxEmit.ReactJSXDev ? "jsxDEV" : isStaticChildren ? "jsxs" : "jsx";
}

function getJsxFactoryCallee(childrenLength: number) {
const type = getJsxFactoryCalleePrimitive(childrenLength);
function getJsxFactoryCallee(isStaticChildren: boolean) {
const type = getJsxFactoryCalleePrimitive(isStaticChildren);
return getImplicitImportForName(type);
}

Expand Down Expand Up @@ -206,7 +206,7 @@ namespace ts {

function convertJsxChildrenToChildrenPropAssignment(children: readonly JsxChild[]) {
const nonWhitespaceChildren = getSemanticJsxChildren(children);
if (length(nonWhitespaceChildren) === 1) {
if (length(nonWhitespaceChildren) === 1 && !(nonWhitespaceChildren[0] as JsxExpression).dotDotDotToken) {
const result = transformJsxChildToExpression(nonWhitespaceChildren[0]);
return result && factory.createPropertyAssignment("children", result);
}
Expand All @@ -221,16 +221,33 @@ namespace ts {
const attrs = keyAttr ? filter(node.attributes.properties, p => p !== keyAttr) : node.attributes.properties;
const objectProperties = length(attrs) ? transformJsxAttributesToObjectProps(attrs, childrenProp) :
factory.createObjectLiteralExpression(childrenProp ? [childrenProp] : emptyArray); // When there are no attributes, React wants {}
return visitJsxOpeningLikeElementOrFragmentJSX(tagName, objectProperties, keyAttr, length(getSemanticJsxChildren(children || emptyArray)), isChild, location);
return visitJsxOpeningLikeElementOrFragmentJSX(
tagName,
objectProperties,
keyAttr,
children || emptyArray,
isChild,
location
);
}

function visitJsxOpeningLikeElementOrFragmentJSX(tagName: Expression, objectProperties: Expression, keyAttr: JsxAttribute | undefined, childrenLength: number, isChild: boolean, location: TextRange) {
function visitJsxOpeningLikeElementOrFragmentJSX(
tagName: Expression,
objectProperties: Expression,
keyAttr: JsxAttribute | undefined,
children: readonly JsxChild[],
isChild: boolean,
location: TextRange
) {
const nonWhitespaceChildren = getSemanticJsxChildren(children);
const isStaticChildren =
length(nonWhitespaceChildren) > 1 || !!(nonWhitespaceChildren[0] as JsxExpression)?.dotDotDotToken;
const args: Expression[] = [tagName, objectProperties, !keyAttr ? factory.createVoidZero() : transformJsxAttributeInitializer(keyAttr.initializer)];
if (compilerOptions.jsx === JsxEmit.ReactJSXDev) {
const originalFile = getOriginalNode(currentSourceFile);
if (originalFile && isSourceFile(originalFile)) {
// isStaticChildren development flag
args.push(childrenLength > 1 ? factory.createTrue() : factory.createFalse());
args.push(isStaticChildren ? factory.createTrue() : factory.createFalse());
// __source development flag
const lineCol = getLineAndCharacterOfPosition(originalFile, location.pos);
args.push(factory.createObjectLiteralExpression([
Expand All @@ -242,7 +259,10 @@ namespace ts {
args.push(factory.createThis());
}
}
const element = setTextRange(factory.createCallExpression(getJsxFactoryCallee(childrenLength), /*typeArguments*/ undefined, args), location);
const element = setTextRange(
factory.createCallExpression(getJsxFactoryCallee(isStaticChildren), /*typeArguments*/ undefined, args),
location
);

if (isChild) {
startOnNewLine(element);
Expand Down Expand Up @@ -294,7 +314,7 @@ namespace ts {
getImplicitJsxFragmentReference(),
childrenProps || factory.createObjectLiteralExpression([]),
/*keyAttr*/ undefined,
length(getSemanticJsxChildren(children)),
children,
isChild,
location
);
Expand Down
0 ...drenInvalidType(target=es2015).errors.txt → ...dType(jsx=react,target=es2015).errors.txt
View file
File renamed without changes.
0 ...readChildrenInvalidType(target=es2015).js → ...enInvalidType(jsx=react,target=es2015).js
View file
File renamed without changes.
0 ...hildrenInvalidType(target=es2015).symbols → ...alidType(jsx=react,target=es2015).symbols
View file
File renamed without changes.
0 ...dChildrenInvalidType(target=es2015).types → ...nvalidType(jsx=react,target=es2015).types
View file
File renamed without changes.
0 ...hildrenInvalidType(target=es5).errors.txt → ...alidType(jsx=react,target=es5).errors.txt
View file
File renamed without changes.
0 ...xSpreadChildrenInvalidType(target=es5).js → ...ldrenInvalidType(jsx=react,target=es5).js
View file
File renamed without changes.
0 ...adChildrenInvalidType(target=es5).symbols → ...InvalidType(jsx=react,target=es5).symbols
View file
File renamed without changes.
0 ...readChildrenInvalidType(target=es5).types → ...enInvalidType(jsx=react,target=es5).types
View file
File renamed without changes.
41 changes: 41 additions & 0 deletions .../baselines/reference/tsxSpreadChildrenInvalidType(jsx=react-jsx,target=es2015).errors.txt
View file
@@ -0,0 +1,41 @@
tests/cases/conformance/jsx/tsxSpreadChildrenInvalidType.tsx(17,12): error TS2792: Cannot find module 'react/jsx-runtime'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?
tests/cases/conformance/jsx/tsxSpreadChildrenInvalidType.tsx(21,9): error TS2609: JSX spread child must be an array type.


==== tests/cases/conformance/jsx/tsxSpreadChildrenInvalidType.tsx (2 errors) ====
declare module JSX {
interface Element { }
interface IntrinsicElements {
[s: string]: any;
}
}
declare var React: any;

interface TodoProp {
id: number;
todo: string;
}
interface TodoListProps {
todos: TodoProp[];
}
function Todo(prop: { key: number, todo: string }) {
return <div>{prop.key.toString() + prop.todo}</div>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2792: Cannot find module 'react/jsx-runtime'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?
}
function TodoList({ todos }: TodoListProps) {
return <div>
{...<Todo key={todos[0].id} todo={todos[0].todo} />}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2609: JSX spread child must be an array type.
</div>;
}
function TodoListNoError({ todos }: TodoListProps) {
// any is not checked
return <div>
{...(<Todo key={todos[0].id} todo={todos[0].todo} /> as any)}
</div>;
}
let x: TodoListProps;
<TodoList {...x}/>

47 changes: 47 additions & 0 deletions tests/baselines/reference/tsxSpreadChildrenInvalidType(jsx=react-jsx,target=es2015).js
View file
@@ -0,0 +1,47 @@
//// [tsxSpreadChildrenInvalidType.tsx]
declare module JSX {
interface Element { }
interface IntrinsicElements {
[s: string]: any;
}
}
declare var React: any;

interface TodoProp {
id: number;
todo: string;
}
interface TodoListProps {
todos: TodoProp[];
}
function Todo(prop: { key: number, todo: string }) {
return <div>{prop.key.toString() + prop.todo}</div>;
}
function TodoList({ todos }: TodoListProps) {
return <div>
{...<Todo key={todos[0].id} todo={todos[0].todo} />}
</div>;
}
function TodoListNoError({ todos }: TodoListProps) {
// any is not checked
return <div>
{...(<Todo key={todos[0].id} todo={todos[0].todo} /> as any)}
</div>;
}
let x: TodoListProps;
<TodoList {...x}/>


//// [tsxSpreadChildrenInvalidType.js]
function Todo(prop) {
return _jsx("div", { children: prop.key.toString() + prop.todo }, void 0);
}
function TodoList({ todos }) {
return _jsxs("div", { children: [..._jsx(Todo, { todo: todos[0].todo }, todos[0].id)] }, void 0);
}
function TodoListNoError({ todos }) {
// any is not checked
return _jsxs("div", { children: [..._jsx(Todo, { todo: todos[0].todo }, todos[0].id)] }, void 0);
}
let x;
_jsx(TodoList, Object.assign({}, x), void 0);
104 changes: 104 additions & 0 deletions tests/baselines/reference/tsxSpreadChildrenInvalidType(jsx=react-jsx,target=es2015).symbols
View file
@@ -0,0 +1,104 @@
=== tests/cases/conformance/jsx/tsxSpreadChildrenInvalidType.tsx ===
declare module JSX {
>JSX : Symbol(JSX, Decl(tsxSpreadChildrenInvalidType.tsx, 0, 0))

interface Element { }
>Element : Symbol(Element, Decl(tsxSpreadChildrenInvalidType.tsx, 0, 20))

interface IntrinsicElements {
>IntrinsicElements : Symbol(IntrinsicElements, Decl(tsxSpreadChildrenInvalidType.tsx, 1, 22))

[s: string]: any;
>s : Symbol(s, Decl(tsxSpreadChildrenInvalidType.tsx, 3, 3))
}
}
declare var React: any;
>React : Symbol(React, Decl(tsxSpreadChildrenInvalidType.tsx, 6, 11))

interface TodoProp {
>TodoProp : Symbol(TodoProp, Decl(tsxSpreadChildrenInvalidType.tsx, 6, 23))

id: number;
>id : Symbol(TodoProp.id, Decl(tsxSpreadChildrenInvalidType.tsx, 8, 20))

todo: string;
>todo : Symbol(TodoProp.todo, Decl(tsxSpreadChildrenInvalidType.tsx, 9, 15))
}
interface TodoListProps {
>TodoListProps : Symbol(TodoListProps, Decl(tsxSpreadChildrenInvalidType.tsx, 11, 1))

todos: TodoProp[];
>todos : Symbol(TodoListProps.todos, Decl(tsxSpreadChildrenInvalidType.tsx, 12, 25))
>TodoProp : Symbol(TodoProp, Decl(tsxSpreadChildrenInvalidType.tsx, 6, 23))
}
function Todo(prop: { key: number, todo: string }) {
>Todo : Symbol(Todo, Decl(tsxSpreadChildrenInvalidType.tsx, 14, 1))
>prop : Symbol(prop, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 14))
>key : Symbol(key, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 21))
>todo : Symbol(todo, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 34))

return <div>{prop.key.toString() + prop.todo}</div>;
>div : Symbol(JSX.IntrinsicElements, Decl(tsxSpreadChildrenInvalidType.tsx, 1, 22))
>prop.key.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>prop.key : Symbol(key, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 21))
>prop : Symbol(prop, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 14))
>key : Symbol(key, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 21))
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>prop.todo : Symbol(todo, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 34))
>prop : Symbol(prop, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 14))
>todo : Symbol(todo, Decl(tsxSpreadChildrenInvalidType.tsx, 15, 34))
>div : Symbol(JSX.IntrinsicElements, Decl(tsxSpreadChildrenInvalidType.tsx, 1, 22))
}
function TodoList({ todos }: TodoListProps) {
>TodoList : Symbol(TodoList, Decl(tsxSpreadChildrenInvalidType.tsx, 17, 1))
>todos : Symbol(todos, Decl(tsxSpreadChildrenInvalidType.tsx, 18, 19))
>TodoListProps : Symbol(TodoListProps, Decl(tsxSpreadChildrenInvalidType.tsx, 11, 1))

return <div>
>div : Symbol(JSX.IntrinsicElements, Decl(tsxSpreadChildrenInvalidType.tsx, 1, 22))

{...<Todo key={todos[0].id} todo={todos[0].todo} />}
>Todo : Symbol(Todo, Decl(tsxSpreadChildrenInvalidType.tsx, 14, 1))
>key : Symbol(key, Decl(tsxSpreadChildrenInvalidType.tsx, 20, 17))
>todos[0].id : Symbol(TodoProp.id, Decl(tsxSpreadChildrenInvalidType.tsx, 8, 20))
>todos : Symbol(todos, Decl(tsxSpreadChildrenInvalidType.tsx, 18, 19))
>id : Symbol(TodoProp.id, Decl(tsxSpreadChildrenInvalidType.tsx, 8, 20))
>todo : Symbol(todo, Decl(tsxSpreadChildrenInvalidType.tsx, 20, 35))
>todos[0].todo : Symbol(TodoProp.todo, Decl(tsxSpreadChildrenInvalidType.tsx, 9, 15))
>todos : Symbol(todos, Decl(tsxSpreadChildrenInvalidType.tsx, 18, 19))
>todo : Symbol(TodoProp.todo, Decl(tsxSpreadChildrenInvalidType.tsx, 9, 15))

</div>;
>div : Symbol(JSX.IntrinsicElements, Decl(tsxSpreadChildrenInvalidType.tsx, 1, 22))
}
function TodoListNoError({ todos }: TodoListProps) {
>TodoListNoError : Symbol(TodoListNoError, Decl(tsxSpreadChildrenInvalidType.tsx, 22, 1))
>todos : Symbol(todos, Decl(tsxSpreadChildrenInvalidType.tsx, 23, 26))
>TodoListProps : Symbol(TodoListProps, Decl(tsxSpreadChildrenInvalidType.tsx, 11, 1))

// any is not checked
return <div>
>div : Symbol(JSX.IntrinsicElements, Decl(tsxSpreadChildrenInvalidType.tsx, 1, 22))

{...(<Todo key={todos[0].id} todo={todos[0].todo} /> as any)}
>Todo : Symbol(Todo, Decl(tsxSpreadChildrenInvalidType.tsx, 14, 1))
>key : Symbol(key, Decl(tsxSpreadChildrenInvalidType.tsx, 26, 18))
>todos[0].id : Symbol(TodoProp.id, Decl(tsxSpreadChildrenInvalidType.tsx, 8, 20))
>todos : Symbol(todos, Decl(tsxSpreadChildrenInvalidType.tsx, 23, 26))
>id : Symbol(TodoProp.id, Decl(tsxSpreadChildrenInvalidType.tsx, 8, 20))
>todo : Symbol(todo, Decl(tsxSpreadChildrenInvalidType.tsx, 26, 36))
>todos[0].todo : Symbol(TodoProp.todo, Decl(tsxSpreadChildrenInvalidType.tsx, 9, 15))
>todos : Symbol(todos, Decl(tsxSpreadChildrenInvalidType.tsx, 23, 26))
>todo : Symbol(TodoProp.todo, Decl(tsxSpreadChildrenInvalidType.tsx, 9, 15))

</div>;
>div : Symbol(JSX.IntrinsicElements, Decl(tsxSpreadChildrenInvalidType.tsx, 1, 22))
}
let x: TodoListProps;
>x : Symbol(x, Decl(tsxSpreadChildrenInvalidType.tsx, 29, 3))
>TodoListProps : Symbol(TodoListProps, Decl(tsxSpreadChildrenInvalidType.tsx, 11, 1))

<TodoList {...x}/>
>TodoList : Symbol(TodoList, Decl(tsxSpreadChildrenInvalidType.tsx, 17, 1))
>x : Symbol(x, Decl(tsxSpreadChildrenInvalidType.tsx, 29, 3))

108 changes: 108 additions & 0 deletions tests/baselines/reference/tsxSpreadChildrenInvalidType(jsx=react-jsx,target=es2015).types
View file
@@ -0,0 +1,108 @@
=== tests/cases/conformance/jsx/tsxSpreadChildrenInvalidType.tsx ===
declare module JSX {
interface Element { }
interface IntrinsicElements {
[s: string]: any;
>s : string
}
}
declare var React: any;
>React : any

interface TodoProp {
id: number;
>id : number

todo: string;
>todo : string
}
interface TodoListProps {
todos: TodoProp[];
>todos : TodoProp[]
}
function Todo(prop: { key: number, todo: string }) {
>Todo : (prop: { key: number; todo: string;}) => JSX.Element
>prop : { key: number; todo: string; }
>key : number
>todo : string

return <div>{prop.key.toString() + prop.todo}</div>;
><div>{prop.key.toString() + prop.todo}</div> : JSX.Element
>div : any
>prop.key.toString() + prop.todo : string
>prop.key.toString() : string
>prop.key.toString : (radix?: number) => string
>prop.key : number
>prop : { key: number; todo: string; }
>key : number
>toString : (radix?: number) => string
>prop.todo : string
>prop : { key: number; todo: string; }
>todo : string
>div : any
}
function TodoList({ todos }: TodoListProps) {
>TodoList : ({ todos }: TodoListProps) => JSX.Element
>todos : TodoProp[]

return <div>
><div> {...<Todo key={todos[0].id} todo={todos[0].todo} />} </div> : JSX.Element
>div : any

{...<Todo key={todos[0].id} todo={todos[0].todo} />}
><Todo key={todos[0].id} todo={todos[0].todo} /> : JSX.Element
>Todo : (prop: { key: number; todo: string; }) => JSX.Element
>key : number
>todos[0].id : number
>todos[0] : TodoProp
>todos : TodoProp[]
>0 : 0
>id : number
>todo : string
>todos[0].todo : string
>todos[0] : TodoProp
>todos : TodoProp[]
>0 : 0
>todo : string

</div>;
>div : any
}
function TodoListNoError({ todos }: TodoListProps) {
>TodoListNoError : ({ todos }: TodoListProps) => JSX.Element
>todos : TodoProp[]

// any is not checked
return <div>
><div> {...(<Todo key={todos[0].id} todo={todos[0].todo} /> as any)} </div> : JSX.Element
>div : any

{...(<Todo key={todos[0].id} todo={todos[0].todo} /> as any)}
>(<Todo key={todos[0].id} todo={todos[0].todo} /> as any) : any
><Todo key={todos[0].id} todo={todos[0].todo} /> as any : any
><Todo key={todos[0].id} todo={todos[0].todo} /> : JSX.Element
>Todo : (prop: { key: number; todo: string; }) => JSX.Element
>key : number
>todos[0].id : number
>todos[0] : TodoProp
>todos : TodoProp[]
>0 : 0
>id : number
>todo : string
>todos[0].todo : string
>todos[0] : TodoProp
>todos : TodoProp[]
>0 : 0
>todo : string

</div>;
>div : any
}
let x: TodoListProps;
>x : TodoListProps

<TodoList {...x}/>
><TodoList {...x}/> : JSX.Element
>TodoList : ({ todos }: TodoListProps) => JSX.Element
>x : TodoListProps

0 comments on commit 9b1ba8f

Please sign in to comment.