Eslint-plugin-react: [destructuring-assignment] False positive.

[RebeccaStevens]

I believe the following case is a false positive for the "react/destructuring-assignment" rule.

const MyComponentWrapper = ({ ...props }) => {
  const someValue = props.someObject?.someValue;
  //                ~~~~~~~~~~~~~~~~ Must use destructuring props assignment - eslint(react/destructuring-assignment)

  if (someValue !== undefined) {
    props.someOtherThing = getSomething(someValue);
  }

  return <MyComponent {...props} />;
};

Note: If props is renamed to something else, the error goes away.

[ljharb]

I don’t think it is - first of all, why on earth would you object spread props and make a new object unnecessarily? Second, you want const { someObject } = props; const someValue = someObject?.someValue;. And third, you probably want the signature to be ({ someObject, someOtherThing }) => {.

[ljharb]

The bug is that it depends on the variable name; it should be warning on any name.

[RebeccaStevens]

Thanks for fast reply.

My intention with object spreading props to make a new object was just to make a shallow clone of props. This has the same effect as const { someObject } = props; but for all properties of props. However, yes I agree with your point 2; being explicit is the better way to do this.

With regard to point 3, doing this would remove the purpose of the wrapper. That purpose being to make props.someOtherThing a computed value based on props.someObject?.someValue.

Edit: Just realised that the point of const { someObject } = props; isn't to shallow clone at all - as that's not what it does. Sorry for being dumb; I'm still not used to destructuring assignment.

[ljharb]

Why would you need a shallow clone of the props object? It's react, you're not mutating it, and anything that mutates that object is terrible and should be excised.

[ljharb]

ah, i see you are mutating it :-) i'd strongly suggest not doing that, and instead, doing this:

const MyComponentWrapper = ({ someObject }) => {
  const someValue = someObject?.someValue;

  const someOtherThing = someValue ?? getSomething(someValue);

  return <MyComponent {...props} someOtherThing={someOtherThing} />;
};

[RebeccaStevens]

That looks much nicer. Thanks

[jzabala]

@ljharb Should this one be closed?