Skip to content

Transform JSX spread children #45693

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
Sep 24, 2021
Merged

Transform JSX spread children #45693

merged 3 commits into from
Sep 24, 2021

Conversation

jablko
Copy link
Contributor

@jablko jablko commented Sep 2, 2021

#11591 added support for parsing and emitting JSX spread children, however transformers/jsx.ts ignores JsxExpression#dotDotDotToken, so roughly it transforms <div>{...todos}</div> -> React.createElement("div", null, todos) vs. React.createElement("div", null, ...todos).

This isn't a problem for React.createElement() because that runtime treats both cases equivalently (it flattens children), however it is an issue for pickier runtimes.

This PR adds factory.createSpreadElement() to transformers/jsx.ts if JsxExpression#dotDotDotToken is defined.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 2, 2021
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

A new test that doesn't downlevel all the way to the __spreadArray helper (so targeting es6) would be nice, just to capture the difference simply. Other than that, this looks good.

@jablko
Copy link
Contributor Author

jablko commented Sep 15, 2021

@weswigham Thanks! Assuming that downleveling and JSX are independent, can we add @target: es6 to the existing test, to clarify the output? @sandersn mentioned here that tsxSpreadChildrenInvalidType.tsx covers the jsx: react emit.

That's what I've done in the added commit. Otherwise I'd copy tests/cases/conformance/jsx/tsxSpreadChildren.tsx -> tsxSpreadChildren_es6.tsx and add:

// @jsx: react
// @target: es6

Not sure if retaining both targets (default and es6) adds anything though?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants