From 925450f753feac365b46ead62b64501ed4a133d3 Mon Sep 17 00:00:00 2001 From: himynameisdave Date: Wed, 12 Dec 2018 18:29:48 -0800 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=94=A7=20Fix=20for=20usage=20of=20Rea?= =?UTF-8?q?ct.Children=20methods?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the case where you are iterating over a children prop via the React.Children API. --- lib/rules/no-array-index-key.js | 41 ++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/lib/rules/no-array-index-key.js b/lib/rules/no-array-index-key.js index c5b1fe0a32..92cbddc216 100644 --- a/lib/rules/no-array-index-key.js +++ b/lib/rules/no-array-index-key.js @@ -47,6 +47,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 === 'React') { + return true; + } + + return false; + } + function getMapIndexParamName(node) { const callee = node.callee; if (callee.type !== 'MemberExpression') { @@ -59,16 +85,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) { @@ -132,24 +161,20 @@ module.exports = { && ['createElement', 'cloneElement'].indexOf(node.callee.property.name) !== -1 && node.arguments.length > 1 ) { - // React.createElement if (!indexParamNames.length) { return; } - const props = node.arguments[1]; if (props.type !== 'ObjectExpression') { return; } - props.properties.forEach(prop => { if (!prop.key || prop.key.name !== 'key') { // { ...foo } // { foo: bar } return; } - checkPropValue(prop.value); }); From cb425c3060655b05a2d38d939c7d3a8d2b09dbb0 Mon Sep 17 00:00:00 2001 From: himynameisdave Date: Wed, 12 Dec 2018 18:30:50 -0800 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=94=AC=20Adds=20unit=20test=20coverag?= =?UTF-8?q?e=20for=20new=20use=20cases?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/lib/rules/no-array-index-key.js | 53 +++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/lib/rules/no-array-index-key.js b/tests/lib/rules/no-array-index-key.js index b27932ec67..222f5810ed 100644 --- 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(), [])' + }, + + { + 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'}] } + ] }); From 9253c10fc1e18d38138aa33ddea77214560cc62b Mon Sep 17 00:00:00 2001 From: himynameisdave Date: Wed, 12 Dec 2018 18:32:43 -0800 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=93=96=20Adds=20warning=20cases=20to?= =?UTF-8?q?=20rule=20documentation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/rules/no-array-index-key.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/rules/no-array-index-key.md b/docs/rules/no-array-index-key.md index 0b8e6337e3..9f401afbd9 100644 --- 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() ), []); + +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: From 8be52c754f0bc5060dc9ef572509a99119069961 Mon Sep 17 00:00:00 2001 From: himynameisdave Date: Fri, 21 Dec 2018 11:37:32 -0500 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=93=9D=20Addresses=20CR=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Uses pragma context instead of 'React' - Adds back spaces I removed which increase readability --- lib/rules/no-array-index-key.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-array-index-key.js b/lib/rules/no-array-index-key.js index 92cbddc216..0d0a7f3fdc 100644 --- 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 @@ -66,7 +67,7 @@ module.exports = { if (obj && obj.name === 'Children') { return true; } - if (obj && obj.object && obj.object.name === 'React') { + if (obj && obj.object && obj.object.name === pragma.getFromContext(context)) { return true; } @@ -161,20 +162,24 @@ module.exports = { && ['createElement', 'cloneElement'].indexOf(node.callee.property.name) !== -1 && node.arguments.length > 1 ) { + // React.createElement if (!indexParamNames.length) { return; } + const props = node.arguments[1]; if (props.type !== 'ObjectExpression') { return; } + props.properties.forEach(prop => { if (!prop.key || prop.key.name !== 'key') { // { ...foo } // { foo: bar } return; } + checkPropValue(prop.value); });