Merge pull request #2085 from himynameisdave/issues/2083

Fix: no-array-index-key not working in React.Children methods
jsx-eslint · Dec 22, 2018 · 14451d4 · 14451d4
2 parents 77e3fd0 + 8be52c7
commit 14451d4
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 4 deletions.
diff --git a/docs/rules/no-array-index-key.md b/docs/rules/no-array-index-key.md
@@ -50,6 +50,14 @@ things.reduce((collection, thing, index) => (
 things.reduceRight((collection, thing, index) => (
   collection.concat(<Hello key={index} />)
 ), []);
+
+React.Children.map(this.props.children, (child, index) => (
+  React.cloneElement(child, { key: index })
+))
+
+Children.forEach(this.props.children, (child, index) => (
+  React.cloneElement(child, { key: index })
+))
 ```
 
 The following patterns are **not** considered warnings:

diff --git a/lib/rules/no-array-index-key.js b/lib/rules/no-array-index-key.js
@@ -7,6 +7,7 @@
 const has = require('has');
 const astUtil = require('../util/ast');
 const docsUrl = require('../util/docsUrl');
+const pragma = require('../util/pragma');
 
 // ------------------------------------------------------------------------------
 // Rule Definition
@@ -47,6 +48,32 @@ module.exports = {
         && indexParamNames.indexOf(node.name) !== -1;
     }
 
+    function isUsingReactChildren(node) {
+      const callee = node.callee;
+      if (
+        !callee
+        || !callee.property
+        || !callee.object
+      ) {
+        return null;
+      }
+
+      const isReactChildMethod = ['map', 'forEach'].indexOf(callee.property.name) > -1;
+      if (!isReactChildMethod) {
+        return null;
+      }
+
+      const obj = callee.object;
+      if (obj && obj.name === 'Children') {
+        return true;
+      }
+      if (obj && obj.object && obj.object.name === pragma.getFromContext(context)) {
+        return true;
+      }
+
+      return false;
+    }
+
     function getMapIndexParamName(node) {
       const callee = node.callee;
       if (callee.type !== 'MemberExpression') {
@@ -59,16 +86,19 @@ module.exports = {
         return null;
       }
 
-      const firstArg = node.arguments[0];
-      if (!firstArg) {
+      const callbackArg = isUsingReactChildren(node)
+        ? node.arguments[1]
+        : node.arguments[0];
+
+      if (!callbackArg) {
         return null;
       }
 
-      if (!astUtil.isFunctionLikeExpression(firstArg)) {
+      if (!astUtil.isFunctionLikeExpression(callbackArg)) {
         return null;
       }
 
-      const params = firstArg.params;
+      const params = callbackArg.params;
 
       const indexParamPosition = iteratorFunctionsToIndexParamPosition[callee.property.name];
       if (params.length < indexParamPosition + 1) {

diff --git a/tests/lib/rules/no-array-index-key.js b/tests/lib/rules/no-array-index-key.js
@@ -89,6 +89,22 @@ ruleTester.run('no-array-index-key', rule, {
 
     {
       code: 'foo.reduceRight((a, b, i) => a.concat(<Foo key={b.id} />), [])'
+    },
+
+    {
+      code: `
+      React.Children.map(this.props.children, (child, index, arr) => {
+        return React.cloneElement(child, { key: child.id });
+      })
+      `
+    },
+
+    {
+      code: `
+      Children.forEach(this.props.children, (child, index, arr) => {
+        return React.cloneElement(child, { key: child.id });
+      })
+      `
     }
   ],
 
@@ -227,6 +243,43 @@ ruleTester.run('no-array-index-key', rule, {
     {
       code: 'foo.findIndex((bar, i) => { baz.push(React.createElement(\'Foo\', { key: i })); })',
       errors: [{message: 'Do not use Array index in keys'}]
+    },
+
+    {
+      code: `
+      Children.map(this.props.children, (child, index) => {
+        return React.cloneElement(child, { key: index });
+      })
+      `,
+      errors: [{message: 'Do not use Array index in keys'}]
+    },
+
+    {
+      code: `
+      React.Children.map(this.props.children, (child, index) => {
+        return React.cloneElement(child, { key: index });
+      })
+      `,
+      errors: [{message: 'Do not use Array index in keys'}]
+    },
+
+    {
+      code: `
+      Children.forEach(this.props.children, (child, index) => {
+        return React.cloneElement(child, { key: index });
+      })
+      `,
+      errors: [{message: 'Do not use Array index in keys'}]
+    },
+
+    {
+      code: `
+      React.Children.forEach(this.props.children, (child, index) => {
+        return React.cloneElement(child, { key: index });
+      })
+      `,
+      errors: [{message: 'Do not use Array index in keys'}]
     }
+
   ]
 });