Skip to content

Commit

Permalink
[New] jsx-key: add warnDuplicates option to warn on duplicate jsx…
Browse files Browse the repository at this point in the history
… keys in an array

Fixes #2614
  • Loading branch information
ljharb committed Feb 19, 2022
1 parent 227cf88 commit dbc8e8c
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions docs/rules/jsx-key.md
Expand Up @@ -57,6 +57,19 @@ Examples of **incorrect** code for this rule:
<span {...spread} key={"key-after-spread"} />;
```

### `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 = [
<span key="notunique"/>,
<span key="notunique"/>,
];
```

## When Not To Use It

If you are not using JSX then you can disable this rule.
Expand Down
52 changes: 42 additions & 10 deletions lib/rules/jsx-key.js
Expand Up @@ -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');
Expand All @@ -18,6 +19,7 @@ const report = require('../util/report');
const defaultOptions = {
checkFragmentShorthand: false,
checkKeyMustBeforeSpread: false,
warnOnDuplicates: false,
};

const messages = {
Expand All @@ -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 = {
Expand All @@ -50,6 +53,10 @@ module.exports = {
type: 'boolean',
default: defaultOptions.checkKeyMustBeforeSpread,
},
warnOnDuplicates: {
type: 'boolean',
default: defaultOptions.warnOnDuplicates,
},
},
additionalProperties: false,
}],
Expand All @@ -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);

Expand Down Expand Up @@ -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,
});
});
});
}
},
Expand Down
21 changes: 21 additions & 0 deletions tests/lib/rules/jsx-key.js
Expand Up @@ -66,6 +66,14 @@ ruleTester.run('jsx-key', rule, {
code: '<div key="keyBeforeSpread" {...{}} />;',
options: [{ checkKeyMustBeforeSpread: true }],
},
{
code: `
const spans = [
<span key="notunique"/>,
<span key="notunique"/>,
];
`,
},
]),
invalid: parsers.all([
{
Expand Down Expand Up @@ -144,5 +152,18 @@ ruleTester.run('jsx-key', rule, {
settings,
errors: [{ messageId: 'keyBeforeSpread' }],
},
{
code: `
const spans = [
<span key="notunique"/>,
<span key="notunique"/>,
];
`,
options: [{ warnOnDuplicates: true }],
errors: [
{ messageId: 'nonUniqueKeys', line: 3 },
{ messageId: 'nonUniqueKeys', line: 4 },
],
},
]),
});

0 comments on commit dbc8e8c

Please sign in to comment.