-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[New] Support shorthand fragment syntax #1956
Conversation
* @param {object} node - node to check. | ||
* @returns {boolean} Whether or not the node if a JSX element or fragment. | ||
*/ | ||
function isJSX(node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestions for a better name welcome (isJSXElementOrFragment
seemed too wordy)
@@ -32,6 +32,10 @@ ruleTester.run('jsx-curly-brace-presence', rule, { | |||
{ | |||
code: '<App {...props}>foo</App>' | |||
}, | |||
{ | |||
code: '<>foo</>', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rule doesn't really care whether it's an element or a fragment, so I just added one simple valid test, and one invalid.
I'll wait to merge this til we've updated all the relevant rules. |
Quick q: I noticed some of the rules are using selectors e.g. /~https://github.com/yannickcr/eslint-plugin-react/blob/master/lib/rules/jsx-curly-brace-presence.js#L249 |
Fair point; altho nobody's complained. I guess we should fix that. |
I was asking mostly because this change could use the syntax for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming all the relevant test cases have been updated
I believe I covered what needed to be covered, perhaps even redundantly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me. Test coverage is very good. Nothing seems to break as a result. Nice one!
Resolves #1694.