Skip to content

Ensure all JSX spread properties get visited in ES2018+ #55859

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.

Sign up for GitHub

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
jakebailey merged 1 commit into from
Oct 27, 2023
Merged

Ensure all JSX spread properties get visited in ES2018+ #55859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/compiler/transformers/jsx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
isJsxSelfClosingElement,
isJsxSpreadAttribute,
isLineBreak,
isObjectLiteralElementLike,
isObjectLiteralExpression,
isPropertyAssignment,
isSourceFile,
Expand Down Expand Up @@ -63,6 +64,7 @@ import {
ObjectLiteralElementLike,
ObjectLiteralExpression,
PropertyAssignment,
sameMap,
ScriptTarget,
setIdentifierGeneratedImportReference,
setParentRecursive,
Expand Down Expand Up @@ -444,7 +446,7 @@ export function transformJsx(context: TransformationContext): (x: SourceFile | B

function transformJsxSpreadAttributeToProps(node: JsxSpreadAttribute) {
if (isObjectLiteralExpression(node.expression) && !hasProto(node.expression)) {
return node.expression.properties;
return sameMap(node.expression.properties, p => Debug.checkDefined(visitNode(p, visitor, isObjectLiteralElementLike)));
Copy link
Member

@jakebailey jakebailey Sep 26, 2023

Choose a reason for hiding this comment

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

This seems reasonable, but I can't help but notice that sameMap is never used in **/transformers/**, so I get a bad feeling that there's some other canonical method for doing this...

Copy link
Contributor Author

@Andarist Andarist Sep 27, 2023

Choose a reason for hiding this comment

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

Using this helper wasn't super intentional on my side. I just knew that there is such an util function and since this was returned as-is here before I decided to use it here (as they were returned as-is returning the same array shouldn't be the problem).

I can change this to a regular map though - the outcome would be the same.

Copy link
Member

@jakebailey jakebailey Sep 27, 2023

Choose a reason for hiding this comment

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

This does seem like the helper to use, I'm just surprised we haven't used it already...

}
return factory.createSpreadAssignment(Debug.checkDefined(visitNode(node.expression, visitor, isExpression)));
}
Expand Down
0 ...selines/reference/jsxSpreadTag.errors.txt → ...ce/jsxSpreadTag(target=es2015).errors.txt
View file
Open in desktop
File renamed without changes.
0 tests/baselines/reference/jsxSpreadTag.js → .../reference/jsxSpreadTag(target=es2015).js
View file
Open in desktop
File renamed without changes.
0 .../baselines/reference/jsxSpreadTag.symbols → ...rence/jsxSpreadTag(target=es2015).symbols
View file
Open in desktop
File renamed without changes.
0 tests/baselines/reference/jsxSpreadTag.types → ...ference/jsxSpreadTag(target=es2015).types
View file
Open in desktop
File renamed without changes.
36 changes: 36 additions & 0 deletions tests/baselines/reference/jsxSpreadTag(target=esnext).errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/a.tsx(5,13): error TS2304: Cannot find name 'Comp'.
/a.tsx(9,13): error TS2304: Cannot find name 'Comp'.
/a.tsx(13,13): error TS2304: Cannot find name 'Comp'.
/a.tsx(17,13): error TS2304: Cannot find name 'Comp'.


==== /a.tsx (4 errors) ====
declare const React: any;

const t1 = <div {...<span />} />;
const t2 = <div {...<span className="foo" />} />;
const t3 = <Comp
~~~~
!!! error TS2304: Cannot find name 'Comp'.
right={<div>x</div>}
{...{ wrong: <div>x</div>}}
/>;
const t4 = <Comp
~~~~
!!! error TS2304: Cannot find name 'Comp'.
right={<div>x</div>}
{...{ wrong() { return <div>x</div>; }}}
/>;
const t5 = <Comp
~~~~
!!! error TS2304: Cannot find name 'Comp'.
right={<div>x</div>}
{...{ get wrong() { return <div>x</div>; }}}
/>;
const t6 = <Comp
~~~~
!!! error TS2304: Cannot find name 'Comp'.
right={<div>x</div>}
{...{ set wrong(s) { let a = <div>x</div>; }}}
/>;

32 changes: 32 additions & 0 deletions tests/baselines/reference/jsxSpreadTag(target=esnext).js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//// [tests/cases/compiler/jsxSpreadTag.ts] ////

//// [a.tsx]
declare const React: any;

const t1 = <div {...<span />} />;
const t2 = <div {...<span className="foo" />} />;
const t3 = <Comp
right={<div>x</div>}
{...{ wrong: <div>x</div>}}
/>;
const t4 = <Comp
right={<div>x</div>}
{...{ wrong() { return <div>x</div>; }}}
/>;
const t5 = <Comp
right={<div>x</div>}
{...{ get wrong() { return <div>x</div>; }}}
/>;
const t6 = <Comp
right={<div>x</div>}
{...{ set wrong(s) { let a = <div>x</div>; }}}
/>;


//// [a.js]
const t1 = React.createElement("div", { ...React.createElement("span", null) });
const t2 = React.createElement("div", { ...React.createElement("span", { className: "foo" }) });
const t3 = React.createElement(Comp, { right: React.createElement("div", null, "x"), wrong: React.createElement("div", null, "x") });
const t4 = React.createElement(Comp, { right: React.createElement("div", null, "x"), wrong() { return React.createElement("div", null, "x"); } });
const t5 = React.createElement(Comp, { right: React.createElement("div", null, "x"), get wrong() { return React.createElement("div", null, "x"); } });
const t6 = React.createElement(Comp, { right: React.createElement("div", null, "x"), set wrong(s) { let a = React.createElement("div", null, "x"); } });
56 changes: 56 additions & 0 deletions tests/baselines/reference/jsxSpreadTag(target=esnext).symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
//// [tests/cases/compiler/jsxSpreadTag.ts] ////

=== /a.tsx ===
declare const React: any;
>React : Symbol(React, Decl(a.tsx, 0, 13))

const t1 = <div {...<span />} />;
>t1 : Symbol(t1, Decl(a.tsx, 2, 5))

const t2 = <div {...<span className="foo" />} />;
>t2 : Symbol(t2, Decl(a.tsx, 3, 5))
>className : Symbol(className, Decl(a.tsx, 3, 25))

const t3 = <Comp
>t3 : Symbol(t3, Decl(a.tsx, 4, 5))

right={<div>x</div>}
>right : Symbol(right, Decl(a.tsx, 4, 16))

{...{ wrong: <div>x</div>}}
>wrong : Symbol(wrong, Decl(a.tsx, 6, 9))

/>;
const t4 = <Comp
>t4 : Symbol(t4, Decl(a.tsx, 8, 5))

right={<div>x</div>}
>right : Symbol(right, Decl(a.tsx, 8, 16))

{...{ wrong() { return <div>x</div>; }}}
>wrong : Symbol(wrong, Decl(a.tsx, 10, 9))

/>;
const t5 = <Comp
>t5 : Symbol(t5, Decl(a.tsx, 12, 5))

right={<div>x</div>}
>right : Symbol(right, Decl(a.tsx, 12, 16))

{...{ get wrong() { return <div>x</div>; }}}
>wrong : Symbol(wrong, Decl(a.tsx, 14, 9))

/>;
const t6 = <Comp
>t6 : Symbol(t6, Decl(a.tsx, 16, 5))

right={<div>x</div>}
>right : Symbol(right, Decl(a.tsx, 16, 16))

{...{ set wrong(s) { let a = <div>x</div>; }}}
>wrong : Symbol(wrong, Decl(a.tsx, 18, 9))
>s : Symbol(s, Decl(a.tsx, 18, 20))
>a : Symbol(a, Decl(a.tsx, 18, 28))

/>;

100 changes: 100 additions & 0 deletions tests/baselines/reference/jsxSpreadTag(target=esnext).types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
//// [tests/cases/compiler/jsxSpreadTag.ts] ////

=== /a.tsx ===
declare const React: any;
>React : any

const t1 = <div {...<span />} />;
>t1 : any
><div {...<span />} /> : any
>div : any
><span /> : any
>span : any

const t2 = <div {...<span className="foo" />} />;
>t2 : any
><div {...<span className="foo" />} /> : any
>div : any
><span className="foo" /> : any
>span : any
>className : string

const t3 = <Comp
>t3 : any
><Comp right={<div>x</div>} {...{ wrong: <div>x</div>}}/> : any
>Comp : any

right={<div>x</div>}
>right : any
><div>x</div> : any
>div : any
>div : any

{...{ wrong: <div>x</div>}}
>{ wrong: <div>x</div>} : { wrong: any; }
>wrong : any
><div>x</div> : any
>div : any
>div : any

/>;
const t4 = <Comp
>t4 : any
><Comp right={<div>x</div>} {...{ wrong() { return <div>x</div>; }}}/> : any
>Comp : any

right={<div>x</div>}
>right : any
><div>x</div> : any
>div : any
>div : any

{...{ wrong() { return <div>x</div>; }}}
>{ wrong() { return <div>x</div>; }} : { wrong(): any; }
>wrong : () => any
><div>x</div> : any
>div : any
>div : any

/>;
const t5 = <Comp
>t5 : any
><Comp right={<div>x</div>} {...{ get wrong() { return <div>x</div>; }}}/> : any
>Comp : any

right={<div>x</div>}
>right : any
><div>x</div> : any
>div : any
>div : any

{...{ get wrong() { return <div>x</div>; }}}
>{ get wrong() { return <div>x</div>; }} : { readonly wrong: any; }
>wrong : any
><div>x</div> : any
>div : any
>div : any

/>;
const t6 = <Comp
>t6 : any
><Comp right={<div>x</div>} {...{ set wrong(s) { let a = <div>x</div>; }}}/> : any
>Comp : any

right={<div>x</div>}
>right : any
><div>x</div> : any
>div : any
>div : any

{...{ set wrong(s) { let a = <div>x</div>; }}}
>{ set wrong(s) { let a = <div>x</div>; }} : { wrong: any; }
>wrong : any
>s : any
>a : any
><div>x</div> : any
>div : any
>div : any

/>;

2 changes: 1 addition & 1 deletion tests/cases/compiler/jsxSpreadTag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @jsx: react
// @target: es2015
// @target: es2015,esnext
// @filename: /a.tsx

declare const React: any;
Expand Down