Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: no-leaked-render won't accept explicit Boolean() coercion, only implicit coercion with !! #3811

Open
2 tasks done
xml opened this issue Aug 31, 2024 · 3 comments
Open
2 tasks done

Comments

@xml
Copy link

xml commented Aug 31, 2024

Is there an existing issue for this?

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

Description Overview

The goal of the rule - as I understand it - is to ensure that values used in a JSX expression are verifiably booleans, so that they don't throw. Makes sense. But, the only acknowledged method - in rule docs and seemingly in the code - of coercing a value to boolean is the use of !!. This use is extremely common, and many folks (including me) don't even think to use the more explicit method of casting a value to boolean with the Boolean() constructor.

But, if you're being super oriented toward readability, that "implicit" coercion takes an extra couple seconds to read and parse, and junior devs may not even understand it. So, there's even an integrated eslint rule no-implicit-coercion which forbids !!value and recommends Boolean(value) instead. And... we use that rule. (As part of Tim W. James' wonderful eslint-config).

So, I tried converting my statement to use !! to satisfy this rule... only to run into a conflict with that other one. So, I took the advice to "use Boolean(value) instead"... only to realize that this rule doesn't understand it as an acceptable way to ensure the value is boolean by the time it's evaluated.

Code:

This rule throws a warning from: {showDevtools && <ReactQueryDevtools position="right" />}

This rule is OK with: {!!showDevtools && <ReactQueryDevtools position="right" />}, but that triggers a warning from eslint's own no-implicit-coercion

So I tried: {Boolean(showDevtools) && <ReactQueryDevtools position="right" />}, but this rule doesn't understand that as a legitimate way to get it done. (And, it's also entirely absent from the docs, which suggests to me it's intentional.)

Expected Behavior

The rule should permit use of Boolean(value), not just use of !!value

eslint-plugin-react version

v7.35.0

eslint version

v8.57.0

node version

v22.7.0

@xml xml added the bug label Aug 31, 2024
@xml xml changed the title [Bug]: no-leaked-render won't accept explicit boolean coercion, only implicit coercion with !! [Bug]: no-leaked-render won't accept explicit Boolean() coercion, only implicit coercion with !! Aug 31, 2024
@ljharb
Copy link
Member

ljharb commented Aug 31, 2024

While i think the readability of !! is actually identical to Boolean(), and it’s also more robust since its syntax instead of API, and thus the much better choice - i agree that the rule should accept Boolean().

@xml
Copy link
Author

xml commented Sep 25, 2024

FWIW, I totally agree with you (even though my linter config does not). I am not trying to indict use of !!, just to suggest that this rule should accept both options as equally valid, since they both have the same effect.

@developer-bandi
Copy link
Contributor

developer-bandi commented Oct 11, 2024

@xml If I understand it correctly, what we need to do is to make sure that the rule doesn't cause an error in the code below:

{Boolean(showDevtools) && <ReactQueryDevtools position="right" />}

However, what can be confirmed from the current test code, documentation, and actual code is that there is no issue with the above code.

Test code and documentation

const Component = ({ elements }) => {
  return <div>{Boolean(elements.length) && <List elements={elements}/>}</div>
}

Even if you add Boolean, no issue occurs.

actual code

const isCoerceValidLeftSide = COERCE_VALID_LEFT_SIDE_EXPRESSIONS
   .some((validExpression) => validExpression === leftSide.type);
if (validStrategies.has(COERCE_STRATEGY)) {
    if (isCoerceValidLeftSide || getIsCoerceValidNestedLogicalExpression(leftSide)) {
      return;
    }
}

When coerce type, function call( like Boolean()) no makes error because isCoerceValidLeftSide is true

Therefore, it seems necessary to reproduce the phenomenon in which the issue is occurring. Could you please provide a little more information?

but one problem is that if you include only coerce in the validStrategies option and change it to the && operator, the code is automatically modified to !! instead of Boolean(). If this is a problem, it may need to add an additional option to that option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants