[Bug]: jsx-no-leaked-render causes code to be incorrect by removing parentheses

[dusanristic]

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

I'm encountering an issue with a code transformation performed by the jsx-no-leaked-renderer. Original condition: connection && (hasError || hasErrorUpdate). Converted condition: (!!connection && hasError) || hasErrorUpdate. These conditions are not identical and produce different results for various inputs. For example, it will return different values for false, any, and true (respectively). In the first case it will return false, in the second it will return true.

Despite a reported issue and a pull request aimed at fixing this problem over a year ago, I'm still experiencing the same issue. Am I missing something, or this is a bug?

Expected Behavior

I want to preserve original condition, or if it is converted, to be identical as original.

eslint-plugin-react version

v18.2.0

eslint version

v8.43.0

node version

v18.17.1

[ljharb]

any isn’t a value, so your comment is confusing.

You say you’re using v18 of eslint-plugin-react, but that looks more like a react version - what version of the plugin are you using?

[dusanristic]

When I said 'any,' I meant that it doesn't matter which value the second variable takes; it can be either true or false. This won't have an impact on the condition when the first value is false and the third one is true. Sorry for the confusion.

eslint-plugin-react is v7.32.2 🤦‍♂️

[ljharb]

That version should indeed have the fix. If the bug is indeed reproducible, then it needs a fix asap.

[dusanristic]

I'm not sure if it matters; this condition is passed as a prop to the element (below is original condition):

 <ErrorWidget show={connection && (hasError || hasErrorUpdate)}/>

I am reproducing this error in a few places, this is not the only one.

[ljharb]

what settings are you using in the rule? with validStrategies: ['coerce', 'ternary'] it outputs <ErrorWidget show={!!connection && hasError || hasErrorUpdate}/>

[dusanristic]

Hey, just to check whether I understood correctly: eslint is fixing the mentioned conditional for you as you attached in the comment above? It is changing from connection && (hasError || hasErrorUpdate) to !!connection && hasError || hasErrorUpdate? If yes, this confirms there is an error, as && has precedence over ||, and the problem is the same (it is just that the Prettier rule is adding parentheses for me so that operator's precedence is clearer).

As for settings, the settings I am using are: 'react/jsx-no-leaked-render': ['error', { validStrategies: ['coerce', 'ternary'] }]

[ljharb]

yes, that's correct, and no, && doesn't have precedence over || - it's identical precedence.

[ljharb]

However, that does change the semantics:

code	a truthy, b/c falsy	b truthy, a/c falsy	c truthy, a/b falsy
a && b \|\| c	a	c	a
a && (b \|\| c)	a	a	c

which makes it a seriously bad bug.

[dusanristic]

&& doesn't have precedence over || - it's identical precedence.

Judging by the MDN docs, && has precedence over ||.

Also, looking at the screenshot from the console, it seems that && has precedence. If they have equal precedence, the output value would be false instead of true.

Regarding the bug, do you happen to know when we can expect this bug to be fixed?

[ljharb]

As you can see with 1 || 2 && 0, the 1 is what "wins", and I can assure you as a former editor of the spec itself that they have equal precedence.

It will be fixed when I have time to look into it, or when someone sends a PR.

[NjegosPetrovic]

@ljharb Is this change live yet? How can I update to use the fixed rule?

[ljharb]

No, it’s not released yet.

[ljharb]

It's included in v7.34.1.