feat: add react-x/no-unnecessary-key, closes #950

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

@@ -45,6 +45,7 @@
     "no-set-state-in-component-did-update",
     "no-set-state-in-component-will-update",
     "no-string-refs",
+    "no-unnecessary-key",
     "no-unnecessary-use-callback",
     "no-unnecessary-use-memo",
     "no-unnecessary-use-prefix",

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

@@ -70,6 +70,7 @@ full: true
 | [`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    |          |
 | [`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   |          |
 | [`no-string-refs`](./no-string-refs)                                                 | 2️⃣ |    `🔄`   | Replaces string refs with callback refs                                                             | >=16.3.0 |
+| [`no-unnecessary-key`](./no-unnecessary-key)                                         | 0️⃣ |    `🧪`   | Prevents the use of unnecessary `key` props on JSX elements when rendering lists                    |          |
 | [`no-unnecessary-use-callback`](./no-unnecessary-use-callback)                       | 0️⃣ |    `🧪`   | Disallow unnecessary usage of `useCallback`                                                         |          |
 | [`no-unnecessary-use-memo`](./no-unnecessary-use-memo)                               | 0️⃣ |    `🧪`   | Disallow unnecessary usage of `useMemo`                                                             |          |
 | [`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

@@ -8,7 +8,6 @@ export const rules = {
   ...recommendedTypeScript.rules,
   "react-x/no-leaked-conditional-rendering": "warn",
   "react-x/no-unused-props": "warn",
-  // "react-x/prefer-read-only-props": "warn",
 } as const satisfies RulePreset;
 
 export const settings = {

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

@@ -6,22 +6,15 @@ export const name = "react-x/recommended";
 export const rules = {
   "react-x/jsx-no-comment-textnodes": "warn",
   "react-x/jsx-no-duplicate-props": "warn",
-  // "react-x/jsx-no-iife": "warn",
-  // "react-x/jsx-no-undef": "error",
-  // "react-x/jsx-shorthand-boolean": "warn",
-  // "react-x/jsx-shorthand-fragment": "warn",
   "react-x/jsx-uses-react": "warn",
   "react-x/jsx-uses-vars": "warn",
-
   "react-x/no-access-state-in-setstate": "error",
   "react-x/no-array-index-key": "warn",
   "react-x/no-children-count": "warn",
   "react-x/no-children-for-each": "warn",
   "react-x/no-children-map": "warn",
   "react-x/no-children-only": "warn",
-  // "react-x/no-children-prop": "warn",
   "react-x/no-children-to-array": "warn",
-  // "react-x/no-class-component": "warn",
   "react-x/no-clone-element": "warn",
   "react-x/no-comment-textnodes": "warn",
   "react-x/no-component-will-mount": "error",
@@ -34,11 +27,7 @@ export const rules = {
   "react-x/no-duplicate-key": "error",
   "react-x/no-forward-ref": "warn",
   "react-x/no-implicit-key": "warn",
-  // "react-x/no-leaked-conditional-rendering": "warn",
-  // "react-x/no-missing-component-display-name": "warn",
-  // "react-x/no-missing-context-display-name": "warn",
   "react-x/no-missing-key": "error",
-  // "react-x/no-misused-capture-owner-stack": "error",
   "react-x/no-nested-component-definitions": "error",
   "react-x/no-nested-lazy-component-declarations": "error",
   "react-x/no-prop-types": "error",
@@ -47,23 +36,16 @@ export const rules = {
   "react-x/no-set-state-in-component-did-update": "warn",
   "react-x/no-set-state-in-component-will-update": "warn",
   "react-x/no-string-refs": "error",
-  // "react-x/no-unnecessary-use-callback": "warn",
-  // "react-x/no-unnecessary-use-memo": "warn",
   "react-x/no-unnecessary-use-prefix": "warn",
   "react-x/no-unsafe-component-will-mount": "warn",
   "react-x/no-unsafe-component-will-receive-props": "warn",
   "react-x/no-unsafe-component-will-update": "warn",
   "react-x/no-unstable-context-value": "warn",
   "react-x/no-unstable-default-props": "warn",
   "react-x/no-unused-class-component-members": "warn",
-  // "react-x/no-unused-props": "warn",
   "react-x/no-unused-state": "warn",
   "react-x/no-use-context": "warn",
   "react-x/no-useless-forward-ref": "warn",
-  // "react-x/no-useless-fragment": "warn",
-  // "react-x/prefer-destructuring-assignment": "warn",
-  // "react-x/prefer-namespace-import": "warn",
-  // "react-x/prefer-read-only-props": "error",
   "react-x/prefer-use-state-lazy-initialization": "warn",
 } as const satisfies RulePreset;
 

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

@@ -43,6 +43,7 @@ import noSetStateInComponentDidMount from "./rules/no-set-state-in-component-did
 import noSetStateInComponentDidUpdate from "./rules/no-set-state-in-component-did-update";
 import noSetStateInComponentWillUpdate from "./rules/no-set-state-in-component-will-update";
 import noStringRefs from "./rules/no-string-refs";
+import noUnnecessaryKey from "./rules/no-unnecessary-key";
 import noUnnecessaryUseCallback from "./rules/no-unnecessary-use-callback";
 import noUnnecessaryUseMemo from "./rules/no-unnecessary-use-memo";
 import noUnnecessaryUsePrefix from "./rules/no-unnecessary-use-prefix";
@@ -62,6 +63,7 @@ import preferNamespaceImport from "./rules/prefer-namespace-import";
 import preferReadOnlyProps from "./rules/prefer-read-only-props";
 import preferUseStateLazyInitialization from "./rules/prefer-use-state-lazy-initialization";
 
+// Removed rules
 import avoidShorthandBoolean from "./rules-removed/avoid-shorthand-boolean";
 import avoidShorthandFragment from "./rules-removed/avoid-shorthand-fragment";
 import noCommentTextnodes from "./rules-removed/no-comment-textnodes";
@@ -119,6 +121,7 @@ export const plugin = {
     "no-set-state-in-component-did-update": noSetStateInComponentDidUpdate,
     "no-set-state-in-component-will-update": noSetStateInComponentWillUpdate,
     "no-string-refs": noStringRefs,
+    "no-unnecessary-key": noUnnecessaryKey,
     "no-unnecessary-use-callback": noUnnecessaryUseCallback,
     "no-unnecessary-use-memo": noUnnecessaryUseMemo,
     "no-unnecessary-use-prefix": noUnnecessaryUsePrefix,

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

@@ -0,0 +1,105 @@
+---
+title: no-unnecessary-key
+---
+
+**Full Name in `eslint-plugin-react-x`**
+
+```sh copy
+react-x/no-unnecessary-key
+```
+
+**Full Name in `@eslint-react/eslint-plugin`**
+
+```sh copy
+@eslint-react/no-unnecessary-key
+```
+
+**Features**
+
+`🧪`
+
+## Description
+
+This rule prevents the use of unnecessary `key` props on JSX elements when rendering lists.
+
+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.
+
+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.
+
+## Examples
+
+### Failing
+
+```jsx
+// A key on a child element inside a map is unnecessary
+things.map(thing => (
+  <div key={thing.id}>
+    <p key="child-key">Hello</p> {/* 🚨 This key is unnecessary */}
+  </div>
+))
+
+// The key should be on the Fragment, not the div
+things.map(thing => (
+  <React.Fragment key={thing.id}>
+    <div key={thing.id}> {/* 🚨 This key is redundant */}
+      {thing.name}
+    </div>
+    <div>{thing.description}</div>
+  </React.Fragment>
+))
+
+// This also applies to array literals
+const elements = [
+  <div key="1">
+    <span key="child-span">Item 1</span> {/* 🚨 This key is unnecessary */}
+  </div>
+];
+```
+
+### Passing
+
+```jsx
+// Key is correctly placed on the top-level element of the map
+things.map(thing => <div key={thing.id}>{thing.name}</div>)
+
+// When using Fragments, the key is correctly placed on the Fragment
+things.map(thing => (
+  <React.Fragment key={thing.id}>
+    <div>{thing.name}</div>
+    <div>{thing.description}</div>
+  </React.Fragment>
+))
+
+// Keys on top-level elements in an array literal are correct
+const elements = [<div key="1" />, <div key="2" />];
+
+// Static keys used to re-mount components are not affected by this rule,
+// as they are not inside a list rendering context.
+function ComponentWithStaticKey() {
+  return (
+    <div>
+      <MyComponent key={someValue} />
+    </div>
+  );
+}
+```
+
+## Implementation
+
+- [Rule Source](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.ts)
+- [Test Source](https://github.com/Rel1cx/eslint-react/tree/main/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.spec.ts)
+
+## Further Reading
+
+- [React Docs: Why does React need keys?](https://react.dev/learn/rendering-lists#why-does-react-need-keys)
+
+---
+
+## See Also
+
+- [`no-missing-key`](./no-missing-key)\
+  Prevents missing `key` on items in list rendering.
+- [`no-implicit-key`](./no-implicit-key)\
+  Prevents `key` from not being explicitly specified (e.g. spreading `key` from objects).
+- [`no-array-index-key`](./no-array-index-key)\
+  Warns when an array `index` is used as a `key` prop.

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

@@ -0,0 +1,83 @@
+import tsx from "dedent";
+
+import { allValid, ruleTester } from "../../../../../test";
+import rule, { RULE_NAME } from "./no-unnecessary-key";
+
+ruleTester.run(RULE_NAME, rule, {
+  invalid: [
+    // Invalid: unnecessary key on a child element in a map
+    {
+      code: tsx`
+        things.map(thing => <div key={thing.id}><p key='child-1' /></div>)
+      `,
+      errors: [{ messageId: "noUnnecessaryKey" }],
+    },
+    // Invalid: redundant key on a direct child when the parent Fragment already has the key
+    {
+      code: tsx`
+        things.map(thing => <React.Fragment key={thing.id}><div key={thing.id}>{thing.name}</div></React.Fragment>)
+      `,
+      errors: [{ messageId: "noUnnecessaryKey" }],
+    },
+    // Invalid: multiple unnecessary keys on child elements
+    {
+      code: tsx`
+        things.map(thing => <div key={thing.id}><p key='child-1' /><span key='child-2' /></div>)
+      `,
+      errors: [{ messageId: "noUnnecessaryKey" }, { messageId: "noUnnecessaryKey" }],
+    },
+    // TODO: Add support for array literal
+    // Invalid: unnecessary key on a child element in an array literal
+    // {
+    //   code: tsx`
+    //     const elements = [<div key='1'><p key='child' /></div>]
+    //   `,
+    //   errors: [{ messageId: "noUnnecessaryKey" }],
+    // },
+    // Invalid: unnecessary key within an element returned from a function expression
+    {
+      code: tsx`
+        things.map(function(thing) { return <div key={thing.id}><i key='icon' /></div>; })
+      `,
+      errors: [{ messageId: "noUnnecessaryKey" }],
+    },
+    // Invalid: unnecessary key in a nested map call
+    {
+      code: tsx`
+        outers.map(outer => <div key={outer.id}>{inners.map(inner => <p key={inner.id}><span key='extra' /></p>)}</div>)
+      `,
+      errors: [{ messageId: "noUnnecessaryKey" }],
+    },
+  ],
+  valid: [
+    ...allValid,
+    // Valid: key on the top-level element in a map
+    tsx`
+      things.map(thing => <div key={thing.id} />)
+    `,
+    // Valid: key on the top-level React.Fragment in a map
+    tsx`
+      things.map(thing => <React.Fragment key={thing.id}><div /></React.Fragment>)
+    `,
+    // Valid: key on the top-level element returned from a function expression in a map
+    tsx`
+      things.map(function(thing) { return <p key={thing.id} />; })
+    `,
+    // Valid: key on top-level elements in an array literal
+    tsx`
+      const elements = [<div key='1' />, <div key='2' />]
+    `,
+    // Valid: child elements without a key prop
+    tsx`
+      things.map(thing => <div key={thing.id}><p>Hello</p></div>)
+    `,
+    // Valid: key prop in a non-map/array context (this rule does not handle this case)
+    tsx`
+      <div key='static-key' />
+    `,
+    // Valid: key prop on a child in a non-map/array context
+    tsx`
+      <div><p key='static-child-key' /></div>
+    `,
+  ],
+});

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

