Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no-array-index-key can be bypassed with toString #2140

Closed
kognise opened this issue Jan 22, 2019 · 5 comments · Fixed by #2813
Closed

no-array-index-key can be bypassed with toString #2140

kognise opened this issue Jan 22, 2019 · 5 comments · Fixed by #2813

Comments

@kognise
Copy link

kognise commented Jan 22, 2019

You can bypass the no-array-index-key rule with toString().

In other words, this gives an error:

foo.map((bar, index) => (
  <Element key={index} bar={bar} />
))

But this doesn't:

foo.map((bar, index) => (
  <Element key={index.toString()} bar={bar} />
))
@ljharb
Copy link
Member

ljharb commented Jan 22, 2019

There’s a lot of ways to bypass it; it’s not really practical to try to catch them all. The rule is meant as a helpful warning not to rely on indexes in a key, when your items have a unique signature.

@kognise
Copy link
Author

kognise commented Jan 22, 2019

Ok, got it. Should I close this issue?

@ljharb
Copy link
Member

ljharb commented Jan 22, 2019

I’m not sure. We can certainly detect this specific pattern, but I’m not sure how valuable it’d be, especially considering that x.toString() is an anti pattern (prefer String(x)). What do you think?

@kognise
Copy link
Author

kognise commented Jan 23, 2019

Personally, as a naive Reacter, have used index.toString() many times. I am slowly replacing that code with a better approach, but it would've been nice to have been warned.

@ljharb
Copy link
Member

ljharb commented Feb 15, 2022

@kognise in all javascript, everywhere, including all "native" React (RN or React Web), String(x) is preferred over x.toString(), full stop. That said, String(x) is also not warned on, so that fact is moot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants