Update: Support JSXFragment node

[clemmy]

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ x ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

This is related to #9662.
Currently, when a parser, such as babel-eslint is used, it'll now return JSXFragment. This PR updates some standard rules to add some support for fragments so that JSXFragments can generally be used in the place of JSXElement.

Current behavior example

<>foo</>;

warns that foo should be wrapped in quotes, using the options:

"parser": "babel-eslint",
"rules": { "quotes": [ "error", "single" ] }

Behavior with my changes

<>foo</>;

doesn't give any warnings!

Rationale

I'm just throwing this out here because I'm not really sure if this is a change makes sense to make in this repo at this point in time. This is only applicable when an external parser is specified. If this makes sense, then I can refine the PR. 🙂

[ilyavolodin]

I think Acorn JSX plugin has already been updated to support JSX fragments, so we should have native support for them with the next release of Espree. However, I have no idea what they are supposed to do, so I can't comment on if it makes sense to add this to the rule.

[platinumazure]

@ilyavolodin Are we any closer to being able to support JSXFragment in espree? (Is that something we definitely want to do?)

[ilyavolodin]

@platinumazure As far as I understand it, JSXFragment is officially part of the JSX spec, so we should probably support it. However, it looks like we can't upgrade Acorn to the latest version yet, because there are some breaking changes. I think we might have to implement it in Espree ourselves.

[aladdin-add]

seems this is no longer blocked, since eslint/espree#371 merged.

[ilyavolodin]

Changes look fine to me, but I would like to verify that Acorn-JSX actually outputs new node with name JSXFragment before merging this in.

[ilyavolodin]

Actually, looking at it again, we do need some new unit tests to verify those changes.

[platinumazure]

Hi @clemmy, after a long time, it seems we are ready to look at this again. (We needed to do a breaking change with acorn-jsx and so align this with the ESLint major release-- finally we are doing release candidates.)

At this point, it looks like we need some unit tests (per Ilya's review), and we also need you to fix the commit message. My recommendation is to just push some new commits (with the new unit tests and any other changes you might make), and then edit the PR title to follow our commit message guidelines.

Please let us know if we can help with anything, either by leaving a comment here or by visiting us in our Gitter chat.

Thanks again for contributing. Let's get this in! 😄

[clemmy]

Hi @platinumazure, sorry for the delayed response! I’ve been busy for the past few weeks and probably won’t have any time to dive into this for another couple of weeks. I’m wondering if there is currently any progress being made here or if anyone else wants to continue this work? If not, I’d be happy to pick this issue up again in a few weeks’ time. :)

[clemmy]

Ready for review again.

I updated this PR from last time by adding some unit tests, then rebasing & squashing it into 2 commits -- one for lib changes and one for unit test changes.

[aladdin-add]

LGTM, thanks!

Unit tests have been added and Ilya is on vacation. Will ask for one more review from another TSC member.

[platinumazure]

LGTM, thanks!

[btmills]

LGTM, thanks @clemmy!