refactor: pr review changes

jsx-eslint · May 25, 2024 · d1814c1 · d1814c1
1 parent 8aec646
commit d1814c1
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 80 deletions.
diff --git a/docs/rules/no-render-return-undefined.md b/docs/rules/no-render-return-undefined.md
@@ -4,17 +4,19 @@
 
 <!-- end auto-generated rule header -->
 
-> In React 18, components may render undefined, and React will render nothing to the DOM instead of throwing an error. However, accidentally rendering nothing in a component could still cause surprises. This rule will warn if the `return` statement in a React Component returns undefined.
-
-Issue: [#3020](https://github.com/jsx-eslint/eslint-plugin-react/issues/3020)
+> Starting in React 18, components may render undefined, and React will render nothing to the DOM instead of throwing an error. However, accidentally rendering nothing in a component could still cause surprises. This rule will warn if the `return` statement in a React Component returns `undefined`.
 
 ## Rule Details
 
-This rule will warn if the `return` statement in a React component returns undefined.
+This rule will warn if the `return` statement in a React component returns `undefined`.
 
 Examples of **incorrect** code for this rule:
 
 ```jsx
+function App() {}
+
+// OR
+
 function App() {
   return undefined;
 }

diff --git a/lib/rules/no-render-return-undefined.js b/lib/rules/no-render-return-undefined.js
@@ -12,14 +12,88 @@ const report = require('../util/report');
 const variableUtil = require('../util/variable');
 
 const messages = {
-  returnsUndefined: "Don't return undefined from react components",
+  returnsUndefined: "Don't return `undefined` from react components",
 };
 
+function getReturnValue(context, returnNode) {
+  const variables = variableUtil.variablesInScope(context);
+  const returnIdentifierName = returnNode && returnNode.name;
+  const returnIdentifierVar = variableUtil.getVariable(
+    variables,
+    returnIdentifierName
+  );
+
+  if (!returnNode) return undefined;
+
+  if (
+    returnIdentifierVar
+    && returnIdentifierVar.defs
+    && returnIdentifierVar.defs[0]
+  ) {
+    const value = returnIdentifierVar.defs[0].node.init;
+    if (
+      returnIdentifierVar.defs[0].node
+      && returnIdentifierVar.defs[0].node.type === 'VariableDeclarator'
+      && value === null
+    ) {
+      return undefined;
+    }
+    return value;
+  }
+
+  switch (returnNode.type) {
+    case 'LogicalExpression': {
+      return getReturnValue(context, returnNode.right);
+    }
+    case 'ConditionalExpression': {
+      const possibleReturnValue = [
+        getReturnValue(context, returnNode.consequent),
+        getReturnValue(context, returnNode.alternate),
+      ];
+      const returnsUndefined = possibleReturnValue.some((val) => typeof val === 'undefined');
+      if (returnsUndefined) return;
+      return possibleReturnValue;
+    }
+    case 'CallExpression': {
+      if (returnNode.callee.type === 'MemberExpression') {
+        const calleeObjName = returnNode.callee.object.name;
+        const calleePropertyName = returnNode.callee.property.name;
+        const calleeObjNode = variables.find((item) => item && item.name === calleeObjName);
+        const isCalleeObjArray = calleeObjNode.defs[0].node.init.type === 'ArrayExpression';
+        const isMapCall = isCalleeObjArray && calleePropertyName === 'map';
+        if (isMapCall) {
+          const mapCallback = returnNode.arguments[0];
+          const mapReturnStatement = mapCallback.body.type === 'BlockStatement'
+              && astUtil.findReturnStatement(returnNode.arguments[0]);
+          const mapReturnNode = (mapReturnStatement && mapReturnStatement.argument) || mapCallback.body;
+          // console.log('DEBUG', mapReturnNode);
+          return getReturnValue(context, mapReturnNode);
+        }
+      }
+      const calleeName = returnNode.callee.name;
+      const calleeNode = variables.find((item) => item && item.name === calleeName);
+      const calleeDefinitionNode = calleeNode && calleeNode.defs && calleeNode.defs[0] && calleeNode.defs[0].node;
+      const calleeReturnStatement = astUtil.findReturnStatement(calleeDefinitionNode);
+      const calleeReturnNode = (calleeReturnStatement && calleeReturnStatement.argument)
+        || (calleeDefinitionNode.init && calleeDefinitionNode.init.body);
+      return getReturnValue(context, calleeReturnNode);
+    }
+    case 'ArrayExpression': {
+      return returnNode.elements;
+    }
+    case 'JSXElement': {
+      return returnNode;
+    }
+    default:
+      return returnNode.value;
+  }
+}
+
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
   meta: {
     docs: {
-      description: 'Disallow returning undefined from react components',
+      description: 'Disallow returning `undefined` from react components',
       category: 'Best Practices',
       recommended: false,
       url: docsUrl('no-render-return-undefined'),
@@ -29,89 +103,18 @@ module.exports = {
   },
 
   create(context) {
-    function getReturnValue(returnNode) {
-      const variables = variableUtil.variablesInScope(context);
-      const returnIdentifierName = returnNode && returnNode.name;
-      const returnIdentifierVar = variableUtil.getVariable(
-        variables,
-        returnIdentifierName
-      );
-
-      if (!returnNode) return undefined;
-
-      if (
-        returnIdentifierVar
-        && returnIdentifierVar.defs
-        && returnIdentifierVar.defs[0]
-      ) {
-        const value = returnIdentifierVar.defs[0].node.init;
-        if (
-          returnIdentifierVar.defs[0].node
-          && returnIdentifierVar.defs[0].node.type === 'VariableDeclarator'
-          && value === null
-        ) {
-          return undefined;
-        }
-        return value;
-      }
-
-      switch (returnNode.type) {
-        case 'LogicalExpression': {
-          return getReturnValue(returnNode.right);
-        }
-        case 'ConditionalExpression': {
-          const possibleReturnValue = [getReturnValue(returnNode.consequent), getReturnValue(returnNode.alternate)];
-          const returnsUndefined = possibleReturnValue.some((val) => val === undefined);
-          if (returnsUndefined) return undefined;
-          return possibleReturnValue;
-        }
-        case 'CallExpression': {
-          if (returnNode.callee.type === 'MemberExpression') {
-            const calleeObjName = returnNode.callee.object.name;
-            const calleePropertyName = returnNode.callee.property.name;
-            const calleeObjNode = variables.find((item) => item && item.name === calleeObjName);
-            const isCalleeObjArray = calleeObjNode.defs[0].node.init.type === 'ArrayExpression';
-            const isMapCall = isCalleeObjArray && calleePropertyName === 'map';
-            if (isMapCall) {
-              const mapCallback = returnNode.arguments[0];
-              const mapReturnStatement = mapCallback.body.type === 'BlockStatement'
-                  && astUtil.findReturnStatement(returnNode.arguments[0]);
-              const mapReturnNode = (mapReturnStatement && mapReturnStatement.argument) || mapCallback.body;
-              // console.log('DEBUG', mapReturnNode);
-              return getReturnValue(mapReturnNode);
-            }
-          }
-          const calleeName = returnNode.callee.name;
-          const calleeNode = variables.find((item) => item && item.name === calleeName);
-          const calleeDefinitionNode = calleeNode && calleeNode.defs && calleeNode.defs[0] && calleeNode.defs[0].node;
-          const calleeReturnStatement = astUtil.findReturnStatement(calleeDefinitionNode);
-          const calleeReturnNode = (calleeReturnStatement && calleeReturnStatement.argument)
-            || (calleeDefinitionNode.init && calleeDefinitionNode.init.body);
-          return getReturnValue(calleeReturnNode);
-        }
-        case 'ArrayExpression': {
-          return returnNode.elements;
-        }
-        case 'JSXElement': {
-          return returnNode;
-        }
-        default:
-          return returnNode.value;
-      }
-    }
-
     const isReturningUndefined = (returnStatement) => {
       const returnNode = returnStatement && returnStatement.argument;
       const returnIdentifierName = returnNode && returnNode.name;
 
-      const returnIdentifierValue = getReturnValue(returnNode);
+      const returnIdentifierValue = getReturnValue(context, returnNode);
 
       const returnsArrayHavingUndefined = Array.isArray(returnIdentifierValue)
           && returnIdentifierValue.some((el) => el && el.type === 'Identifier' && el.name === 'undefined');
 
       return !returnStatement
           || returnIdentifierName === 'undefined'
-          || returnIdentifierValue === undefined
+          || typeof returnIdentifierValue === 'undefined'
           || (returnIdentifierValue && returnIdentifierValue.name === 'undefined')
           || returnsArrayHavingUndefined;
     };

diff --git a/package.json b/package.json
@@ -15,7 +15,7 @@
     "test": "npm run unit-test",
     "posttest": "aud --production",
     "type-check": "tsc",
-    "unit-test": "istanbul cover node_modules/mocha/bin/_mocha tests/lib/rules/no-render-return-undefined.js",
+    "unit-test": "istanbul cover node_modules/mocha/bin/_mocha tests/lib/**/*.js tests/util/**/*.js tests/index.js",
     "update:eslint-docs": "eslint-doc-generator"
   },
   "repository": {