eslint-plugin-react: False positive for react/jsx-no-leaked-render

This rules is sadly completely broken (at least in TS ?). I’m using it with { "validStrategies": ["ternary"] } and it complains about this perfectly valid code:

checked={selectedCount > 0 && selectedCount === items.length}

and wants to change it to

checked={selectedCount > 0 ? selectedCount === items.length : null}

which doesn’t make sense at all.

Another example:

// source
contentEditable={!disabled && editable}
// fixed
contentEditable={!disabled ? editable : null}

For me personally it would be enough if the rule would not apply to props. I acknowledge that there are valid cases where this rule should apply to props as well but IMO without looking at TS types this can result in a lot of false positives like my example above. The rule should target props that accept JSX only.

Actually I was quite surprised that the rule was also applied to props as I didn’t expect this from looking at the examples in the documentation. My suggestion would be to either remove this rule from props until it got a bit smarter or at least make it configurable if it should be applied to props or not.

Please let me know which of these 2 options would be preferred as a short-term solution and I’m happy to contribute that change. But for me, in its current state, this rule is unusable.

Oh I just found this plugin, which uses TypeScript: npmjs.com/package/eslint-plugin-jsx-expressions

Oh nice, looks like a nice plugin!

@hpersson since you’re the author of this, would you be interested in contributing TypeScript type checking also to react/jsx-no-leaked-render rule in eslint-plugin-react?

Yeah we can’t use this rule either. It’s not inferring bools. One of these days I’ll be on a project where I can afford to contribute back here. Thank you to the maintainers and contributors.

Oh I just found this plugin, which uses TypeScript: https://www.npmjs.com/package/eslint-plugin-jsx-expressions

We don’t want to forbid && entirely tho - many uses of it are perfectly fine.

This rule is unusable to me until PR #3441 is landed. I need to handle a very basic usecase of <Component isFooBar={isFoo && isBar}>. This rule currently auto-breaks that line by converting it into <Component isFooBar={isFoo ? isBar : null }>. My component doesn’t take null as a prop, nor should it be required to.

Has typescript awareness landed for this rule?

Yes, but a present undefined prop isn’t the same thing as an absent prop either.

In this specific case, if the prop value is foo && bar === 0 then the rule simply shouldn’t be checking it - so yes, I do agree that this rule should not be checking prop values that aren’t “children on an HTML element”.

@levrik IMO we should include an option in the rule to avoid reporting props other than children (enabled by default to avoid changing the default behavior I guess).

“null should not be used” is useless and antiquated advice that the larger JS ecosystem has resoundingly rejected, so let’s just throw that out the window right now.

A new option would be good. Props should not be validated by default.

Also the fix should not result to null in ternary operator but to undefined or it should be configurable. null according to Crockford should not be used: if you want a falsy value use undefined, because null does not work with default params and therefore completely changes the code flow.

I’m also getting a very incorrect error for this:

const DISPLAY = {
  WELCOME: 'WELCOME',
  FOO: 'FOO'
};

const Component = () => {
  const [display, setDisplay] = useState(DISPLAY.WELCOME);

  const test = !display || display === DISPLAY.WELCOME;

  console.log(typeof(test)); // logs 'boolean'
    
  return (
    <div>
      {(!display || display === DISPLAY.WELCOME) && (
         <span>my condition produces a linting error even though it is correctly interpreted as a boolean</span>
      )}
    </div>
  );
};

This could be even fixed by introducing a new option: reportProps. It could receive the following values:

• "always": as it is now
• "components": when the RHS is a component, as suggested
• "never": so no props are reported

Yes, I’m aligned with your suggestion. That should do the trick.

The RHS is an === comparison; it can’t ever be anything but a boolean. You don’t need to know any types to figure that part out.

Exactly, so since the RHS is something renderable, this rule should complain about it since we don’t know the type for the LHS (it could be a zero, so the rule reported it as expected).

I was suggesting to report props only when there is a && operator and the RHS is a component so we can assume you want to pass that prop to render something, rather than just use the value for logic purposes.

The RHS is an === comparison; it can’t ever be anything but a boolean. You don’t need to know any types to figure that part out.

The right side should be able to be any value that’s safe to render, which includes booleans.

Has this problem been resolved?

@ljharb I tested with version 7.31.8

Has typescript awareness landed for this rule?

@shamilovtim I’m afraid it didn’t, and I’m not really available to handle this for now.

@pke I see, I guess we can discuss on how to improve the rule reporting on props other than children. Not really sure what to do with it to be honest, apart from the option to decide between null or undefined.

@ljharb I’ll need way more time to try checking the TS types when available for the operators of the logical expression as discussed in this issue, since I’ve never done this before.

In the case of https://github.com/jsx-eslint/eslint-plugin-react/issues/3292#issuecomment-1132947856, with no type information, obviously both of those should warn.

Agreed. So what’s you recommendation?

As for now, I simply rewrote my code, adding a local isDisabled boolean. And since this makes the code clearer, I guess it’s a good solution.

@Robloche regarding the false positive reported: I think sometimes you would be interested into having this rule applied to props also, like:

<MyReactComponent
  header={someNumber && <Header />}
  isDisabled={foo && bar === 0}
/>

So I think the proper fix for this would be to make sure the right side of the && operator is not a condition.