Skip to content
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

Do not quote JSX attribute keys for IdentifierName #8045

Merged
merged 1 commit into from
May 27, 2018

Conversation

arv
Copy link
Contributor

@arv arv commented May 25, 2018

Q                       A
Fixed Issues?
Patch: Bug Fix? Bug fix
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR
Any Dependency Changes? No
License MIT

Given the following

a = <F new/>

We used to generate:

a = React.createElement(F, {"new": true})

but now we generate

a = React.createElement(F, {new: true})

If you need to quote these (for ES3 for example, you can use transform-property-literals)

Given the following

```js
a = <F new/>
```

We used to generate:

```js
a = React.createElement(F, {"new": true})
```

but now we generate

```js
a = React.createElement(F, {new: true})
```

If you need to quote these (ie for ES3 you can use
transform-property-literals)
@babel-bot
Copy link
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8195/

1 similar comment
@babel-bot
Copy link
Collaborator

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8195/

@arv
Copy link
Contributor Author

arv commented May 25, 2018

What is the process for merging this?

@nicolo-ribaudo
Copy link
Member

Two approved reviews

@xtuc
Copy link
Member

xtuc commented May 26, 2018

I'm just wondering why we don't quote everything?

@arv
Copy link
Contributor Author

arv commented May 26, 2018

@xtuc Do you quote all your keys in object literals? In member expressions?

@xtuc
Copy link
Member

xtuc commented May 27, 2018

@arv quoting every key would simplify the logic here and it's in the generated code.

@jridgewell jridgewell merged commit 7846eae into babel:master May 27, 2018
@arv
Copy link
Contributor Author

arv commented May 27, 2018

@xtuc The code here is pretty simple and the gain in readability in the generated code makes it worth it.

Plus, quoting keys had some bad side effects for people using Closure Compiler further down their build pipeline.

@arv
Copy link
Contributor Author

arv commented May 27, 2018

Thanks, @jridgewell

@hzoo hzoo added PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release PR: Bug Fix 🐛 A type of pull request used for our changelog categories area: jsx labels May 28, 2018
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: jsx outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants