Skip to content

Commit e967d43

Browse files
Rel1cxRel1cx
authored
feat: add react-x/no-unnecessary-key, closes #950 (#1196)
1 parent 9dab0d1 commit e967d43

File tree

10 files changed

+270
-21
lines changed

10 files changed

+270
-21
lines changed

‎apps/website/content/docs/rules/meta.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
"no-set-state-in-component-did-update",
4646
"no-set-state-in-component-will-update",
4747
"no-string-refs",
48+
"no-unnecessary-key",
4849
"no-unnecessary-use-callback",
4950
"no-unnecessary-use-memo",
5051
"no-unnecessary-use-prefix",

‎apps/website/content/docs/rules/overview.mdx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ full: true
7070
| [`no-set-state-in-component-did-update`](./no-set-state-in-component-did-update) | 1️⃣ | | Disallow calling `this.setState` in `componentDidUpdate` outside of functions, such as callbacks | |
7171
| [`no-set-state-in-component-will-update`](./no-set-state-in-component-will-update) | 1️⃣ | | Disallow calling `this.setState` in `componentWillUpdate` outside of functions, such as callbacks | |
7272
| [`no-string-refs`](./no-string-refs) | 2️⃣ | `🔄` | Replaces string refs with callback refs | >=16.3.0 |
73+
| [`no-unnecessary-key`](./no-unnecessary-key) | 0️⃣ | `🧪` | Prevents the use of unnecessary `key` props on JSX elements when rendering lists | |
7374
| [`no-unnecessary-use-callback`](./no-unnecessary-use-callback) | 0️⃣ | `🧪` | Disallow unnecessary usage of `useCallback` | |
7475
| [`no-unnecessary-use-memo`](./no-unnecessary-use-memo) | 0️⃣ | `🧪` | Disallow unnecessary usage of `useMemo` | |
7576
| [`no-unnecessary-use-prefix`](./no-unnecessary-use-prefix) | 0️⃣ | | Enforces that a function with the `use` prefix should use at least one Hook inside of it | |

‎packages/plugins/eslint-plugin-react-x/src/configs/recommended-type-checked.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ export const rules = {
88
...recommendedTypeScript.rules,
99
"react-x/no-leaked-conditional-rendering": "warn",
1010
"react-x/no-unused-props": "warn",
11-
// "react-x/prefer-read-only-props": "warn",
1211
} as const satisfies RulePreset;
1312

