[Bug]: `jsx-no-leaked-render` report `&&` in prop

[AhmedBaset]

Is there an existing issue for this?

• [X] I have searched the existing issues and my issue is unique
• [X] My issue appears in the command-line and not only in the text editor

Description Overview

Imagine you have a prop (e.g. isOpen), you want to set the value as firstCondition === true && anotherCondition === true. Currently the plugin report wrongly this kind of code which should be valid.

Expected Behavior

I expect to not throw

eslint-plugin-react version

7.34.1

eslint version

8.57.0

node version

18

[ljharb]

i agree, this should be valid (altho !!firstCondition && !!anotherCondition would be more idiomatic)

[AhmedBaset]

One more thing is frustrating to me.

number && value might cause unintentional reslut but boolean && value won't.

Why the plugin requires ternary all the time even when no potential unexpected result? Could we use the TypeScrypt parser to only report the potential errors? && conditional rendering is a JS feature not something developer should avoid.

[ljharb]

@A7med3bdulBaset specifically because a 0 is falsy but will render a literal zero - that's the point of the rule, and indeed is why && conditional rendering is dangerous.

[AhmedBaset]

I understand well && and its cons/pros.

I know 0 && content will render 0. My point is why boolean && content, contentOrNullOrUndefinedOrBoolean && content is prevented?

[ljharb]

I agree that neither of those should be prevented when the type is known.

[AhmedBaset]

Thanks @akulsr0 for working on this

[AhmedBaset]

I know 0 && content will render 0. My point is why boolean && content, contentOrNullOrUndefinedOrBoolean && content is prevented?

I agree that neither of those should be prevented when the type is known.

@ljharb, should I create an issue for this?

[ljharb]

@AhmedBaset if, with the latest unreleased code, you still have an issue, please do file one.

[AhmedBaset]

<Comp prop={cond1 && cond2} /> was solved. But, dataOrUndefined && data... still an issue. IMO the rule should only complain if the left side is of type number.

[ljharb]

More specifically, it should only complain if the left side is a known type and is not a number.

This should also be an option, I'd think, because TS isn't actually "type safety" and folks may not want to rely on it.

[AhmedBaset]

More specifically, it should only complain if the left side is a known type and is not a number.

This should also be an option, I'd think, because TS isn't actually "type safety" and folks may not want to rely on it.

hmm. then, worth be an issue?

[ljharb]

Yes.

[AhmedBaset]

Yes.

Done in #3754

[yusufkinatas]

Seems like this issue is closed, but I think the code can be improved for checking if left side is boolean or not. Currently, the implementation only looks if the variable is assigned to a boolean in it's current scope. So it is not working if I am using a TS-typed component prop.

I think this can be improved by actually checking TS-type of the variable on the left-side.

Here is an example demonstrating the pain-point.

I checked the codebase a bit, but could not find an easy way to get it right by checking the TS type. I think we need to somehow get to what IDE shows as variable type when hovered.