Skip to content

jsx-key rule should error on returning short hand fragments (<>) from iterators to avoid false negative #3657

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

Closed
GNRSN opened this issue Nov 22, 2023 · 5 comments
Labels

Comments

@GNRSN
Copy link

GNRSN commented Nov 22, 2023

Issue

Mapping a list into multiple components per list element, surrounding them with jsx shorthand fragments <> currently results in no eslint error with jsx-key rule enabled, even though React will warn/error about missing keys.

According to the documentation, the recommendation is to instead always return full fragments from iterators, i.e.

<Fragment key={key}>
   ...
</Fragment>

See the short section in: https://react.dev/reference/react/Fragment#rendering-a-list-of-fragments

Suggestion

With the jsx-key rule enabled, shorthand fragments returned from an iterator should still error without key, fix could update to <React.Fragment key={key}>

Motivation

Since missing keys can be pretty hard to debug, especially on fragments which won't show up in devtools, having eslint help with this would be extremely valuable instead of having to debug once it hits staging.

Misc

#2365 seems to be the most recent issue on this topic, discussing the removal of key requirement for shorthand fragments as they don't support it

@GNRSN GNRSN changed the title jsx-key rule should enforce explicit <Fragment> replacing shorthand fragments (<>) so that a key can be added when required jsx-key rule should error on returning short hand fragments (<>) from iterators to avoid false negative Nov 22, 2023
@ljharb
Copy link
Member

ljharb commented Nov 22, 2023

These are arrays, not iterators, to be clear.

#2365 is explicitly about fixing the error message when fragment shorthand is used - meaning, it does error.

It can’t be auto fixed because we can’t pick the key for the developer.

Can you provide a repro repo where it doesn’t error?

@GNRSN
Copy link
Author

GNRSN commented Nov 22, 2023

Rel1cx added a commit to Rel1cx/eslint-react that referenced this issue Nov 23, 2023
@ljharb
Copy link
Member

ljharb commented Nov 25, 2023

@GNRSN are you perhaps unaware that there's a checkFragmentShorthand option that's not enabled by default? When I enable it, it works fine.

You shouldn't use "recommended" configs, including ours - you should use the airbnb config.

@GNRSN
Copy link
Author

GNRSN commented Dec 1, 2023

@ljharb Thank you for pointing out my ignorance, as usual, the answer was in the documentation.

I'm phasing in stricer lint rules in a large legacy code-base so starting out with the Airbnb config (which is from my limited experience with it, extremely strict) isn't really an option.

@ljharb
Copy link
Member

ljharb commented Dec 1, 2023

I’d suggest starting with the Airbnb config and using overrides initially to allow existing files to violate whatever, and then gradually remove the overrides. That way, new files are in compliance.

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

No branches or pull requests

2 participants