1413
export const settings = {

‎packages/plugins/eslint-plugin-react-x/src/configs/recommended.ts‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,15 @@ export const name = "react-x/recommended";
66
export const rules = {
77
"react-x/jsx-no-comment-textnodes": "warn",
88
"react-x/jsx-no-duplicate-props": "warn",
9-
// "react-x/jsx-no-iife": "warn",
10-
// "react-x/jsx-no-undef": "error",
11-
// "react-x/jsx-shorthand-boolean": "warn",
12-
// "react-x/jsx-shorthand-fragment": "warn",
139
"react-x/jsx-uses-react": "warn",
1410
"react-x/jsx-uses-vars": "warn",
15-
1611
"react-x/no-access-state-in-setstate": "error",
1712
"react-x/no-array-index-key": "warn",
1813
"react-x/no-children-count": "warn",
1914
"react-x/no-children-for-each": "warn",
2015
"react-x/no-children-map": "warn",
2116
"react-x/no-children-only": "warn",
22-
// "react-x/no-children-prop": "warn",
2317
"react-x/no-children-to-array": "warn",
24-
// "react-x/no-class-component": "warn",
2518
"react-x/no-clone-element": "warn",
2619
"react-x/no-comment-textnodes": "warn",
2720
"react-x/no-component-will-mount": "error",
@@ -34,11 +27,7 @@ export const rules = {
3427
"react-x/no-duplicate-key": "error",
3528
"react-x/no-forward-ref": "warn",
3629
"react-x/no-implicit-key": "warn",
37-
// "react-x/no-leaked-conditional-rendering": "warn",
38-
// "react-x/no-missing-component-display-name": "warn",
39-
// "react-x/no-missing-context-display-name": "warn",
4030
"react-x/no-missing-key": "error",
41-
// "react-x/no-misused-capture-owner-stack": "error",
4231
"react-x/no-nested-component-definitions": "error",
4332
"react-x/no-nested-lazy-component-declarations": "error",
4433
"react-x/no-prop-types": "error",
@@ -47,23 +36,16 @@ export const rules = {
4736
"react-x/no-set-state-in-component-did-update": "warn",
4837
"react-x/no-set-state-in-component-will-update": "warn",
4938
"react-x/no-string-refs": "error",
50-
// "react-x/no-unnecessary-use-callback": "warn",
51-
// "react-x/no-unnecessary-use-memo": "warn",
5239
"react-x/no-unnecessary-use-prefix": "warn",
5340
"react-x/no-unsafe-component-will-mount": "warn",
5441
"react-x/no-unsafe-component-will-receive-props": "warn",
5542
"react-x/no-unsafe-component-will-update": "warn",
5643
"react-x/no-unstable-context-value": "warn",
5744
"react-x/no-unstable-default-props": "warn",
5845
"react-x/no-unused-class-component-members": "warn",
59-
// "react-x/no-unused-props": "warn",
6046
"react-x/no-unused-state": "warn",
6147
"react-x/no-use-context": "warn",
6248
"react-x/no-useless-forward-ref": "warn",
63-
// "react-x/no-useless-fragment": "warn",
64-
// "react-x/prefer-destructuring-assignment": "warn",
65-
// "react-x/prefer-namespace-import": "warn",
66-
// "react-x/prefer-read-only-props": "error",
6749
"react-x/prefer-use-state-lazy-initialization": "warn",
6850
} as const satisfies RulePreset;
6951

‎packages/plugins/eslint-plugin-react-x/src/plugin.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import noSetStateInComponentDidMount from "./rules/no-set-state-in-component-did
4343
import noSetStateInComponentDidUpdate from "./rules/no-set-state-in-component-did-update";
4444
import noSetStateInComponentWillUpdate from "./rules/no-set-state-in-component-will-update";
4545
import noStringRefs from "./rules/no-string-refs";
46+
import noUnnecessaryKey from "./rules/no-unnecessary-key";
4647
import noUnnecessaryUseCallback from "./rules/no-unnecessary-use-callback";
4748
import noUnnecessaryUseMemo from "./rules/no-unnecessary-use-memo";
4849
import noUnnecessaryUsePrefix from "./rules/no-unnecessary-use-prefix";
@@ -62,6 +63,7 @@ import preferNamespaceImport from "./rules/prefer-namespace-import";
6263
import preferReadOnlyProps from "./rules/prefer-read-only-props";
6364
import preferUseStateLazyInitialization from "./rules/prefer-use-state-lazy-initialization";
6465

66+
// Removed rules
6567
import avoidShorthandBoolean from "./rules-removed/avoid-shorthand-boolean";
6668
import avoidShorthandFragment from "./rules-removed/avoid-shorthand-fragment";
6769
import noCommentTextnodes from "./rules-removed/no-comment-textnodes";
@@ -119,6 +121,7 @@ export const plugin = {
119121
"no-set-state-in-component-did-update": noSetStateInComponentDidUpdate,
120122
"no-set-state-in-component-will-update": noSetStateInComponentWillUpdate,
121123
"no-string-refs": noStringRefs,
124+
"no-unnecessary-key": noUnnecessaryKey,
122125
"no-unnecessary-use-callback": noUnnecessaryUseCallback,
123126
"no-unnecessary-use-memo": noUnnecessaryUseMemo,
124127
"no-unnecessary-use-prefix": noUnnecessaryUsePrefix,

‎packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.mdx‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
---
2+
title: no-unnecessary-key
3+
---
4+
5+
**Full Name in `eslint-plugin-react-x`**
6+
7+
```sh copy
8+
react-x/no-unnecessary-key
9+
```
10+
11+
**Full Name in `@eslint-react/eslint-plugin`**
12+
13+
```sh copy
14+
@eslint-react/no-unnecessary-key
15+
```
16+
17+
**Features**
18+
19+
`🧪`
20+
21+
## Description
22+
23+
This rule prevents the use of unnecessary `key` props on JSX elements when rendering lists.
24+
25+
When rendering a list of elements in React, the `key` prop should only be placed on the outermost element for each item in the list. Adding keys to nested child elements is redundant, can cause confusion, and may lead to subtle bugs during refactoring.
26+
27+
For example, if an element with a `key` is wrapped with a `React.Fragment` or another component, the `key` must be moved to the new wrapping element. Forgetting to remove the original `key` from the child element can lead to runtime warnings from React if it's duplicated, or simply leave unnecessary code. This rule helps identify and remove these redundant `key` props.
28+
29+
## Examples
30+
31+
### Failing
32+
33+
```jsx
34+
// A key on a child element inside a map is unnecessary
35+
things.map(thing => (
36+
<div key={thing.id}>
37+
<p key="child-key">Hello</p> {/* 🚨 This key is unnecessary */}
38+
</div>
39+
))
40+
41+
// The key should be on the Fragment, not the div
42+
things.map(thing => (
43+
<React.Fragment key={thing.id}>
44+
<div key={thing.id}> {/* 🚨 This key is redundant */}
45+
{thing.name}
46+
</div>
47+
<div>{thing.description}</div>
48+
</React.Fragment>
49+
))
50+
51+
// This also applies to array literals
52+
const elements = [
53+
<div key="1">
54+
<span key="child-span">Item 1</span> {/* 🚨 This key is unnecessary */}
55+
</div>
56+
];
57+
```
58+
59+
### Passing
60+
61+
```jsx
62+
// Key is correctly placed on the top-level element of the map
63+
things.map(thing => <div key={thing.id}>{thing.name}</div>)
64+
65+
// When using Fragments, the key is correctly placed on the Fragment
66+
things.map(thing => (
67+
<React.Fragment key={thing.id}>
68+
<div>{thing.name}</div>
69+
<div>{thing.description}</div>
70+
</React.Fragment>
71+
))
72+
73+
// Keys on top-level elements in an array literal are correct
74+
const elements = [<div key="1" />, <div key="2" />];
75+
76+
// Static keys used to re-mount components are not affected by this rule,
77+
// as they are not inside a list rendering context.
78+
function ComponentWithStaticKey() {
79+
return (
80+
<div>
81+
<MyComponent key={someValue} />
82+
</div>
83+
);
84+
}
85+
```
86+
87+
## Implementation
88+
89+
- [Rule Source](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.ts)
90+
- [Test Source](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.spec.ts)
91+
92+
## Further Reading
93+
94+
- [React Docs: Why does React need keys?](https://react.dev/learn/rendering-lists#why-does-react-need-keys)
95+
96+
---
97+
98+
## See Also
99+
100+
- [`no-missing-key`](./no-missing-key)\
101+
Prevents missing `key` on items in list rendering.
102+
- [`no-implicit-key`](./no-implicit-key)\
103+
Prevents `key` from not being explicitly specified (e.g. spreading `key` from objects).
104+
- [`no-array-index-key`](./no-array-index-key)\
105+
Warns when an array `index` is used as a `key` prop.

‎packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.spec.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import tsx from "dedent";
2+
3+
import { allValid, ruleTester } from "../../../../../test";
4+
import rule, { RULE_NAME } from "./no-unnecessary-key";
5+
6+
ruleTester.run(RULE_NAME, rule, {
7+
invalid: [
8+
// Invalid: unnecessary key on a child element in a map
9+
{
10+
code: tsx`
11+
things.map(thing => <div key={thing.id}><p key='child-1' /></div>)
12+
`,
13+
errors: [{ messageId: "noUnnecessaryKey" }],
14+
},
15+
// Invalid: redundant key on a direct child when the parent Fragment already has the key
16+
{
17+
code: tsx`
18+
things.map(thing => <React.Fragment key={thing.id}><div key={thing.id}>{thing.name}</div></React.Fragment>)
19+
`,
20+
errors: [{ messageId: "noUnnecessaryKey" }],
21+
},
22+
// Invalid: multiple unnecessary keys on child elements
23+
{
24+
code: tsx`
25+
things.map(thing => <div key={thing.id}><p key='child-1' /><span key='child-2' /></div>)
26+
`,
27+
errors: [{ messageId: "noUnnecessaryKey" }, { messageId: "noUnnecessaryKey" }],
28+
},
29+
// TODO: Add support for array literal
30+
// Invalid: unnecessary key on a child element in an array literal
31+
// {
32+
// code: tsx`
33+
// const elements = [<div key='1'><p key='child' /></div>]
34+
// `,
35+
// errors: [{ messageId: "noUnnecessaryKey" }],
36+
// },
37+
// Invalid: unnecessary key within an element returned from a function expression
38+
{
39+
code: tsx`
40+
things.map(function(thing) { return <div key={thing.id}><i key='icon' /></div>; })
41+
`,
42+
errors: [{ messageId: "noUnnecessaryKey" }],
43+
},
44+
// Invalid: unnecessary key in a nested map call
45+
{
46+
code: tsx`
47+
outers.map(outer => <div key={outer.id}>{inners.map(inner => <p key={inner.id}><span key='extra' /></p>)}</div>)
48+
`,
49+
errors: [{ messageId: "noUnnecessaryKey" }],
50+
},
51+
],
52+
valid: [
53+
...allValid,
54+
// Valid: key on the top-level element in a map
55+
tsx`
56+
things.map(thing => <div key={thing.id} />)
57+
`,
58+
// Valid: key on the top-level React.Fragment in a map
59+
tsx`
60+
things.map(thing => <React.Fragment key={thing.id}><div /></React.Fragment>)
61+
`,
62+
// Valid: key on the top-level element returned from a function expression in a map
63+
tsx`
64+
things.map(function(thing) { return <p key={thing.id} />; })
65+
`,
66+
// Valid: key on top-level elements in an array literal
67+
tsx`
68+
const elements = [<div key='1' />, <div key='2' />]
69+
`,
70+
// Valid: child elements without a key prop
71+
tsx`
72+
things.map(thing => <div key={thing.id}><p>Hello</p></div>)
73+
`,
74+
// Valid: key prop in a non-map/array context (this rule does not handle this case)
75+
tsx`
76+
<div key='static-key' />
77+
`,
78+
// Valid: key prop on a child in a non-map/array context
79+
tsx`
80+
<div><p key='static-child-key' /></div>
81+
`,
82+
],
83+
});

‎packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.ts‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import * as AST from "@eslint-react/ast";
2+
import { type RuleContext, type RuleFeature } from "@eslint-react/kit";
3+
import type { TSESTree } from "@typescript-eslint/types";
4+
import type { RuleListener } from "@typescript-eslint/utils/ts-eslint";
5+
import type { CamelCase } from "string-ts";
6+
7+
import { AST_NODE_TYPES as T } from "@typescript-eslint/types";
8+
9+
import { createRule } from "../utils";
10+
11+
export const RULE_NAME = "no-unnecessary-key";
12+
13+
export const RULE_FEATURES = [
14+
"EXP",
15+
] as const satisfies RuleFeature[];
16+
17+
export type MessageID = CamelCase<typeof RULE_NAME>;
18+
19+
export default createRule<[], MessageID>({
20+
meta: {
21+
type: "problem",
22+
docs: {
23+
description: "Prevents the use of unnecessary `key` props on JSX elements when rendering lists.",
24+
[Symbol.for("rule_features")]: RULE_FEATURES,
25+
},
26+
messages: {
27+
noUnnecessaryKey:
28+
"Unnecessary `key` prop on this element. The `key` should be on the top-level element returned from the array.",
29+
},
30+
schema: [],
31+
},
32+
name: RULE_NAME,
33+
create,
34+
defaultOptions: [],
35+
});
36+
37+
export function create(context: RuleContext<MessageID, []>): RuleListener {
38+
if (!context.sourceCode.getText().includes("key=")) {
39+
return {};
40+
}
41+
42+
return {
43+
JSXAttribute(node: TSESTree.JSXAttribute) {
44+
if (node.name.name !== "key") return;
45+
const jsxElement = node.parent.parent;
46+
const pMapCallback = AST.findParentNode(jsxElement, isMapCallback);
47+
if (pMapCallback == null) return;
48+
const pKeyedElementOrElse = AST.findParentNode(
49+
jsxElement,
50+
(n) => {
51+
if (n === pMapCallback) return true;
52+
return AST.isJSXElement(n)
53+
&& n
54+
.openingElement
55+
.attributes
56+
.some((n) =>
57+
n.type === T.JSXAttribute
58+
&& n.name.type === T.JSXIdentifier
59+
&& n.name.name === "key"
60+
);
61+
},
62+
);
63+
// No parent JSX element with a key prop found between the map callback and this element
64+
if (pKeyedElementOrElse == null || pKeyedElementOrElse === pMapCallback) return;
65+
context.report({ messageId: "noUnnecessaryKey", node });
66+
},
67+
};
68+
}
69+
70+
function isMapCallback(node: TSESTree.Node) {
71+
if (node.parent == null) return false;
72+
if (!AST.isArrayMapCall(node.parent)) return false;
73+
return AST.isOneOf([T.ArrowFunctionExpression, T.FunctionExpression])(AST.getJSExpression(node));
74+
}

‎packages/plugins/eslint-plugin/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,8 @@ export default tseslint.config({
165165

166166
Contributions are welcome!
167167

168-
Please follow our [contributing guidelines](https://github.com/Rel1cx/eslint-react/tree/2.0.0/.github/CONTRIBUTING.md).
168+
Please follow our [contributing guidelines](https://github.com/Rel1cx/eslint-react/tree/no-unnecessary-key/.github/CONTRIBUTING.md).
169169

170170
## License
171171

172-
This project is licensed under the MIT License - see the [LICENSE](https://github.com/Rel1cx/eslint-react/tree/2.0.0/LICENSE) file for details.
172+
This project is licensed under the MIT License - see the [LICENSE](https://github.com/Rel1cx/eslint-react/tree/no-unnecessary-key/LICENSE) file for details.

‎packages/plugins/eslint-plugin/src/configs/all.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export const rules = {
5454
"@eslint-react/no-set-state-in-component-did-update": "warn",
5555
"@eslint-react/no-set-state-in-component-will-update": "warn",
5656
"@eslint-react/no-string-refs": "error",
57+
"@eslint-react/no-unnecessary-key": "warn",
5758
"@eslint-react/no-unnecessary-use-callback": "warn",
5859
"@eslint-react/no-unnecessary-use-memo": "warn",
5960
"@eslint-react/no-unnecessary-use-prefix": "warn",

0 commit comments

Comments
 (0)