Eslint-plugin-react: react/jsx-key Doesn't work w/ destructured props

Created on 2 May 2019  Â·  5Comments  Â·  Source: yannickcr/eslint-plugin-react

A component like this causes this rule to error out, even though there most definitely _is_ a key prop on the underlying JSX:

const Foo = props => (
  [
    <div {...{
      key:1
      {...props} />,
    <div {...{
      key:2
      {...props} />
  ]
)
question

Most helpful comment

If you think you can statically detect the presence of a key in this way, it’d be great to review a PR that does it. I don’t believe it’s possible.

I’m sorry you’re frustrated, but yes, the purpose of eslint rules is absolutely to dictate subjective coding style preferences - it’s just that often they have multiple flavors. I also don’t “lock down” threads anywhere unless it’s a garbage fire of trolling, which this isn’t at this time, so im not sure where that accusation comes from.

You’re always welcome to disable any rule you don’t like.

All 5 comments

This is just the limits of static analysis - that should be inlined like key="1", and not spread in as an object.

const Foo = props => (
  [
    <div key="1" {...props} />,
    <div key="2" {...props} />
  ]
)

is much more efficient and clear anyhow.

that should be inlined like key="1"

Well that's just an opinion of code style.

Doing what you're suggesting would undermine the purpose of using destructured props like so:

render() {
  const {
    foo,
    bar
  ) = this.props
  const baz = foo + bar
  return <div {...{
    key: 1,
    foo,
    bar,
    baz
  }} />
}

Seems rather pedantic to be forced to use _two_ formats for props (key is literally just another prop) just because the linter forces one to do so.

It's more than that; it's clarity and performance, and also, key must be a string, so passing a number isn't even correct.

In general, you should avoid spreading an object - there I'd do <div key="1" foo={foo} bar={bar} baz={baz} />. It's more explicit, avoids bugs, and aids static analysis (which also avoids bugs).

"more explicit"? really? How so?

Not to mention, you're basically enforcing a violation of the DRY principle. Destructuring an object for props also allows for simple, quick // commenting of individual props. There's a number of benefits in using plain JS objectes for props over the pedantic syntax you're suggesting.

But I digress. the subjective syntax code-style opinions here, both yours and mine, are not the issue here. Again, your opinions, shouldn't be dictating to others how code and tooling actually _works_.

In the end, it's perfectly valid JSX to add the key for a component inside of a destructured object and not as a JSX argument. If it weren't, React itself _would still throw a warning_ regarding keys. It does not.

So the fact that ESLint _does_ throw a warning, when the key _is definitely present_, regardless of your subjective syntax preferences, is a bug. Period. That's the only _objective_ way to see this.

But go ahead.. close the ticket, lock it down for commenting, as I've seen you do time and again on this project. So much for listening to your community.

If you think you can statically detect the presence of a key in this way, it’d be great to review a PR that does it. I don’t believe it’s possible.

I’m sorry you’re frustrated, but yes, the purpose of eslint rules is absolutely to dictate subjective coding style preferences - it’s just that often they have multiple flavors. I also don’t “lock down” threads anywhere unless it’s a garbage fire of trolling, which this isn’t at this time, so im not sure where that accusation comes from.

You’re always welcome to disable any rule you don’t like.

Was this page helpful?
0 / 5 - 0 ratings