jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.97k stars 2.77k forks source link

`jsx-no-leaked-render` should only complain if there is a certian leaked render #3754

Open AhmedBaset opened 4 months ago

AhmedBaset commented 4 months ago

Is there an existing issue for this?

Description Overview

This was previously discussed in #3719

The goal of the rule is to avoid leaked render specially when the left side of && is 0, NaN, or "" because react-dom will render a 0 in the first case (while some would expect to render nothing) and react-native will crach in the three. Currently, the rule complain for potential leaked render while most of the cases there is no any leaked render. for example, when the left side is boolean, undefined, null, ojbect, or array.

The following code is an eslint error, while it doesn't have any side effects.

let data: Item[] | undefined = getData();

return (
  <div>
     {data && <List items={data} />}
  </div>
)

Proposal

It should only complain if the left side is one of the potential leaked render number (0 or NaN) or string (""). Others types should be valid. AFAIK, this is possible with the @typescript-eslint/parser

eslint-plugin-react version

7.33.2

eslint version

8.56.0

node version

22

ljharb commented 4 months ago

Sounds like a great enhancement. It should be behind an option (perhaps the option disables this behavior so that the default is friendlier?). It should only kick in when the AST has type info and it knows for sure it’s not a number or string or bigint.

AhmedBaset commented 4 months ago

It should only kick in when the AST has type info and it knows for sure it’s not a number or string or bigint.

Does the plugin use TS type parsing at the moment?

ljharb commented 4 months ago

If the TS parser is used, and attaches type info to the AST, we can (and often do) use it.

yusufkinatas commented 4 months ago

I think this issue means making the rule recognize all 3 variables below are essentially boolean values.

Screenshot 2024-05-29 at 12 21 45

I guess similar to how VSCode tells the type of a variable when hovered?

I would love to help improving this rule, but pretty new to this repository. Do we have any examples of similar TS type parsing?

ljharb commented 4 months ago

@yusufkinatas we don't typically have type-specific stuff, so off the top of my head i'm not sure where to look. however, you could always do the brute force approach and add some test cases and some console.logs, and see what type info is available on the AST nodes :-)

Rel1cx commented 3 months ago

The detection accuracy of eslint-plugin-react-x/src/rules/no-leaked-conditional-rendering.ts is qualitatively improved by introducing type information.

ljharb commented 3 months ago

@Rel1cx it sounds like something worth upstreaming (i'm not sure why that project even exists tbh, if there's rules or enhancements in it that this project should have, please PR them in :-) )