Skip to content

Commit

Permalink
[Fix] jsx-key: detect keys in Array.from's mapping function
Browse files Browse the repository at this point in the history
  • Loading branch information
sjarva authored and ljharb committed Aug 25, 2022
1 parent b0d0ca1 commit bb4d1b3
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 15 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

## Unreleased

### Fixed
* [`jsx-key`]: fix detecting missing key in `Array.from`'s mapping function ([#3369][] @sjarva)

[#3369]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3369

## [7.31.0] - 2022.08.24

### Added
Expand Down
8 changes: 8 additions & 0 deletions docs/rules/jsx-key.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ data.map(x => <Hello>{x}</Hello>);
<Hello {...{ key: id, id, caption }} />
```

```jsx
Array.from([1, 2, 3], (x) => <Hello>{x}</Hello>);
```

In the last example the key is being spread, which is currently possible, but discouraged in favor of the statically provided key.

Examples of **correct** code for this rule:
Expand All @@ -37,6 +41,10 @@ data.map((x) => <Hello key={x.id}>{x}</Hello>);
<Hello key={id} {...{ id, caption }} />
```

```jsx
Array.from([1, 2, 3], (x) => <Hello key={x.id}>{x}</Hello>);
```

## Rule Options

```js
Expand Down
61 changes: 46 additions & 15 deletions lib/rules/jsx-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const values = require('object.values');
const docsUrl = require('../util/docsUrl');
const pragmaUtil = require('../util/pragma');
const report = require('../util/report');
const astUtil = require('../util/ast');

// ------------------------------------------------------------------------------
// Rule Definition
Expand Down Expand Up @@ -124,6 +125,36 @@ module.exports = {
});
}

/**
* Checks if the given node is a function expression or arrow function,
* and checks if there is a missing key prop in return statement's arguments
* @param {ASTNode} node
*/
function checkFunctionsBlockStatement(node) {
if (astUtil.isFunctionLikeExpression(node)) {
if (node.body.type === 'BlockStatement') {
getReturnStatements(node.body)
.filter((returnStatement) => returnStatement && returnStatement.argument)
.forEach((returnStatement) => {
checkIteratorElement(returnStatement.argument);
});
}
}
}

/**
* Checks if the given node is an arrow function that has an JSX Element or JSX Fragment in its body,
* and the JSX is missing a key prop
* @param {ASTNode} node
*/
function checkArrowFunctionWithJSX(node) {
const isArrFn = node && node.type === 'ArrowFunctionExpression';

if (isArrFn && (node.body.type === 'JSXElement' || node.body.type === 'JSXFragment')) {
checkIteratorElement(node.body);
}
}

const seen = new WeakSet();

return {
Expand Down Expand Up @@ -196,26 +227,26 @@ module.exports = {
OptionalCallExpression[callee.type="MemberExpression"][callee.property.name="map"],\
OptionalCallExpression[callee.type="OptionalMemberExpression"][callee.property.name="map"]'(node) {
const fn = node.arguments[0];
const isFn = fn && fn.type === 'FunctionExpression';
const isArrFn = fn && fn.type === 'ArrowFunctionExpression';

if (!fn && !isFn && !isArrFn) {
if (!astUtil.isFunctionLikeExpression(fn)) {
return;
}

if (isArrFn && (fn.body.type === 'JSXElement' || fn.body.type === 'JSXFragment')) {
checkIteratorElement(fn.body);
}
checkArrowFunctionWithJSX(fn);

if (isFn || isArrFn) {
if (fn.body.type === 'BlockStatement') {
getReturnStatements(fn.body)
.filter((returnStatement) => returnStatement && returnStatement.argument)
.forEach((returnStatement) => {
checkIteratorElement(returnStatement.argument);
});
}
checkFunctionsBlockStatement(fn);
},

// Array.from
'CallExpression[callee.type="MemberExpression"][callee.property.name="from"]'(node) {
const fn = node.arguments.length > 1 && node.arguments[1];

if (!astUtil.isFunctionLikeExpression(fn)) {
return;
}

checkArrowFunctionWithJSX(fn);

checkFunctionsBlockStatement(fn);
},
};
},
Expand Down
17 changes: 17 additions & 0 deletions tests/lib/rules/jsx-key.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ ruleTester.run('jsx-key', rule, {
{ code: '[1, 2, 3].map(function(x) { return <App key={x} /> });' },
{ code: '[1, 2, 3].map(x => <App key={x} />);' },
{ code: '[1, 2, 3].map(x => { return <App key={x} /> });' },
{ code: 'Array.from([1, 2, 3], function(x) { return <App key={x} /> });' },
{ code: 'Array.from([1, 2, 3], (x => <App key={x} />));' },
{ code: 'Array.from([1, 2, 3], (x => {return <App key={x} />}));' },
{ code: 'Array.from([1, 2, 3], someFn);' },
{ code: 'Array.from([1, 2, 3]);' },
{ code: '[1, 2, 3].foo(x => <App />);' },
{ code: 'var App = () => <div />;' },
{ code: '[1, 2, 3].map(function(x) { return; });' },
Expand Down Expand Up @@ -174,6 +179,18 @@ ruleTester.run('jsx-key', rule, {
code: '[1, 2 ,3].map(x => { return <App /> });',
errors: [{ messageId: 'missingIterKey' }],
},
{
code: 'Array.from([1, 2 ,3], function(x) { return <App /> });',
errors: [{ messageId: 'missingIterKey' }],
},
{
code: 'Array.from([1, 2 ,3], (x => { return <App /> }));',
errors: [{ messageId: 'missingIterKey' }],
},
{
code: 'Array.from([1, 2 ,3], (x => <App />));',
errors: [{ messageId: 'missingIterKey' }],
},
{
code: '[1, 2, 3]?.map(x => <BabelEslintApp />)',
features: ['no-default'],
Expand Down

0 comments on commit bb4d1b3

Please sign in to comment.