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

fix(jsx-key): handle shorthand fragment syntax #2316

Merged
merged 4 commits into from
Jun 23, 2019
Merged

fix(jsx-key): handle shorthand fragment syntax #2316

merged 4 commits into from
Jun 23, 2019

Conversation

kaykayehnn
Copy link
Contributor

In eslint/eslint#9664 eslint added support for the shorthand fragment syntax (<></>) as a new node type: JSXFragment. The jsx-key rule hasn't been updated to handle it correctly so in some cases where it should report errors it doesn't. Here is an example

[1, 2, 3].map(x => <>{x}</>);

This PR fixes this and adds some tests. The error messages could probably be improved and maybe we should add an example to the docs?

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

I can't decide if this is a breaking change or not; if so, it'd need to go behind an option.

@kaykayehnn
Copy link
Contributor Author

Yeah, this would be a breaking change. I added it behind a checkFragmentShorthand option.
It would make sense that by default this option should be enabled. Maybe the option could be removed and be the default in the next major?

@yannickcr yannickcr merged commit c52b61b into jsx-eslint:master Jun 23, 2019
@yannickcr
Copy link
Member

Merged, thank you 🙂

(and yes it will be definitely set as the default in next major)

@kaykayehnn
Copy link
Contributor Author

Great! I was planning to update the docs and tweak the message a bit, I should have left a comment about that. Is it ok if I make a follow up PR for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants