diff --git a/apps/website/content/docs/rules/meta.json b/apps/website/content/docs/rules/meta.json index c8dae46f2..82297ed17 100644 --- a/apps/website/content/docs/rules/meta.json +++ b/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", diff --git a/apps/website/content/docs/rules/overview.mdx b/apps/website/content/docs/rules/overview.mdx index 58e46132f..3a270bf85 100644 --- a/apps/website/content/docs/rules/overview.mdx +++ b/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 | | diff --git a/packages/plugins/eslint-plugin-react-x/src/configs/recommended-type-checked.ts b/packages/plugins/eslint-plugin-react-x/src/configs/recommended-type-checked.ts index caee1003d..fb2caef33 100644 --- a/packages/plugins/eslint-plugin-react-x/src/configs/recommended-type-checked.ts +++ b/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 = { diff --git a/packages/plugins/eslint-plugin-react-x/src/configs/recommended.ts b/packages/plugins/eslint-plugin-react-x/src/configs/recommended.ts index 7a3ad3db9..2e7912f16 100644 --- a/packages/plugins/eslint-plugin-react-x/src/configs/recommended.ts +++ b/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,8 +36,6 @@ 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", @@ -56,14 +43,9 @@ export const rules = { "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; diff --git a/packages/plugins/eslint-plugin-react-x/src/plugin.ts b/packages/plugins/eslint-plugin-react-x/src/plugin.ts index 885bda8c5..124735805 100644 --- a/packages/plugins/eslint-plugin-react-x/src/plugin.ts +++ b/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, diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.mdx b/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.mdx new file mode 100644 index 000000000..dbf985d6b --- /dev/null +++ b/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 => ( +
+

Hello

{/* 🚨 This key is unnecessary */} +
+)) + +// The key should be on the Fragment, not the div +things.map(thing => ( + +
{/* 🚨 This key is redundant */} + {thing.name} +
+
{thing.description}
+ +)) + +// This also applies to array literals +const elements = [ +
+ Item 1 {/* 🚨 This key is unnecessary */} +
+]; +``` + +### Passing + +```jsx +// Key is correctly placed on the top-level element of the map +things.map(thing =>
{thing.name}
) + +// When using Fragments, the key is correctly placed on the Fragment +things.map(thing => ( + +
{thing.name}
+
{thing.description}
+ +)) + +// Keys on top-level elements in an array literal are correct +const elements = [
,
]; + +// 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 ( +
+ +
+ ); +} +``` + +## 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. diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.spec.ts b/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.spec.ts new file mode 100644 index 000000000..d7bd5e4dd --- /dev/null +++ b/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 =>

) + `, + errors: [{ messageId: "noUnnecessaryKey" }], + }, + // Invalid: redundant key on a direct child when the parent Fragment already has the key + { + code: tsx` + things.map(thing =>
{thing.name}
) + `, + errors: [{ messageId: "noUnnecessaryKey" }], + }, + // Invalid: multiple unnecessary keys on child elements + { + code: tsx` + things.map(thing =>

) + `, + 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 = [

] + // `, + // errors: [{ messageId: "noUnnecessaryKey" }], + // }, + // Invalid: unnecessary key within an element returned from a function expression + { + code: tsx` + things.map(function(thing) { return
; }) + `, + errors: [{ messageId: "noUnnecessaryKey" }], + }, + // Invalid: unnecessary key in a nested map call + { + code: tsx` + outers.map(outer =>
{inners.map(inner =>

)}
) + `, + errors: [{ messageId: "noUnnecessaryKey" }], + }, + ], + valid: [ + ...allValid, + // Valid: key on the top-level element in a map + tsx` + things.map(thing =>
) + `, + // Valid: key on the top-level React.Fragment in a map + tsx` + things.map(thing =>
) + `, + // Valid: key on the top-level element returned from a function expression in a map + tsx` + things.map(function(thing) { return

; }) + `, + // Valid: key on top-level elements in an array literal + tsx` + const elements = [

,
] + `, + // Valid: child elements without a key prop + tsx` + things.map(thing =>

Hello

) + `, + // Valid: key prop in a non-map/array context (this rule does not handle this case) + tsx` +
+ `, + // Valid: key prop on a child in a non-map/array context + tsx` +

+ `, + ], +}); diff --git a/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.ts b/packages/plugins/eslint-plugin-react-x/src/rules/no-unnecessary-key.ts new file mode 100644 index 000000000..66890895a --- /dev/null +++ b/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; + +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): 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)); +} diff --git a/packages/plugins/eslint-plugin/README.md b/packages/plugins/eslint-plugin/README.md index 27065390d..73ca20a49 100644 --- a/packages/plugins/eslint-plugin/README.md +++ b/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. diff --git a/packages/plugins/eslint-plugin/src/configs/all.ts b/packages/plugins/eslint-plugin/src/configs/all.ts index 916708920..f5acba1fb 100644 --- a/packages/plugins/eslint-plugin/src/configs/all.ts +++ b/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",