Skip to content

Commit

Permalink
[Fix] no-array-index-key: detect named-imported cloneElement/`cre…
Browse files Browse the repository at this point in the history 
…ateElement`

Fixes #3213
  • Loading branch information
@ljharb
ljharb committed Feb 26, 2022
1 parent 625010a commit 7fdfd2c
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 18 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -9,9 +9,11 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`jsx-curly-brace-presence`]: avoid warning on curlies containing quote characters ([#3214][] @ljharb)
* [`jsx-indent`]: do not report on non-jsx-returning ternaries that contain null ([#3222][] @ljharb)
* [`jsx-indent`]: properly report on returned ternaries with jsx ([#3222][] @ljharb)
* [`no-array-index-key`]: detect named-imported `cloneElement`/`createElement` ([#3213][] @ljharb)

[#3222]: https://github.com/yannickcr/eslint-plugin-react/issues/3222
[#3214]: https://github.com/yannickcr/eslint-plugin-react/issues/3214
[#3213]: https://github.com/yannickcr/eslint-plugin-react/issues/3213

## [7.29.1] - 2022.02.25

Expand Down
29 changes: 23 additions & 6 deletions lib/rules/no-array-index-key.js
View file
Expand Up @@ -10,11 +10,33 @@ const astUtil = require('../util/ast');
const docsUrl = require('../util/docsUrl');
const pragma = require('../util/pragma');
const report = require('../util/report');
const variableUtil = require('../util/variable');

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

function isCreateCloneElement(node, context) {
if (!node) {
return false;
}

if (node.type === 'MemberExpression' || node.type === 'OptionalMemberExpression') {
return node.object
&& node.object.name === pragma.getFromContext(context)
&& ['createElement', 'cloneElement'].indexOf(node.property.name) !== -1;
}

if (node.type === 'Identifier') {
const variable = variableUtil.findVariableByName(context, node.name);
if (variable && variable.type === 'ImportSpecifier') {
return variable.parent.source.value === 'react';
}
}

return false;
}

const messages = {
noArrayIndex: 'Do not use Array index in keys',
};
Expand Down Expand Up @@ -205,12 +227,7 @@ module.exports = {

return {
'CallExpression, OptionalCallExpression'(node) {
if (
node.callee
&& (node.callee.type === 'MemberExpression' || node.callee.type === 'OptionalMemberExpression')
&& ['createElement', 'cloneElement'].indexOf(node.callee.property.name) !== -1
&& node.arguments.length > 1
) {
if (isCreateCloneElement(node.callee, context) && node.arguments.length > 1) {
// React.createElement
if (!indexParamNames.length) {
return;
Expand Down
4 changes: 4 additions & 0 deletions lib/util/variable.js
View file
Expand Up @@ -69,6 +69,10 @@ function findVariableByName(context, name) {
return variable.defs[0].node.right;
}

if (variable.defs[0].type === 'ImportBinding') {
return variable.defs[0].node;
}

return variable.defs[0].node.init;
}

Expand Down
110 changes: 98 additions & 12 deletions tests/lib/rules/no-array-index-key.js
View file
Expand Up @@ -54,6 +54,9 @@ ruleTester.run('no-array-index-key', rule, {
{
code: 'foo.map((baz, i) => React.cloneElement(someChild, { ...someChild.props }))',
},
{
code: 'foo.map((baz, i) => cloneElement(someChild, { ...someChild.props }))',
},
{
code: `
foo.map((item, i) => {
Expand All @@ -63,6 +66,15 @@ ruleTester.run('no-array-index-key', rule, {
})
`,
},
{
code: `
foo.map((item, i) => {
return cloneElement(someChild, {
key: item.id
})
})
`,
},
{
code: 'foo.map((baz, i) => <Foo key />)',
},
Expand Down Expand Up @@ -100,13 +112,27 @@ ruleTester.run('no-array-index-key', rule, {
})
`,
},
{
code: `
React.Children.map(this.props.children, (child, index, arr) => {
return cloneElement(child, { key: child.id });
})
`,
},
{
code: `
Children.forEach(this.props.children, (child, index, arr) => {
return React.cloneElement(child, { key: child.id });
})
`,
},
{
code: `
Children.forEach(this.props.children, (child, index, arr) => {
return cloneElement(child, { key: child.id });
})
`,
},
{
code: 'foo?.map(child => <Foo key={child.i} />)',
features: ['optional chaining'],
Expand Down Expand Up @@ -145,6 +171,14 @@ ruleTester.run('no-array-index-key', rule, {
code: 'foo.map((baz, i) => React.cloneElement(someChild, { ...someChild.props, key: i }))',
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
import { cloneElement } from 'react';
foo.map((baz, i) => cloneElement(someChild, { ...someChild.props, key: i }))
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
foo.map((item, i) => {
Expand All @@ -155,6 +189,18 @@ ruleTester.run('no-array-index-key', rule, {
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
import { cloneElement } from 'react';
foo.map((item, i) => {
return cloneElement(someChild, {
key: i
})
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: 'foo.forEach((bar, i) => { baz.push(<Foo key={i} />); })',
errors: [{ messageId: 'noArrayIndex' }],
Expand Down Expand Up @@ -229,33 +275,73 @@ ruleTester.run('no-array-index-key', rule, {
},
{
code: `
Children.map(this.props.children, (child, index) => {
return React.cloneElement(child, { key: index });
})
Children.map(this.props.children, (child, index) => {
return React.cloneElement(child, { key: index });
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
import { cloneElement } from 'react';
Children.map(this.props.children, (child, index) => {
return cloneElement(child, { key: index });
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
React.Children.map(this.props.children, (child, index) => {
return React.cloneElement(child, { key: index });
})
React.Children.map(this.props.children, (child, index) => {
return React.cloneElement(child, { key: index });
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
Children.forEach(this.props.children, (child, index) => {
return React.cloneElement(child, { key: index });
})
import { cloneElement } from 'react';
React.Children.map(this.props.children, (child, index) => {
return cloneElement(child, { key: index });
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
React.Children.forEach(this.props.children, (child, index) => {
return React.cloneElement(child, { key: index });
})
Children.forEach(this.props.children, (child, index) => {
return React.cloneElement(child, { key: index });
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
import { cloneElement } from 'react';
Children.forEach(this.props.children, (child, index) => {
return cloneElement(child, { key: index });
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
React.Children.forEach(this.props.children, (child, index) => {
return React.cloneElement(child, { key: index });
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
{
code: `
import { cloneElement } from 'react';
React.Children.forEach(this.props.children, (child, index) => {
return cloneElement(child, { key: index });
})
`,
errors: [{ messageId: 'noArrayIndex' }],
},
Expand Down

0 comments on commit 7fdfd2c

Please sign in to comment.