Skip to content

Commit fe32c7d

Browse files
lencionilencioni
authored
Merge pull request #978 from yannickcr/no-array-index-key
Add no-array-index-key rule
2 parents 333938f + cff595e commit fe32c7d

File tree

5 files changed

+549
-0
lines changed

5 files changed

+549
-0
lines changed

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
8383
* [react/display-name](docs/rules/display-name.md): Prevent missing `displayName` in a React component definition
8484
* [react/forbid-component-props](docs/rules/forbid-component-props.md): Forbid certain props on Components
8585
* [react/forbid-prop-types](docs/rules/forbid-prop-types.md): Forbid certain propTypes
86+
* [react/no-array-index-key](docs/rules/no-array-index-key.md): Prevent using Array index in `key` props
8687
* [react/no-children-prop](docs/rules/no-children-prop.md): Prevent passing children as props
8788
* [react/no-danger](docs/rules/no-danger.md): Prevent usage of dangerous JSX properties
8889
* [react/no-danger-with-children](docs/rules/no-danger-with-children.md): Prevent problem with children and props.dangerouslySetInnerHTML

‎docs/rules/no-array-index-key.md

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
# Prevent usage of Array index in keys
2+
3+
Warn if an element uses an Array index in its `key`.
4+
5+
The `key` is used by React to [identify which items have changed, are added, or are removed and should be stable](https://facebook.github.io/react/docs/lists-and-keys.html#keys).
6+
7+
It's a bad idea to use the array index since it doesn't uniquely identify your elements. In cases where the array is sorted or an element is added to the beginning of the array, the index will be changed even though the element representing that index may be the same. This results in in unnecessary renders.
8+
9+
## Rule Details
10+
11+
The following patterns are considered warnings:
12+
13+
```jsx
14+
things.map((thing, index) => (
15+
<Hello key={index} />
16+
));
17+
18+
things.map((thing, index) => (
19+
React.cloneElement(thing, { key: index })
20+
));
21+
22+
things.forEach((thing, index) => {
23+
otherThings.push(<Hello key={index} />);
24+
});
25+
26+
things.filter((thing, index) => {
27+
otherThings.push(<Hello key={index} />);
28+
});
29+
30+
things.some((thing, index) => {
31+
otherThings.push(<Hello key={index} />);
32+
});
33+
34+
things.every((thing, index) => {
35+
otherThings.push(<Hello key={index} />);
36+
});
37+
38+
things.find((thing, index) => {
39+
otherThings.push(<Hello key={index} />);
40+
});
41+
42+
things.findIndex((thing, index) => {
43+
otherThings.push(<Hello key={index} />);
44+
});
45+
46+
things.reduce((collection, thing, index) => (
47+
collection.concat(<Hello key={index} />)
48+
), []);
49+
50+
things.reduceRight((collection, thing, index) => (
51+
collection.concat(<Hello key={index} />)
52+
), []);
53+
```
54+
55+
The following patterns are not considered warnings:
56+
57+
```jsx
58+
things.map((thing) => (
59+
<Hello key={thing.id} />
60+
));
61+
62+
things.map((thing) => (
63+
React.cloneElement(thing, { key: thing.id })
64+
));
65+
66+
things.forEach((thing) => {
67+
otherThings.push(<Hello key={thing.id} />);
68+
});
69+
70+
things.filter((thing) => {
71+
otherThings.push(<Hello key={thing.id} />);
72+
});
73+
74+
things.some((thing) => {
75+
otherThings.push(<Hello key={thing.id} />);
76+
});
77+
78+
things.every((thing) => {
79+
otherThings.push(<Hello key={thing.id} />);
80+
});
81+
82+
things.find((thing) => {
83+
otherThings.push(<Hello key={thing.id} />);
84+
});
85+
86+
things.findIndex((thing) => {
87+
otherThings.push(<Hello key={thing.id} />);
88+
});
89+
90+
things.reduce((collection, thing) => (
91+
collection.concat(<Hello key={thing.id} />)
92+
), []);
93+
94+
things.reduceRight((collection, thing) => (
95+
collection.concat(<Hello key={thing.id} />)
96+
), []);
97+
```

‎index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ var allRules = {
88
'jsx-wrap-multilines': require('./lib/rules/jsx-wrap-multilines'),
99
'self-closing-comp': require('./lib/rules/self-closing-comp'),
1010
'jsx-no-comment-textnodes': require('./lib/rules/jsx-no-comment-textnodes'),
11+
'no-array-index-key': require('./lib/rules/no-array-index-key'),
1112
'no-danger': require('./lib/rules/no-danger'),
1213
'no-set-state': require('./lib/rules/no-set-state'),
1314
'no-is-mounted': require('./lib/rules/no-is-mounted'),

‎lib/rules/no-array-index-key.js

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
/**
2+
* @fileoverview Prevent usage of Array index in keys
3+
* @author Joe Lencioni
4+
*/
5+
'use strict';
6+
7+
// ------------------------------------------------------------------------------
8+
// Rule Definition
9+
// ------------------------------------------------------------------------------
10+
11+
module.exports = {
12+
meta: {
13+
docs: {
14+
description: 'Prevent usage of Array index in keys',
15+
category: 'Best Practices',
16+
recommended: false
17+
},
18+
19+
schema: []
20+
},
21+
22+
create: function(context) {
23+
// --------------------------------------------------------------------------
24+
// Public
25+
// --------------------------------------------------------------------------
26+
var indexParamNames = [];
27+
var iteratorFunctionsToIndexParamPosition = {
28+
every: 1,
29+
filter: 1,
30+
find: 1,
31+
findIndex: 1,
32+
forEach: 1,
33+
map: 1,
34+
reduce: 2,
35+
reduceRight: 2,
36+
some: 1
37+
};
38+
var ERROR_MESSAGE = 'Do not use Array index in keys';
39+
40+
function isArrayIndex(node) {
41+
return node.type === 'Identifier'
42+
&& indexParamNames.indexOf(node.name) !== -1;
43+
}
44+
45+
function getMapIndexParamName(node) {
46+
var callee = node.callee;
47+
if (callee.type !== 'MemberExpression') {
48+
return null;
49+
}
50+
if (callee.property.type !== 'Identifier') {
51+
return null;
52+
}
53+
if (!iteratorFunctionsToIndexParamPosition.hasOwnProperty(callee.property.name)) {
54+
return null;
55+
}
56+
57+
var firstArg = node.arguments[0];
58+
if (!firstArg) {
59+
return null;
60+
}
61+
62+
var isFunction = [
63+
'ArrowFunctionExpression',
64+
'FunctionExpression'
65+
].indexOf(firstArg.type) !== -1;
66+
if (!isFunction) {
67+
return null;
68+
}
69+
70+
var params = firstArg.params;
71+
72+
var indexParamPosition = iteratorFunctionsToIndexParamPosition[callee.property.name];
73+
if (params.length < indexParamPosition + 1) {
74+
return null;
75+
}
76+
77+
return params[indexParamPosition].name;
78+
}
79+
80+
function getIdentifiersFromBinaryExpression(side) {
81+
if (side.type === 'Identifier') {
82+
return side;
83+
}
84+
85+
if (side.type === 'BinaryExpression') {
86+
// recurse
87+
var left = getIdentifiersFromBinaryExpression(side.left);
88+
var right = getIdentifiersFromBinaryExpression(side.right);
89+
return [].concat(left, right).filter(Boolean);
90+
}
91+
92+
return null;
93+
}
94+
95+
function checkPropValue(node) {
96+
if (isArrayIndex(node)) {
97+
// key={bar}
98+
context.report({
99+
node: node,
100+
message: ERROR_MESSAGE
101+
});
102+
return;
103+
}
104+
105+
if (node.type === 'TemplateLiteral') {
106+
// key={`foo-${bar}`}
107+
node.expressions.filter(isArrayIndex).forEach(function() {
108+
context.report({node: node, message: ERROR_MESSAGE});
109+
});
110+
111+
return;
112+
}
113+
114+
if (node.type === 'BinaryExpression') {
115+
// key={'foo' + bar}
116+
var identifiers = getIdentifiersFromBinaryExpression(node);
117+
118+
identifiers.filter(isArrayIndex).forEach(function() {
119+
context.report({node: node, message: ERROR_MESSAGE});
120+
});
121+
122+
return;
123+
}
124+
}
125+
126+
return {
127+
CallExpression: function(node) {
128+
if (
129+
node.callee
130+
&& node.callee.type === 'MemberExpression'
131+
&& ['createElement', 'cloneElement'].indexOf(node.callee.property.name) !== -1
132+
&& node.arguments.length > 1
133+
) {
134+
// React.createElement
135+
if (!indexParamNames.length) {
136+
return;
137+
}
138+
139+
var props = node.arguments[1];
140+
141+
if (props.type !== 'ObjectExpression') {
142+
return;
143+
}
144+
145+
props.properties.forEach(function (prop) {
146+
if (prop.key.name !== 'key') {
147+
// { foo: bar }
148+
return;
149+
}
150+
151+
checkPropValue(prop.value);
152+
});
153+
154+
return;
155+
}
156+
157+
var mapIndexParamName = getMapIndexParamName(node);
158+
if (!mapIndexParamName) {
159+
return;
160+
}
161+
162+
indexParamNames.push(mapIndexParamName);
163+
},
164+
165+
JSXAttribute: function(node) {
166+
if (node.name.name !== 'key') {
167+
// foo={bar}
168+
return;
169+
}
170+
171+
if (!indexParamNames.length) {
172+
// Not inside a call expression that we think has an index param.
173+
return;
174+
}
175+
176+
var value = node.value;
177+
if (value.type !== 'JSXExpressionContainer') {
178+
// key='foo'
179+
return;
180+
}
181+
182+
checkPropValue(value.expression);
183+
},
184+
185+
'CallExpression:exit': function(node) {
186+
var mapIndexParamName = getMapIndexParamName(node);
187+
if (!mapIndexParamName) {
188+
return;
189+
}
190+
191+
indexParamNames.pop();
192+
}
193+
};
194+
195+
}
196+
};

0 commit comments

Comments
 (0)