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

babel-types lists JSXIdentifier as an Expression #6960

Merged
merged 1 commit into from
Dec 18, 2017
Merged

babel-types lists JSXIdentifier as an Expression #6960

merged 1 commit into from
Dec 18, 2017

Conversation

tolmasky
Copy link
Contributor

@tolmasky tolmasky commented Dec 3, 2017

Per the JSX definition ( https://facebook.github.io/jsx/ ), only JSXElement and JSXFragment are actually Expressions.

Currently, Babel is treating JSXIdentifier, JSXMemberExpression, and JSXEmptyExpression as expressions as well, which
means Babel will for example incorrectly allow you to place these anywhere an expression is wanted. This fixes that.

Closes #6851.

Not sure if anything else is necessary, this seemed to be the place to edit.

@babel-bot
Copy link
Collaborator

babel-bot commented Dec 3, 2017

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

@babel babel deleted a comment from babel-bot Dec 3, 2017
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Thank you for this pr!
The correct way of modifying a node type is updating it in the babel-types package:

defineType("JSXIdentifier", {
builder: ["name"],
aliases: ["JSX", "Expression"],
fields: {
name: {
validate: assertValueType("string"),
},
},
});

Than you should run node scripts/generate-interfaces.js to update lib/types.js and node scripts/generate-babel-types-docs.js to update packages/babel-types/README.md

@tolmasky
Copy link
Contributor Author

tolmasky commented Dec 3, 2017

Thanks for the pointer! Per your advice I have modified the right file and then generated it using the script.

@nicolo-ribaudo
Copy link
Member

Did you also run node scripts/generate-babel-types-docs.js?

@tolmasky
Copy link
Contributor Author

tolmasky commented Dec 3, 2017

Nope, just added.

@tolmasky
Copy link
Contributor Author

Anything else I need to do? Just checking in.

…lement and JSXFragment are actually Expressions.

Currently, Babel is treating JSXIdentifier, JSXMemberExpression, and JSXEmptyExpression as expressions as well, which
means Babel will for example incorrectly allow you to place these anywhere an expression is wanted. This fixes that.

Closes #6851.
@existentialism existentialism added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Dec 18, 2017
@existentialism existentialism merged commit f0e4677 into babel:master Dec 18, 2017
@tolmasky tolmasky deleted the fix-jsx-types-as-expressions branch March 7, 2018 21:51
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: types 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.

babel-types lists JSXIdentifier as an Expression
4 participants