From dbc8e8c35a39e9351ce73d3132e00af2d0113b80 Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Fri, 18 Feb 2022 23:53:06 -0800 Subject: [PATCH] [New] `jsx-key`: add `warnDuplicates` option to warn on duplicate jsx keys in an array Fixes #2614 --- CHANGELOG.md | 2 ++ docs/rules/jsx-key.md | 13 ++++++++++ lib/rules/jsx-key.js | 52 ++++++++++++++++++++++++++++++-------- tests/lib/rules/jsx-key.js | 21 +++++++++++++++ 4 files changed, 78 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 359794171b..3188c031a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange * add [`iframe-missing-sandbox`] rule ([#2753][] @tosmolka @ljharb) * [`no-did-mount-set-state`], [`no-did-update-set-state`]: no-op with react >= 16.3 ([#1754][] @ljharb) * [`jsx-sort-props`]: support multiline prop groups ([#3198][] @duhamelgm) +* [`jsx-key`]: add `warnDuplicates` option to warn on duplicate jsx keys in an array ([#2614][] @ljharb) ### Fixed * [`prop-types`], `propTypes`: add support for exported type inference ([#3163][] @vedadeepta) @@ -57,6 +58,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange [#2921]: https://github.com/yannickcr/eslint-plugin-react/pull/2921 [#2813]: https://github.com/yannickcr/eslint-plugin-react/pull/2813 [#2753]: https://github.com/yannickcr/eslint-plugin-react/pull/2753 +[#2614]: https://github.com/yannickcr/eslint-plugin-react/issues/2614 [#2596]: https://github.com/yannickcr/eslint-plugin-react/issues/2596 [#2061]: https://github.com/yannickcr/eslint-plugin-react/issues/2061 [#1817]: https://github.com/yannickcr/eslint-plugin-react/issues/1817 diff --git a/docs/rules/jsx-key.md b/docs/rules/jsx-key.md index 6baed4c42c..70c4160fb3 100644 --- a/docs/rules/jsx-key.md +++ b/docs/rules/jsx-key.md @@ -57,6 +57,19 @@ Examples of **incorrect** code for this rule: ; ``` +### `warnOnDuplicates` (default: `false`) + +When `true` the rule will check for any duplicate key prop values. + +Examples of **incorrect** code for this rule: + +```jsx +const spans = [ + , + , +]; +``` + ## When Not To Use It If you are not using JSX then you can disable this rule. diff --git a/lib/rules/jsx-key.js b/lib/rules/jsx-key.js index 99b8ec19cd..207b10da58 100644 --- a/lib/rules/jsx-key.js +++ b/lib/rules/jsx-key.js @@ -7,6 +7,7 @@ const hasProp = require('jsx-ast-utils/hasProp'); const propName = require('jsx-ast-utils/propName'); +const values = require('object.values'); const docsUrl = require('../util/docsUrl'); const pragmaUtil = require('../util/pragma'); const report = require('../util/report'); @@ -18,6 +19,7 @@ const report = require('../util/report'); const defaultOptions = { checkFragmentShorthand: false, checkKeyMustBeforeSpread: false, + warnOnDuplicates: false, }; const messages = { @@ -26,6 +28,7 @@ const messages = { missingArrayKey: 'Missing "key" prop for element in array', missingArrayKeyUsePrag: 'Missing "key" prop for element in array. Shorthand fragment syntax does not support providing keys. Use {{reactPrag}}.{{fragPrag}} instead', keyBeforeSpread: '`key` prop must be placed before any `{...spread}, to avoid conflicting with React’s new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html`', + nonUniqueKeys: '`key` prop must be unique', }; module.exports = { @@ -50,6 +53,10 @@ module.exports = { type: 'boolean', default: defaultOptions.checkKeyMustBeforeSpread, }, + warnOnDuplicates: { + type: 'boolean', + default: defaultOptions.warnOnDuplicates, + }, }, additionalProperties: false, }], @@ -59,6 +66,7 @@ module.exports = { const options = Object.assign({}, defaultOptions, context.options[0]); const checkFragmentShorthand = options.checkFragmentShorthand; const checkKeyMustBeforeSpread = options.checkKeyMustBeforeSpread; + const warnOnDuplicates = options.warnOnDuplicates; const reactPragma = pragmaUtil.getFromContext(context); const fragmentPragma = pragmaUtil.getFragmentFromContext(context); @@ -97,19 +105,43 @@ module.exports = { } return { - JSXElement(node) { - if (hasProp(node.openingElement.attributes, 'key')) { - if (checkKeyMustBeforeSpread && isKeyAfterSpread(node.openingElement.attributes)) { - report(context, messages.keyBeforeSpread, 'keyBeforeSpread', { - node, - }); - } + ArrayExpression(node) { + const jsx = node.elements.filter((x) => x.type === 'JSXElement'); + if (jsx.length === 0) { return; } - if (node.parent.type === 'ArrayExpression') { - report(context, messages.missingArrayKey, 'missingArrayKey', { - node, + const map = {}; + jsx.forEach((element) => { + const attrs = element.openingElement.attributes; + const keys = attrs.filter((x) => x.name && x.name.name === 'key'); + + if (keys.length === 0) { + report(context, messages.missingArrayKey, 'missingArrayKey', { + node: element, + }); + } else { + keys.forEach((attr) => { + const value = context.getSourceCode().getText(attr.value); + if (!map[value]) { map[value] = []; } + map[value].push(attr); + + if (checkKeyMustBeforeSpread && isKeyAfterSpread(attrs)) { + report(context, messages.keyBeforeSpread, 'keyBeforeSpread', { + node, + }); + } + }); + } + }); + + if (warnOnDuplicates) { + values(map).filter((v) => v.length > 1).forEach((v) => { + v.forEach((n) => { + report(context, messages.nonUniqueKeys, 'nonUniqueKeys', { + node: n, + }); + }); }); } }, diff --git a/tests/lib/rules/jsx-key.js b/tests/lib/rules/jsx-key.js index bb0b13e5e3..8f13319046 100644 --- a/tests/lib/rules/jsx-key.js +++ b/tests/lib/rules/jsx-key.js @@ -66,6 +66,14 @@ ruleTester.run('jsx-key', rule, { code: '
;', options: [{ checkKeyMustBeforeSpread: true }], }, + { + code: ` + const spans = [ + , + , + ]; + `, + }, ]), invalid: parsers.all([ { @@ -144,5 +152,18 @@ ruleTester.run('jsx-key', rule, { settings, errors: [{ messageId: 'keyBeforeSpread' }], }, + { + code: ` + const spans = [ + , + , + ]; + `, + options: [{ warnOnDuplicates: true }], + errors: [ + { messageId: 'nonUniqueKeys', line: 3 }, + { messageId: 'nonUniqueKeys', line: 4 }, + ], + }, ]), });