@@ -0,0 +1,74 @@
+import * as AST from "@eslint-react/ast";
+import { type RuleContext, type RuleFeature } from "@eslint-react/kit";
+import type { TSESTree } from "@typescript-eslint/types";
+import type { RuleListener } from "@typescript-eslint/utils/ts-eslint";
+import type { CamelCase } from "string-ts";
+
+import { AST_NODE_TYPES as T } from "@typescript-eslint/types";
+
+import { createRule } from "../utils";
+
+export const RULE_NAME = "no-unnecessary-key";
+
+export const RULE_FEATURES = [
+  "EXP",
+] as const satisfies RuleFeature[];
+
+export type MessageID = CamelCase<typeof RULE_NAME>;
+
+export default createRule<[], MessageID>({
+  meta: {
+    type: "problem",
+    docs: {
+      description: "Prevents the use of unnecessary `key` props on JSX elements when rendering lists.",
+      [Symbol.for("rule_features")]: RULE_FEATURES,
+    },
+    messages: {
+      noUnnecessaryKey:
+        "Unnecessary `key` prop on this element. The `key` should be on the top-level element returned from the array.",
+    },
+    schema: [],
+  },
+  name: RULE_NAME,
+  create,
+  defaultOptions: [],
+});
+
+export function create(context: RuleContext<MessageID, []>): RuleListener {
+  if (!context.sourceCode.getText().includes("key=")) {
+    return {};
+  }
+
+  return {
+    JSXAttribute(node: TSESTree.JSXAttribute) {
+      if (node.name.name !== "key") return;
+      const jsxElement = node.parent.parent;
+      const pMapCallback = AST.findParentNode(jsxElement, isMapCallback);
+      if (pMapCallback == null) return;
+      const pKeyedElementOrElse = AST.findParentNode(
+        jsxElement,
+        (n) => {
+          if (n === pMapCallback) return true;
+          return AST.isJSXElement(n)
+            && n
+              .openingElement
+              .attributes
+              .some((n) =>
+                n.type === T.JSXAttribute
+                && n.name.type === T.JSXIdentifier
+                && n.name.name === "key"
+              );
+        },
+      );
+      // No parent JSX element with a key prop found between the map callback and this element
+      if (pKeyedElementOrElse == null || pKeyedElementOrElse === pMapCallback) return;
+      context.report({ messageId: "noUnnecessaryKey", node });
+    },
+  };
+}
+
+function isMapCallback(node: TSESTree.Node) {
+  if (node.parent == null) return false;
+  if (!AST.isArrayMapCall(node.parent)) return false;
+  return AST.isOneOf([T.ArrowFunctionExpression, T.FunctionExpression])(AST.getJSExpression(node));
+}

packages/plugins/eslint-plugin/README.md

@@ -165,8 +165,8 @@ export default tseslint.config({
 
 Contributions are welcome!
 
-Please follow our [contributing guidelines](https://github.com/Rel1cx/eslint-react/tree/2.0.0/.github/CONTRIBUTING.md).
+Please follow our [contributing guidelines](https://github.com/Rel1cx/eslint-react/tree/no-unnecessary-key/.github/CONTRIBUTING.md).
 
 ## License
 
-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.
+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

@@ -54,6 +54,7 @@ export const rules = {
   "@eslint-react/no-set-state-in-component-did-update": "warn",
   "@eslint-react/no-set-state-in-component-will-update": "warn",
   "@eslint-react/no-string-refs": "error",
+  "@eslint-react/no-unnecessary-key": "warn",
   "@eslint-react/no-unnecessary-use-callback": "warn",
   "@eslint-react/no-unnecessary-use-memo": "warn",
   "@eslint-react/no-unnecessary-use-prefix": "warn",