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

[RavenHursT]

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} />
  ]
)

[ljharb]

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.

[RavenHursT]

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.

[ljharb]

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).

[RavenHursT]

"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.

[ljharb]

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.

[osdiab]

@ljharb can this work for typescript projects, where the type information is known? or is that out of scope for this package? I ask since libraries like downshift and react-table provide getXProps() functions that do provide the key, but they dont seem to get picked up by the jsx-key rule.

[ljharb]

@osdiab the rule could be enhanced with type information when available, yes. A PR that does that would be welcome.

[akomm]

Quote: key must be a string... is just wrong:

// excerpt from @types/react
declare namespace React {
    type Key = string | number
}

I'd say the typing represents the reality even in absence of typing -> or the typing is wrong?

Regarding issue: Example: https://react-table.tanstack.com/docs/quick-start#applying-the-table-instance-to-markup

But its such a common case in general.

I understand the problem that this plugin is not a typescript plugin, so it currently does not type-check, right? The question is whether this here needs to solve the problem, or rather when using ts, you would disable this one and there needs to be a plugin (or added rule to existing TS plugin) to catch key issues using TS typing

[ljharb]

@akomm this issue is about destructuring and spreading; assuming it allows string and number, certainly we should update the error message here. Want to make a PR?

[salemhilal]

Could this issue be re-opened in absence of a PR to use type information, or would you prefer a separate issue? I also ended up here because I'm also using react-table, which has you spreading props on your own elements pretty often.

[ljharb]

@salemhilal my comment above is about updating the error message only.

[salemhilal]

@ljharb Sorry, I was referencing your comment further up the thread: https://github.com/yannickcr/eslint-plugin-react/issues/2258#issuecomment-860785176

[ljharb]

ah. I think a new issue for that would be best.