[feat] `react-x/no-unnecessary-key`

[kachkaev]

Describe the problem

React keys are necessary for dynamic lists and there are a few good rules to lint them. Keys are also allowed in the ‘static’ React markup and are used to re-mount a component when needed. The latter case is relatively rare and there is a specific reason why a component needs to fully reset. This reason is usually added to code in a form of a comment.

In my experience, React keys in static markup are often added by mistake. For example, imagine we have:

things.map((thing) => <div key={thing.id}>{thing.name}</div>)

All good so far. Now imagine we need to add a sibling element to each list element. We may end up with this:

things.map((thing) => <><div key={thing.id}>{thing.name}</div><div>{thing.description}</div></>)

ESLint will rightfully complain about a missing key and we'll do:

things.map((thing) => <React.Fragment key={thing.id}><div key={thing.id}>{thing.name}</div><div>{thing.description}</div></React.Fragment>)

We fixed the problem, but we forgot to remove key={thing.id} from the inner div. It is quite vivid in the above example but imagine we have a component with quite a few props in which key={} gets lost visually. Thus, the now redundant key will probably stay in JSX unless it is spotted by a human.

Here is what may also happen: someone copy-pastes <div key={thing.id}>{thing.name}</div> to show description and we end up with:

things.map((thing) => <React.Fragment key={thing.id}><div key={thing.id}>{thing.name}</div><div key={thing.id}>{thing.description}</div></React.Fragment>)

So we get two ‘static‘ JSX elements with the same key without a good reason. This triggers no ESLint warnings but causes a React warning at runtime. I saw this situation inside a context menu when my colleague has replaced a single menu item per thing with a few.

Describe the solution you'd like

I would like to have a rule that flags any usage of key={} in ‘static’ JSX markup. If we need to reset a component via key, I would like to add eslint-disable-next-line with an explanation of the edge case. This would be similar to @eslint-react/dom/no-dangerously-set-innerhtml: we don’t want dangerouslySetInnerHTML most of the time, but if we opt-in, we add eslint-disable-next-line and explain why we make an exception.

Alternatives considered

This rule can be possibly created via no-restricted-syntax. However, it’d be a bit laborious to setup an AST selector for it.

Additional context

This rule was discussed in jsx-eslint/eslint-plugin-react#3148 a while ago but got rejected.

[Rel1cx]

In 2.0, I may consider adding back support for React.createElement() to the related rules, including this one, which means that these rules will not only detect JSX markup, but React.createElement() call expressions as well, so I would prefer no-unnecessary-key (called no-useless-key in the prior issue, which is not completely useless).