[Fix] jsx-no-leaked-render: avoid unnecessary negation operators an…

…d ternary branches deletion Closes #3297. Relates to #3292.
jsx-eslint · May 26, 2022 · e7fc22f · e7fc22f
1 parent c42b624
commit e7fc22f
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,11 +7,13 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 
 ### Fixed
 * [`display-name`]: fix false positive for HOF returning only nulls ([#3291][] @golopot)
+* [`jsx-no-leaked-render`]: avoid unnecessary negation operators and ternary branches deletion ([#3299][] @Belco90)
 
 ### Changed
 * [Docs] [`jsx-tag-spacing`]: rename option from [#3264][] ([#3294[] @ljharb)
 * [Docs] [`jsx-key`]: split the examples ([#3293][] @ioggstream)
 
+[#3299]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3299
 [#3294]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3294
 [#3293]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3293
 [#3291]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3291

diff --git a/lib/rules/jsx-no-leaked-render.js b/lib/rules/jsx-no-leaked-render.js
@@ -21,6 +21,7 @@ const COERCE_STRATEGY = 'coerce';
 const TERNARY_STRATEGY = 'ternary';
 const DEFAULT_VALID_STRATEGIES = [TERNARY_STRATEGY, COERCE_STRATEGY];
 const COERCE_VALID_LEFT_SIDE_EXPRESSIONS = ['UnaryExpression', 'BinaryExpression', 'CallExpression'];
+const TERNARY_INVALID_ALTERNATE_VALUES = [undefined, null, false];
 
 function trimLeftNode(node) {
   // Remove double unary expression (boolean coercion), so we avoid trimming valid negations
@@ -31,6 +32,14 @@ function trimLeftNode(node) {
   return node;
 }
 
+function getIsCoerceValidNestedLogicalExpression(node) {
+  if (node.type === 'LogicalExpression') {
+    return getIsCoerceValidNestedLogicalExpression(node.left) && getIsCoerceValidNestedLogicalExpression(node.right);
+  }
+
+  return COERCE_VALID_LEFT_SIDE_EXPRESSIONS.some((validExpression) => validExpression === node.type);
+}
+
 function ruleFixer(context, fixStrategy, fixer, reportedNode, leftNode, rightNode) {
   const sourceCode = context.getSourceCode();
   const rightSideText = sourceCode.getText(rightNode);
@@ -102,11 +111,12 @@ module.exports = {
       'JSXExpressionContainer > LogicalExpression[operator="&&"]'(node) {
         const leftSide = node.left;
 
-        if (
-          validStrategies.has(COERCE_STRATEGY)
-          && COERCE_VALID_LEFT_SIDE_EXPRESSIONS.some((validExpression) => validExpression === leftSide.type)
-        ) {
-          return;
+        const isCoerceValidLeftSide = COERCE_VALID_LEFT_SIDE_EXPRESSIONS
+          .some((validExpression) => validExpression === leftSide.type);
+        if (validStrategies.has(COERCE_STRATEGY)) {
+          if (isCoerceValidLeftSide || getIsCoerceValidNestedLogicalExpression(leftSide)) {
+            return;
+          }
         }
 
         report(context, messages.noPotentialLeakedRender, 'noPotentialLeakedRender', {
@@ -122,6 +132,11 @@ module.exports = {
           return;
         }
 
+        const isValidTernaryAlternate = TERNARY_INVALID_ALTERNATE_VALUES.indexOf(node.alternate.value) === -1;
+        if (isValidTernaryAlternate) {
+          return;
+        }
+
         report(context, messages.noPotentialLeakedRender, 'noPotentialLeakedRender', {
           node,
           fix(fixer) {

diff --git a/tests/lib/rules/jsx-no-leaked-render.js b/tests/lib/rules/jsx-no-leaked-render.js
@@ -93,36 +93,87 @@ ruleTester.run('jsx-no-leaked-render', rule, {
       `,
     },
     {
-      options: [{ validStrategies: ['ternary'] }],
       code: `
         const Component = ({ elements, count }) => {
           return <div>{count ? <List elements={elements}/> : null}</div>
         }
       `,
+      options: [{ validStrategies: ['ternary'] }],
     },
     {
-      options: [{ validStrategies: ['coerce'] }],
       code: `
         const Component = ({ elements, count }) => {
           return <div>{!!count && <List elements={elements}/>}</div>
         }
       `,
+      options: [{ validStrategies: ['coerce'] }],
     },
     {
-      options: [{ validStrategies: ['coerce', 'ternary'] }],
       code: `
         const Component = ({ elements, count }) => {
           return <div>{count ? <List elements={elements}/> : null}</div>
         }
       `,
+      options: [{ validStrategies: ['coerce', 'ternary'] }],
     },
     {
+      code: `
+        const Component = ({ elements, count }) => {
+          return <div>{!!count && <List elements={elements}/>}</div>
+        }
+      `,
       options: [{ validStrategies: ['coerce', 'ternary'] }],
+    },
+    {
       code: `
         const Component = ({ elements, count }) => {
           return <div>{!!count && <List elements={elements}/>}</div>
         }
       `,
+      options: [{ validStrategies: ['coerce'] }],
+    },
+
+    // Fixes for:
+    // - https://github.com/jsx-eslint/eslint-plugin-react/issues/3292
+    // - https://github.com/jsx-eslint/eslint-plugin-react/issues/3297
+    {
+      // It shouldn't delete valid alternate from ternary expressions when "coerce" is the only valid strategy
+      code: `
+        const Component = ({ elements, count }) => {
+          return (
+            <div>
+              <div> {direction ? (direction === "down" ? "▼" : "▲") : ""} </div>
+              <div>{ containerName.length > 0 ? "Loading several stuff" : "Loading" }</div>
+            </div>
+          )
+        }
+      `,
+      options: [{ validStrategies: ['coerce'] }],
+    },
+    {
+      // It shouldn't delete valid branches from ternary expressions when ["coerce", "ternary"] are only valid strategies
+      code: `
+        const Component = ({ elements, count }) => {
+          return <div>{direction ? (direction === "down" ? "▼" : "▲") : ""}</div>
+        }
+      `,
+      options: [{ validStrategies: ['coerce', 'ternary'] }],
+    },
+    {
+      // It shouldn't report nested logical expressions when "coerce" is the only valid strategy
+      code: `
+        const Component = ({ direction }) => {
+          return (
+            <div>
+              <div>{!!direction && direction === "down" && "▼"}</div>
+              <div>{direction === "down" && !!direction && "▼"}</div>
+              <div>{direction === "down" || !!direction && "▼"}</div>
+              <div>{(!display || display === DISPLAY.WELCOME) && <span>foo</span>}</div>
+            </div>
+          )
+        }
+      `,
+      options: [{ validStrategies: ['coerce'] }],
     },
   ]),