Skip to content

wrap JSX element attributes in expression#6006

Merged
hzoo merged 1 commit intobabel:7.0from
noahlemen:7.0
Jul 25, 2017
Merged

wrap JSX element attributes in expression#6006
hzoo merged 1 commit intobabel:7.0from
noahlemen:7.0

Conversation

@noahlemen
Copy link
Copy Markdown
Member

@noahlemen noahlemen commented Jul 25, 2017

Q A
Patch: Bug Fix? ✔️
Major: Breaking Change?
Minor: New Feature?
Deprecations?
Spec Compliancy?
Tests Added/Pass? ✔️ ✔️
Fixed Tickets Fixes #6004
License MIT
Doc PR
Dependency Changes

Adds JSXAttribute visitor function in https://github.com/babel/babel/blob/7.0/packages/babel-plugin-transform-react-jsx/src/index.js

If the value of a JSXAttribute is a JSXElement, wrap it in a JSXExpressionContainer

@mention-bot
Copy link
Copy Markdown

mention-bot commented Jul 25, 2017

@kedromelon, thanks for your PR! By analyzing the history of the files in this pull request, we identified @existentialism, @hzoo and @chicoxyzzy to be potential reviewers.

@noahlemen
Copy link
Copy Markdown
Member Author

noahlemen commented Jul 25, 2017 •
edited
Loading

@hzoo mentioned in #6004 (comment) that it might be good to document this in the readme -- any suggestions how to give an example of this? Thought about it for a while but couldn't come up with anything that seemed coherent.

Also, I added the fixture mentioned in that issue -- are there others that would be good to add as well?

(Thanks for the help everyone!)

@noahlemen
add JSXAttribute visitor function, wraps JSXElement attributes in a J…
0a3b8de 
…SXExpressionContainer; also adds test fixtures
@existentialism existentialism added area: react PR: Bug Fix 🐛 A type of pull request used for our changelog categories labels Jul 25, 2017
@hzoo hzoo merged commit 51a2936 into babel:7.0 Jul 25, 2017
@existentialism
Copy link
Copy Markdown
Member

existentialism commented Jul 25, 2017

@kedromelon awesome work!

@hzoo
Copy link
Copy Markdown
Member

hzoo commented Jul 25, 2017

Wow nice work 👍! Creative use of checkmarks for the issue description too haha. We don't have to add the example in the readme but figured it could be useful to point out somewhere (maybe in react docs instead) Looking forward to more of your contributions!!

@noahlemen
Copy link
Copy Markdown
Member Author

noahlemen commented Jul 25, 2017

thanks!! looking forward to contributing more 🙂

good point about the react docs -- I'll see if it makes more sense to document this there

@hzoo
Copy link
Copy Markdown
Member

hzoo commented Jul 25, 2017

And maybe we/they didn't want to encourage it 😄 , and given that it's always been a bug seems like no one is using it?

This was referenced Jul 26, 2017
Closed
Closed
@loganfsmyth loganfsmyth mentioned this pull request Aug 15, 2017
Open
@goodmind goodmind mentioned this pull request Mar 9, 2019
Open
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jridgewell jridgewell jridgewell approved these changes

Assignees

No one assigned

Labels

area: react outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@noahlemen @mention-bot @existentialism @hzoo @jridgewell