Eslint-plugin-react: Add no-array-index-key to recommended configuration

[lencioni]

This rule prevents the use of Array index in key. It seems useful enough to turn on as a recommended rule. Thoughts?

[ljharb]

There seem to be some issues with this rule, so I'm going to hold off adding it to the recommended configuration just yet.

[yannickcr]

What issues are remaining (beside documentation) ? I think this is the last thing that is holding back the 7.0.0 right now.

[ljharb]

I don't think this needs to hold back v7 either way; it can just be bumped to v8 if v7 is otherwise ready.

That said, I think the problem is more that conceptual issue that there are legitimate use cases for using an index as the key, and the rule can't statically determine them - so it might be too strong a rule to enable by default in a config.

[yannickcr]

Great! Then if there is no issues in RC1, I will release the v7 tomorrow.

[ljharb]

(#1170 is an example of why I think this rule probably can't be in the recommended set)

[jeffvandyke]

It's WAY too strong of a rule, IMO. I've got an array of strings, how can I possibly concisely display it in a list of

's without using index as the key? It's simply the cleanest way to do it without React throwing a warning.

I still can't think of how you could determine if a component is "simple enough" to not have a useful id field. That would be best, if possible, but I don't think it is.

[jeffvandyke]

As a matter of fact, at the bottom of the Keys section in React docs, it explicitly says: "When you don't have stable IDs for rendered items, you may use the item index as a key as a last resort". For my use case above, which is not rare, what better alternative is there?

[ljharb]

@jeffvandyke you can use the string itself as the key, if they're unique. Obviously if they're not unique, you'd need an index - at which point, you'd use an override comment.

It is definitely appropriate to have an eslint rule that works for the 99% case, but requires an override comment for the 1% case - it happens a lot.

[jeffvandyke]

So I've thought through this a bit, and I'm willing to grant that perhaps my project is a bit more unique, but I find that even after having some element-based keys as you suggested, about 40% of my project's map(...) calls are on arrays of simple data types that are unsuitable as keys due to their non-uniqueness. The arrays are mostly generated from processing raw data where the position in the array really does uniquely identify what that element's identifier is.

I can probably get by with eslint-disable comments, but it feels like a workaround, especially given that React's docs recognize this use case. I'm just not sure if it's that rare of a use case for most other projects either.

[ljharb]

You could always disable the entire rule project-wide; since based on what you're describing, the rule isn't adding value for you.

[PatNeedham]

@ljharb I'm curious, what is the reasoning behind this rule's existence? If one were to map over an array of strings, where there were some duplicates, wouldn't one have to include the array element's index as part of the component key to ensure uniqueness?

[ljharb]

@PatNeedham yes, which is a place where you'd disable the rule. The rule assumes that the common case is for rendering arrays that do contain unique items.