From 499e2611e97890fd05546728d3dc35c2dcab6314 Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Tue, 7 May 2024 07:00:11 +0530 Subject: [PATCH 01/12] [Feat] no-render-return-undefined: initital commit --- lib/rules/no-render-return-undefined.js | 99 +++++++++++ package.json | 2 +- tests/lib/rules/no-render-return-undefined.js | 158 ++++++++++++++++++ 3 files changed, 258 insertions(+), 1 deletion(-) create mode 100644 lib/rules/no-render-return-undefined.js create mode 100644 tests/lib/rules/no-render-return-undefined.js diff --git a/lib/rules/no-render-return-undefined.js b/lib/rules/no-render-return-undefined.js new file mode 100644 index 0000000000..262455afe6 --- /dev/null +++ b/lib/rules/no-render-return-undefined.js @@ -0,0 +1,99 @@ +/** + * @fileoverview Prevent returning undefined from react components + * @author Akul Srivastava + */ + +'use strict'; + +const astUtil = require('../util/ast'); +const docsUrl = require('../util/docsUrl'); +const report = require('../util/report'); +const variableUtil = require('../util/variable'); + +const messages = { + returnsUndefined: "Don't return undefined from react components", +}; + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + docs: { + description: 'Disallow returning undefined from react components', + category: 'Best Practices', + recommended: false, + url: docsUrl('no-render-return-undefined'), + }, + messages, + schema: [], + }, + + create(context) { + return { + FunctionDeclaration(node) { + const fnName = node.id.name; + const isReactComponent = fnName[0] === fnName[0].toUpperCase(); + const returnStatement = astUtil.findReturnStatement(node); + + if (!isReactComponent) return; + + const variables = variableUtil.variablesInScope(context); + const returnNode = returnStatement && returnStatement.argument; + const returnIdentifierName = returnNode && returnNode.name; + const returnIdentifierVar = variableUtil.getVariable( + variables, + returnIdentifierName + ); + const returnIdentifierValue = (() => { + if (!returnNode) return undefined; + if ( + returnIdentifierVar + && returnIdentifierVar.defs + && returnIdentifierVar.defs[0] + ) { + const value = returnIdentifierVar.defs[0].node.init; + if ( + returnIdentifierVar.defs[0].node.type === 'VariableDeclarator' + && value === null + ) { + return undefined; + } + return value; + } + + if (returnNode.type === 'ArrayExpression') { + return returnNode.elements; + } + + if (returnNode.type === 'JSXElement') { + return returnNode; + } + + return returnNode.value; + })(); + + // console.log('DEBUG', returnIdentifierValue); + + const returnsArrayHavingUndefined = Array.isArray(returnIdentifierValue) + && returnIdentifierValue.some((el) => el.type === 'Identifier' && el.name === 'undefined'); + + if ( + !returnStatement + || returnIdentifierName === 'undefined' + || returnIdentifierValue === undefined + || (returnIdentifierValue && returnIdentifierValue.name === 'undefined') + || returnsArrayHavingUndefined + ) { + report(context, messages.returnsUndefined, 'returnsUndefined', { + node, + }); + } + }, + // FunctionExpression(node) { + // // console.log('DEBUG fn expression', node); + // }, + // ArrowFunctionExpression(node) { + // // console.log('DEBUG fn arrow function', node); + // }, + }; + }, +}; diff --git a/package.json b/package.json index 685e65a222..7b88133c93 100644 --- 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/**/*.js tests/util/**/*.js tests/index.js", + "unit-test": "istanbul cover node_modules/mocha/bin/_mocha tests/lib/rules/no-render-return-undefined.js", "update:eslint-docs": "eslint-doc-generator" }, "repository": { diff --git a/tests/lib/rules/no-render-return-undefined.js b/tests/lib/rules/no-render-return-undefined.js new file mode 100644 index 0000000000..664a3d412f --- /dev/null +++ b/tests/lib/rules/no-render-return-undefined.js @@ -0,0 +1,158 @@ +/** + * @fileoverview Tests for no-danger + * @author Scott Andrews + */ + +'use strict'; + +// ----------------------------------------------------------------------------- +// Requirements +// ----------------------------------------------------------------------------- + +const RuleTester = require('eslint').RuleTester; +const rule = require('../../../lib/rules/no-render-return-undefined'); + +const parsers = require('../../helpers/parsers'); + +const parserOptions = { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + jsx: true, + }, +}; + +// ----------------------------------------------------------------------------- +// Tests +// ----------------------------------------------------------------------------- + +const ruleTester = new RuleTester({ parserOptions }); +ruleTester.run('no-render-return-undefined', rule, { + valid: parsers.all([ + { + code: ` + function App() { + return 123; + } + `, + }, + { + code: ` + function App() { + return 'Hello World'; + } + `, + }, + { + code: ` + function App() { + return null; + } + `, + }, + { + code: ` + function App() { + return []; + } + `, + }, + { + code: ` + function App() { + return
; + } + `, + }, + { + code: ` + function App() { + return
; + } + `, + }, + { + code: ` + function App() { + return
Hello World
; + } + `, + }, + { + code: ` + function App() { + return ; + } + `, + }, + ]), + invalid: parsers.all([ + { + code: ` + function App() {} + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + return undefined; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + const toReturn = undefined; + return toReturn; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + var toReturn; + return toReturn; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + let toReturn; + return toReturn; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + var foo; + function App() { + return foo; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + let foo; + function App() { + return foo; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + return [undefined]; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + ]), +}); From 86d7ac8a80f65874408ca7fb42dd3af14ac1b2dd Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Wed, 8 May 2024 14:27:48 +0530 Subject: [PATCH 02/12] chore: add more test cases --- lib/rules/no-render-return-undefined.js | 105 +++++++++--------- tests/lib/rules/no-render-return-undefined.js | 87 ++++++++++++++- 2 files changed, 134 insertions(+), 58 deletions(-) diff --git a/lib/rules/no-render-return-undefined.js b/lib/rules/no-render-return-undefined.js index 262455afe6..7f845cb691 100644 --- a/lib/rules/no-render-return-undefined.js +++ b/lib/rules/no-render-return-undefined.js @@ -28,72 +28,67 @@ module.exports = { }, create(context) { - return { - FunctionDeclaration(node) { - const fnName = node.id.name; - const isReactComponent = fnName[0] === fnName[0].toUpperCase(); - const returnStatement = astUtil.findReturnStatement(node); + const handleFunctionalComponents = (node) => { + const fnName = (node.id && node.id.name) || node.parent.id.name; + const isReactComponent = fnName[0] === fnName[0].toUpperCase(); + const returnStatement = astUtil.findReturnStatement(node); - if (!isReactComponent) return; + if (!isReactComponent) return; - const variables = variableUtil.variablesInScope(context); - const returnNode = returnStatement && returnStatement.argument; - const returnIdentifierName = returnNode && returnNode.name; - const returnIdentifierVar = variableUtil.getVariable( - variables, - returnIdentifierName - ); - const returnIdentifierValue = (() => { - if (!returnNode) return undefined; + const variables = variableUtil.variablesInScope(context); + const returnNode = returnStatement && returnStatement.argument; + const returnIdentifierName = returnNode && returnNode.name; + const returnIdentifierVar = variableUtil.getVariable( + variables, + returnIdentifierName + ); + const returnIdentifierValue = (() => { + if (!returnNode) return undefined; + if ( + returnIdentifierVar + && returnIdentifierVar.defs + && returnIdentifierVar.defs[0] + ) { + const value = returnIdentifierVar.defs[0].node.init; if ( - returnIdentifierVar - && returnIdentifierVar.defs - && returnIdentifierVar.defs[0] + returnIdentifierVar.defs[0].node.type === 'VariableDeclarator' + && value === null ) { - const value = returnIdentifierVar.defs[0].node.init; - if ( - returnIdentifierVar.defs[0].node.type === 'VariableDeclarator' - && value === null - ) { - return undefined; - } - return value; + return undefined; } + return value; + } - if (returnNode.type === 'ArrayExpression') { - return returnNode.elements; - } + if (returnNode.type === 'ArrayExpression') { + return returnNode.elements; + } - if (returnNode.type === 'JSXElement') { - return returnNode; - } + if (returnNode.type === 'JSXElement') { + return returnNode; + } - return returnNode.value; - })(); + return returnNode.value; + })(); - // console.log('DEBUG', returnIdentifierValue); + const returnsArrayHavingUndefined = Array.isArray(returnIdentifierValue) + && returnIdentifierValue.some((el) => el.type === 'Identifier' && el.name === 'undefined'); - const returnsArrayHavingUndefined = Array.isArray(returnIdentifierValue) - && returnIdentifierValue.some((el) => el.type === 'Identifier' && el.name === 'undefined'); + if ( + !returnStatement + || returnIdentifierName === 'undefined' + || returnIdentifierValue === undefined + || (returnIdentifierValue && returnIdentifierValue.name === 'undefined') + || returnsArrayHavingUndefined + ) { + report(context, messages.returnsUndefined, 'returnsUndefined', { + node, + }); + } + }; - if ( - !returnStatement - || returnIdentifierName === 'undefined' - || returnIdentifierValue === undefined - || (returnIdentifierValue && returnIdentifierValue.name === 'undefined') - || returnsArrayHavingUndefined - ) { - report(context, messages.returnsUndefined, 'returnsUndefined', { - node, - }); - } - }, - // FunctionExpression(node) { - // // console.log('DEBUG fn expression', node); - // }, - // ArrowFunctionExpression(node) { - // // console.log('DEBUG fn arrow function', node); - // }, + return { + FunctionDeclaration: handleFunctionalComponents, + ArrowFunctionExpression: handleFunctionalComponents, }; }, }; diff --git a/tests/lib/rules/no-render-return-undefined.js b/tests/lib/rules/no-render-return-undefined.js index 664a3d412f..9ac76f55f2 100644 --- a/tests/lib/rules/no-render-return-undefined.js +++ b/tests/lib/rules/no-render-return-undefined.js @@ -36,6 +36,13 @@ ruleTester.run('no-render-return-undefined', rule, { } `, }, + { + code: ` + const App = () => { + return 123; + } + `, + }, { code: ` function App() { @@ -85,6 +92,32 @@ ruleTester.run('no-render-return-undefined', rule, { } `, }, + { + code: ` + function App() { + return ; + } + `, + }, + { + code: ` + function App() { + return ( + + + + ); + } + `, + }, + { + code: ` + const ui = ; + function App() { + return ui; + } + `, + }, ]), invalid: parsers.all([ { @@ -93,6 +126,28 @@ ruleTester.run('no-render-return-undefined', rule, { `, errors: [{ messageId: 'returnsUndefined' }], }, + { + code: ` + const App = () => {} + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + return; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + const App = () => { + return; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, { code: ` function App() { @@ -101,6 +156,14 @@ ruleTester.run('no-render-return-undefined', rule, { `, errors: [{ messageId: 'returnsUndefined' }], }, + { + code: ` + const App = () => { + return undefined; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, { code: ` function App() { @@ -148,9 +211,27 @@ ruleTester.run('no-render-return-undefined', rule, { }, { code: ` - function App() { - return [undefined]; - } + function App() { + return [undefined]; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + return [undefined, undefined]; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function foo() {} + + function App() { + return foo(); + } `, errors: [{ messageId: 'returnsUndefined' }], }, From 61981c7bce68bebe3a5110f23a868da137be6ec9 Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Wed, 8 May 2024 16:43:12 +0530 Subject: [PATCH 03/12] chore: add test cases --- tests/lib/rules/no-render-return-undefined.js | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/lib/rules/no-render-return-undefined.js b/tests/lib/rules/no-render-return-undefined.js index 9ac76f55f2..2a8bf15ab9 100644 --- a/tests/lib/rules/no-render-return-undefined.js +++ b/tests/lib/rules/no-render-return-undefined.js @@ -1,6 +1,6 @@ /** * @fileoverview Tests for no-danger - * @author Scott Andrews + * @author Akul Srivastava */ 'use strict'; @@ -118,6 +118,13 @@ ruleTester.run('no-render-return-undefined', rule, { } `, }, + { + code: ` + function App() { + return [
, ]; + } + `, + }, ]), invalid: parsers.all([ { @@ -235,5 +242,49 @@ ruleTester.run('no-render-return-undefined', rule, { `, errors: [{ messageId: 'returnsUndefined' }], }, + { + code: ` + function App() { + return +
+ } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + return + [] + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + return + 123 + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + return + "abc" + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function App() { + return;
; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, ]), }); From 34b6a941598ec72c22abd97f1c438d61eb07cb5b Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Wed, 15 May 2024 12:35:22 +0530 Subject: [PATCH 04/12] chore: add tc for class components --- lib/rules/no-render-return-undefined.js | 46 ++-- tests/lib/rules/no-render-return-undefined.js | 230 +++++++++++++++++- 2 files changed, 261 insertions(+), 15 deletions(-) diff --git a/lib/rules/no-render-return-undefined.js b/lib/rules/no-render-return-undefined.js index 7f845cb691..72ba1c965f 100644 --- a/lib/rules/no-render-return-undefined.js +++ b/lib/rules/no-render-return-undefined.js @@ -28,13 +28,7 @@ module.exports = { }, create(context) { - const handleFunctionalComponents = (node) => { - const fnName = (node.id && node.id.name) || node.parent.id.name; - const isReactComponent = fnName[0] === fnName[0].toUpperCase(); - const returnStatement = astUtil.findReturnStatement(node); - - if (!isReactComponent) return; - + const isReturningUndefined = (returnStatement) => { const variables = variableUtil.variablesInScope(context); const returnNode = returnStatement && returnStatement.argument; const returnIdentifierName = returnNode && returnNode.name; @@ -73,13 +67,35 @@ module.exports = { const returnsArrayHavingUndefined = Array.isArray(returnIdentifierValue) && returnIdentifierValue.some((el) => el.type === 'Identifier' && el.name === 'undefined'); - if ( - !returnStatement - || returnIdentifierName === 'undefined' - || returnIdentifierValue === undefined - || (returnIdentifierValue && returnIdentifierValue.name === 'undefined') - || returnsArrayHavingUndefined - ) { + return !returnStatement + || returnIdentifierName === 'undefined' + || returnIdentifierValue === undefined + || (returnIdentifierValue && returnIdentifierValue.name === 'undefined') + || returnsArrayHavingUndefined; + }; + + const handleFunctionalComponents = (node) => { + const fnName = (node.id && node.id.name) || node.parent.id.name; + + // Considering functions starting with Uppercase letters are react components + const isReactComponent = fnName[0] === fnName[0].toUpperCase(); + const returnStatement = astUtil.findReturnStatement(node); + + if (!isReactComponent) return; + + if (isReturningUndefined(returnStatement)) { + report(context, messages.returnsUndefined, 'returnsUndefined', { + node, + }); + } + }; + + const handleClassComponents = (node) => { + const componentProperties = astUtil.getComponentProperties(node); + const renderFnNode = componentProperties.find((prop) => prop.key.name === 'render'); + const returnStatement = astUtil.findReturnStatement(renderFnNode); + + if (isReturningUndefined(returnStatement)) { report(context, messages.returnsUndefined, 'returnsUndefined', { node, }); @@ -89,6 +105,8 @@ module.exports = { return { FunctionDeclaration: handleFunctionalComponents, ArrowFunctionExpression: handleFunctionalComponents, + ClassDeclaration: handleClassComponents, + ClassExpression: handleClassComponents, }; }, }; diff --git a/tests/lib/rules/no-render-return-undefined.js b/tests/lib/rules/no-render-return-undefined.js index 2a8bf15ab9..896d4761e2 100644 --- a/tests/lib/rules/no-render-return-undefined.js +++ b/tests/lib/rules/no-render-return-undefined.js @@ -28,6 +28,7 @@ const parserOptions = { const ruleTester = new RuleTester({ parserOptions }); ruleTester.run('no-render-return-undefined', rule, { + // Valid Cases valid: parsers.all([ { code: ` @@ -125,7 +126,128 @@ ruleTester.run('no-render-return-undefined', rule, { } `, }, + + // Class Components + { + code: ` + class App { + render() { + return 1; + } + } + `, + }, + { + code: ` + const App = class { + render() { + return 1; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return 1; + } + } + `, + }, + { + code: ` + class App { + render() { + return "Hello World"; + } + } + `, + }, + { + code: ` + const App = class { + render() { + return "Hello World"; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return "Hello World"; + } + } + `, + }, + { + code: ` + class App { + render() { + return
; + } + } + `, + }, + { + code: ` + const App = class { + render() { + return
; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return
; + } + } + `, + }, + { + code: ` + class App { + render() { + return [1, "Hello",
]; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return [1, "Hello",
]; + } + } + `, + }, + { + code: ` + class App { + render() { + return null; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return null; + } + } + `, + }, ]), + + // Invalid Cases invalid: parsers.all([ { code: ` @@ -235,7 +357,6 @@ ruleTester.run('no-render-return-undefined', rule, { { code: ` function foo() {} - function App() { return foo(); } @@ -286,5 +407,112 @@ ruleTester.run('no-render-return-undefined', rule, { `, errors: [{ messageId: 'returnsUndefined' }], }, + // Class Components + { + code: ` + class App { + render() { + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App extends React.Component { + render() { + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + const App = class { + render() {} + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App { + render() { + return; + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App extends React.Component { + render() { + return; + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App { + render() { + return undefined; + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App extends React.Component { + render() { + return undefined; + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App { + render() { + return [undefined]; + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App extends React.Component { + render() { + return [undefined]; + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App { + render() { + return [1, undefined]; + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + class App extends React.Component { + render() { + return [1, undefined]; + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, ]), }); From 56e7436906be1fc772509765dfb811fcb7aa5400 Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Wed, 15 May 2024 14:32:14 +0530 Subject: [PATCH 05/12] chore: handle returning call expressions --- lib/rules/no-render-return-undefined.js | 72 ++++--- tests/lib/rules/no-render-return-undefined.js | 186 +++++++++++++++++- 2 files changed, 230 insertions(+), 28 deletions(-) diff --git a/lib/rules/no-render-return-undefined.js b/lib/rules/no-render-return-undefined.js index 72ba1c965f..5e8e89809b 100644 --- a/lib/rules/no-render-return-undefined.js +++ b/lib/rules/no-render-return-undefined.js @@ -28,44 +28,68 @@ module.exports = { }, create(context) { - const isReturningUndefined = (returnStatement) => { + function getReturnValue(returnNode) { const variables = variableUtil.variablesInScope(context); - const returnNode = returnStatement && returnStatement.argument; const returnIdentifierName = returnNode && returnNode.name; const returnIdentifierVar = variableUtil.getVariable( variables, returnIdentifierName ); - const returnIdentifierValue = (() => { - if (!returnNode) return undefined; + + if (!returnNode) return undefined; + + if ( + returnIdentifierVar + && returnIdentifierVar.defs + && returnIdentifierVar.defs[0] + ) { + const value = returnIdentifierVar.defs[0].node.init; if ( - returnIdentifierVar - && returnIdentifierVar.defs - && returnIdentifierVar.defs[0] + returnIdentifierVar.defs[0].node + && returnIdentifierVar.defs[0].node.type === 'VariableDeclarator' + && value === null ) { - const value = returnIdentifierVar.defs[0].node.init; - if ( - returnIdentifierVar.defs[0].node.type === 'VariableDeclarator' - && value === null - ) { - return undefined; - } - return value; + return undefined; } + return value; + } - if (returnNode.type === 'ArrayExpression') { - return returnNode.elements; - } + if (returnNode.type === 'ConditionalExpression') { + const possibleReturnValue = [getReturnValue(returnNode.consequent), getReturnValue(returnNode.alternate)]; + const returnsUndefined = possibleReturnValue.some((val) => val === undefined); + if (returnsUndefined) return undefined; + return possibleReturnValue; + } - if (returnNode.type === 'JSXElement') { - return returnNode; - } + if (returnNode.type === 'CallExpression') { + const calleeName = returnNode.callee.name; + const calleeNode = variableUtil.variablesInScope(context).find((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); + } + + if (returnNode.type === 'ArrayExpression') { + return returnNode.elements; + } + + if (returnNode.type === 'JSXElement') { + return returnNode; + } + + return returnNode.value; + } + + const isReturningUndefined = (returnStatement) => { + const returnNode = returnStatement && returnStatement.argument; + const returnIdentifierName = returnNode && returnNode.name; - return returnNode.value; - })(); + const returnIdentifierValue = getReturnValue(returnNode); const returnsArrayHavingUndefined = Array.isArray(returnIdentifierValue) - && returnIdentifierValue.some((el) => el.type === 'Identifier' && el.name === 'undefined'); + && returnIdentifierValue.some((el) => el && el.type === 'Identifier' && el.name === 'undefined'); return !returnStatement || returnIdentifierName === 'undefined' diff --git a/tests/lib/rules/no-render-return-undefined.js b/tests/lib/rules/no-render-return-undefined.js index 896d4761e2..fa95c2b4ec 100644 --- a/tests/lib/rules/no-render-return-undefined.js +++ b/tests/lib/rules/no-render-return-undefined.js @@ -126,6 +126,112 @@ ruleTester.run('no-render-return-undefined', rule, { } `, }, + { + code: ` + function App() { + function getUI() { + return 1; + } + return getUI(); + } + `, + }, + { + code: ` + function getFoo() { + return 1; + } + + function App() { + function getUI() { + return getFoo(); + } + return getUI(); + } + `, + }, + { + code: ` + const getFoo = () => 1; + + function App() { + function getUI() { + return getFoo(); + } + return getUI(); + } + `, + }, + { + code: ` + function getFoo() { + return 1; + }; + + function App() { + function getUI() { + return getFoo(); + } + return getUI(); + } + `, + }, + { + code: ` + function getA() { + return ; + }; + function getB() { + return ; + }; + + function App() { + function getUI() { + return condition ? getA() : getB(); + } + return getUI(); + } + `, + }, + { + code: ` + const getA = () => ; + const getB = () => ; + + function App() { + function getUI() { + return condition ? getA() : getB(); + } + return getUI(); + } + `, + }, + { + code: ` + const getNum = () => 123; + const getString = () => "ABC"; + + function App() { + function getUI() { + return condition ? getNum() : getString(); + } + return getUI(); + } + `, + }, + { + code: ` + const getA = () => null; + const getB = () => [12, "Hello"]; + + function App() { + function getUI() { + return condition ? getA() : getB(); + } + return getUI(); + } + `, + }, // Class Components { @@ -143,7 +249,7 @@ ruleTester.run('no-render-return-undefined', rule, { render() { return 1; } - } + } `, }, { @@ -170,7 +276,7 @@ ruleTester.run('no-render-return-undefined', rule, { render() { return "Hello World"; } - } + } `, }, { @@ -197,7 +303,7 @@ ruleTester.run('no-render-return-undefined', rule, { render() { return
; } - } + } `, }, { @@ -407,6 +513,78 @@ ruleTester.run('no-render-return-undefined', rule, { `, errors: [{ messageId: 'returnsUndefined' }], }, + { + code: ` + function App() { + function getUI() { + return undefined; + } + return getUI(); + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function getFoo() { + return undefined; + } + + function App() { + function getUI() { + return getFoo(); + } + return getUI(); + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + const getFoo = () => undefined; + + function App() { + function getUI() { + return getFoo(); + } + return getUI(); + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function getA() { + return undefined; + }; + function getB() { + return ; + }; + + function App() { + function getUI() { + return condition ? getA() : getB(); + } + return getUI(); + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + const getA = () => undefined; + const getB = () => ; + + function App() { + function getUI() { + return condition ? getA() : getB(); + } + return getUI(); + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + // Class Components { code: ` @@ -430,7 +608,7 @@ ruleTester.run('no-render-return-undefined', rule, { code: ` const App = class { render() {} - } + } `, errors: [{ messageId: 'returnsUndefined' }], }, From 9202a3036aebfb4a265f106ab6be93c16627a148 Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Wed, 15 May 2024 17:29:11 +0530 Subject: [PATCH 06/12] chore: handle returning logical expressions --- lib/rules/no-render-return-undefined.js | 55 +-- tests/lib/rules/no-render-return-undefined.js | 340 ++++++++++++------ 2 files changed, 250 insertions(+), 145 deletions(-) diff --git a/lib/rules/no-render-return-undefined.js b/lib/rules/no-render-return-undefined.js index 5e8e89809b..6c75885f5a 100644 --- a/lib/rules/no-render-return-undefined.js +++ b/lib/rules/no-render-return-undefined.js @@ -7,6 +7,7 @@ const astUtil = require('../util/ast'); const docsUrl = require('../util/docsUrl'); +const isFirstLetterCapitalized = require('../util/isFirstLetterCapitalized'); const report = require('../util/report'); const variableUtil = require('../util/variable'); @@ -54,32 +55,34 @@ module.exports = { return value; } - if (returnNode.type === 'ConditionalExpression') { - const possibleReturnValue = [getReturnValue(returnNode.consequent), getReturnValue(returnNode.alternate)]; - const returnsUndefined = possibleReturnValue.some((val) => val === undefined); - if (returnsUndefined) return undefined; - return possibleReturnValue; - } - - if (returnNode.type === 'CallExpression') { - const calleeName = returnNode.callee.name; - const calleeNode = variableUtil.variablesInScope(context).find((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); - } - - if (returnNode.type === 'ArrayExpression') { - return returnNode.elements; - } - - if (returnNode.type === 'JSXElement') { - return returnNode; + 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': { + const calleeName = returnNode.callee.name; + const calleeNode = variableUtil.variablesInScope(context).find((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; } - - return returnNode.value; } const isReturningUndefined = (returnStatement) => { @@ -102,7 +105,7 @@ module.exports = { const fnName = (node.id && node.id.name) || node.parent.id.name; // Considering functions starting with Uppercase letters are react components - const isReactComponent = fnName[0] === fnName[0].toUpperCase(); + const isReactComponent = isFirstLetterCapitalized(fnName); const returnStatement = astUtil.findReturnStatement(node); if (!isReactComponent) return; diff --git a/tests/lib/rules/no-render-return-undefined.js b/tests/lib/rules/no-render-return-undefined.js index fa95c2b4ec..28ab7e04d5 100644 --- a/tests/lib/rules/no-render-return-undefined.js +++ b/tests/lib/rules/no-render-return-undefined.js @@ -30,6 +30,138 @@ const ruleTester = new RuleTester({ parserOptions }); ruleTester.run('no-render-return-undefined', rule, { // Valid Cases valid: parsers.all([ + // NonComponents + { + code: ` + function getUser() { + return undefined; + } + `, + }, + { + code: 'function doSomething() {}', + }, + + // Class Components + { + code: ` + class App { + render() { + return 1; + } + } + `, + }, + { + code: ` + const App = class { + render() { + return 1; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return 1; + } + } + `, + }, + { + code: ` + class App { + render() { + return "Hello World"; + } + } + `, + }, + { + code: ` + const App = class { + render() { + return "Hello World"; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return "Hello World"; + } + } + `, + }, + { + code: ` + class App { + render() { + return
; + } + } + `, + }, + { + code: ` + const App = class { + render() { + return
; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return
; + } + } + `, + }, + { + code: ` + class App { + render() { + return [1, "Hello",
]; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return [1, "Hello",
]; + } + } + `, + }, + { + code: ` + class App { + render() { + return null; + } + } + `, + }, + { + code: ` + class App extends React.Component { + render() { + return null; + } + } + `, + }, + + // Functional Components { code: ` function App() { @@ -232,129 +364,153 @@ ruleTester.run('no-render-return-undefined', rule, { } `, }, - - // Class Components { code: ` - class App { - render() { - return 1; - } - } - `, + function App() { + return condition && ; + } + `, }, { code: ` - const App = class { - render() { - return 1; - } - } - `, + function getFoo() { + return ; + } + function App() { + return condition && getFoo(); + } + `, }, { code: ` - class App extends React.Component { + function App() { + return condition || ; + } + `, + }, + { + code: ` + function getFoo() { + return ; + } + function App() { + return condition || getFoo(); + } + `, + }, + ]), + + // Invalid Cases + invalid: parsers.all([ + // Class Components + { + code: ` + class App { render() { - return 1; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - class App { + class App extends React.Component { render() { - return "Hello World"; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` const App = class { - render() { - return "Hello World"; - } + render() {} } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - class App extends React.Component { + class App { render() { - return "Hello World"; + return; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - class App { + class App extends React.Component { render() { - return
; + return; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - const App = class { + class App { render() { - return
; + return undefined; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` class App extends React.Component { render() { - return
; + return undefined; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` class App { render() { - return [1, "Hello",
]; + return [undefined]; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` class App extends React.Component { render() { - return [1, "Hello",
]; + return [undefined]; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` class App { render() { - return null; + return [1, undefined]; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, { code: ` class App extends React.Component { render() { - return null; + return [1, undefined]; } } `, + errors: [{ messageId: 'returnsUndefined' }], }, - ]), - // Invalid Cases - invalid: parsers.all([ + // Functional Components { code: ` function App() {} @@ -584,112 +740,58 @@ ruleTester.run('no-render-return-undefined', rule, { `, errors: [{ messageId: 'returnsUndefined' }], }, - - // Class Components - { - code: ` - class App { - render() { - } - } - `, - errors: [{ messageId: 'returnsUndefined' }], - }, - { - code: ` - class App extends React.Component { - render() { - } - } - `, - errors: [{ messageId: 'returnsUndefined' }], - }, - { - code: ` - const App = class { - render() {} - } - `, - errors: [{ messageId: 'returnsUndefined' }], - }, { code: ` - class App { - render() { - return; - } - } - `, - errors: [{ messageId: 'returnsUndefined' }], - }, - { - code: ` - class App extends React.Component { - render() { - return; - } - } - `, - errors: [{ messageId: 'returnsUndefined' }], - }, - { - code: ` - class App { - render() { - return undefined; - } - } - `, + function App() { + return condition && undefined; + } + `, errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - class App extends React.Component { - render() { - return undefined; - } - } - `, + function App() { + return condition || undefined; + } + `, errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - class App { - render() { - return [undefined]; - } - } - `, + function App() { + return condition && [
, undefined]; + } + `, errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - class App extends React.Component { - render() { - return [undefined]; - } - } - `, + function App() { + return condition || [
, undefined]; + } + `, errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - class App { - render() { - return [1, undefined]; - } - } - `, + function getFoo() { + return undefined; + } + function App() { + return condition && getFoo(); + } + `, errors: [{ messageId: 'returnsUndefined' }], }, { code: ` - class App extends React.Component { - render() { - return [1, undefined]; - } - } - `, + function getFoo() { + return undefined; + } + function App() { + return condition || getFoo(); + } + `, errors: [{ messageId: 'returnsUndefined' }], }, ]), From 639f15fde5313fc9f4d7977730f2c4bf9278a612 Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Thu, 16 May 2024 16:42:26 +0530 Subject: [PATCH 07/12] docs: add rule documentation --- docs/rules/no-render-return-undefined.md | 62 +++++++++++++++++++ tests/lib/rules/no-render-return-undefined.js | 51 +++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 docs/rules/no-render-return-undefined.md diff --git a/docs/rules/no-render-return-undefined.md b/docs/rules/no-render-return-undefined.md new file mode 100644 index 0000000000..a80cdf5666 --- /dev/null +++ b/docs/rules/no-render-return-undefined.md @@ -0,0 +1,62 @@ +# Disallow returning undefined from react components (`react/no-render-return-undefined`) + +💼 This rule is enabled in the ☑️ `recommended` [config](https://github.com/jsx-eslint/eslint-plugin-react/#shareable-configs). + + + +> 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) + +## Rule Details + +This rule will warn if the `return` statement in a React component returns undefined. + +Examples of **incorrect** code for this rule: + +```jsx +function App() { + return undefined; +} + +// OR + +let ui; +function App() { + return ui; +} + +// OR + +function getUI() { + if (condition) return

Hello

; +} +function App() { + return getUI(); +} +``` + +Examples of **correct** code for this rule: + +```jsx +function App() { + return
; +} + +// OR + +let ui =
; +function App() { + return ui; +} + +// OR + +function getUI() { + if (condition) return

Hello

; + return null; +} +function App() { + return getUI(); +} +``` diff --git a/tests/lib/rules/no-render-return-undefined.js b/tests/lib/rules/no-render-return-undefined.js index 28ab7e04d5..5a3ce106c5 100644 --- a/tests/lib/rules/no-render-return-undefined.js +++ b/tests/lib/rules/no-render-return-undefined.js @@ -258,6 +258,25 @@ ruleTester.run('no-render-return-undefined', rule, { } `, }, + { + code: ` + let ui = ; + function App() { + return ui; + } + `, + }, + { + code: ` + function getUI() { + if(condition) return

Hello

; + return null; + } + function App() { + return getUI(); + } + `, + }, { code: ` function App() { @@ -669,6 +688,38 @@ ruleTester.run('no-render-return-undefined', rule, { `, errors: [{ messageId: 'returnsUndefined' }], }, + { + code: ` + let ui; + function App() { + return ui; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + let ui; + function App() { + if(cond) { + ui =
; + } + return ui; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + function getUI() { + if(condition) return

Hello

; + } + function App() { + return getUI(); + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, { code: ` function App() { From 8aec646d27c8b67d9928eedd97dae6714451bd2c Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Thu, 16 May 2024 17:47:57 +0530 Subject: [PATCH 08/12] chore: handle map callbacks --- lib/rules/no-render-return-undefined.js | 19 ++++- tests/lib/rules/no-render-return-undefined.js | 79 +++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-render-return-undefined.js b/lib/rules/no-render-return-undefined.js index 6c75885f5a..cc58408f75 100644 --- a/lib/rules/no-render-return-undefined.js +++ b/lib/rules/no-render-return-undefined.js @@ -66,8 +66,23 @@ module.exports = { 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 = variableUtil.variablesInScope(context).find((item) => item.name === calleeName); + 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) @@ -102,7 +117,7 @@ module.exports = { }; const handleFunctionalComponents = (node) => { - const fnName = (node.id && node.id.name) || node.parent.id.name; + const fnName = (node.id && node.id.name) || (node.parent.id && node.parent.id.name); // Considering functions starting with Uppercase letters are react components const isReactComponent = isFirstLetterCapitalized(fnName); diff --git a/tests/lib/rules/no-render-return-undefined.js b/tests/lib/rules/no-render-return-undefined.js index 5a3ce106c5..0ed62a55ba 100644 --- a/tests/lib/rules/no-render-return-undefined.js +++ b/tests/lib/rules/no-render-return-undefined.js @@ -417,6 +417,42 @@ ruleTester.run('no-render-return-undefined', rule, { } `, }, + { + code: ` + const data = ['John', 'Ram' , 'Akul']; + function App() { + return data.map(name =>

{name}

); + } + `, + }, + { + code: ` + const data = ['John', 'Ram' , 'Akul']; + function App() { + return data.map(name => { + return

{name}

; + }); + } + `, + }, + { + code: ` + const data = ['John', 'Ram' , 'Akul']; + function App() { + return cond && data.map(name => { + return

{name}

; + }); + } + `, + }, + { + code: ` + const data = ['John', 'Ram' , 'Akul']; + function App() { + return cond ? data.map(name =>

{name}

) : null; + } + `, + }, ]), // Invalid Cases @@ -845,5 +881,48 @@ ruleTester.run('no-render-return-undefined', rule, { `, errors: [{ messageId: 'returnsUndefined' }], }, + { + code: ` + const data = ['John', 'Ram' , 'Akul']; + function App() { + return cond ? data.map(name =>

{name}

) : undefined; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + const data = ['John', 'Ram' , 'Akul']; + function App() { + return data.map(name => { + if(cond) return

{name}

; + }); + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + const data = ['John', 'Ram' , 'Akul']; + function App() { + if(cond) { + return data.map(name =>

{name}

); + } + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, + { + code: ` + let x; + const data = ['John', 'Ram' , 'Akul']; + function App() { + return cond ? data.map(name => { + return

{name}

; + }) : x; + } + `, + errors: [{ messageId: 'returnsUndefined' }], + }, ]), }); From d1814c1cf24f68ceff2fccd2e0bd3d4271bfbbd2 Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Sat, 25 May 2024 10:42:44 +0530 Subject: [PATCH 09/12] refactor: pr review changes --- docs/rules/no-render-return-undefined.md | 10 +- lib/rules/no-render-return-undefined.js | 153 ++++++++++++----------- package.json | 2 +- 3 files changed, 85 insertions(+), 80 deletions(-) diff --git a/docs/rules/no-render-return-undefined.md b/docs/rules/no-render-return-undefined.md index a80cdf5666..a1a401d302 100644 --- a/docs/rules/no-render-return-undefined.md +++ b/docs/rules/no-render-return-undefined.md @@ -4,17 +4,19 @@ -> 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 index cc58408f75..ad35d0bada 100644 --- 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 index 7b88133c93..685e65a222 100644 --- 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": { From b7ca1dd903e4fac0cd4d98c7a073ef5d68f758e5 Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Sat, 25 May 2024 17:20:46 +0530 Subject: [PATCH 10/12] fix: failing tests --- lib/rules/index.js | 1 + lib/rules/no-render-return-undefined.js | 2 +- lib/util/eslint.js | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/rules/index.js b/lib/rules/index.js index 784831bba7..14cc2546c7 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -105,4 +105,5 @@ module.exports = { 'static-property-placement': require('./static-property-placement'), 'style-prop-object': require('./style-prop-object'), 'void-dom-elements-no-children': require('./void-dom-elements-no-children'), + 'no-render-return-undefined': require('./no-render-return-undefined'), }; diff --git a/lib/rules/no-render-return-undefined.js b/lib/rules/no-render-return-undefined.js index ad35d0bada..3440f0f0f0 100644 --- a/lib/rules/no-render-return-undefined.js +++ b/lib/rules/no-render-return-undefined.js @@ -16,7 +16,7 @@ const messages = { }; function getReturnValue(context, returnNode) { - const variables = variableUtil.variablesInScope(context); + const variables = variableUtil.variablesInScope(context, returnNode); const returnIdentifierName = returnNode && returnNode.name; const returnIdentifierVar = variableUtil.getVariable( variables, diff --git a/lib/util/eslint.js b/lib/util/eslint.js index 79a0537f3b..d5d4813c2a 100644 --- a/lib/util/eslint.js +++ b/lib/util/eslint.js @@ -11,7 +11,7 @@ function getAncestors(context, node) { function getScope(context, node) { const sourceCode = getSourceCode(context); - if (sourceCode.getScope) { + if (sourceCode.getScope && node) { return sourceCode.getScope(node); } From f4983ae1e659e1edea098fbd2bf5205ccd7e68b9 Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Tue, 28 May 2024 12:08:11 +0530 Subject: [PATCH 11/12] chore: pr review change --- docs/rules/no-render-return-undefined.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/no-render-return-undefined.md b/docs/rules/no-render-return-undefined.md index a1a401d302..e2ea496ae2 100644 --- a/docs/rules/no-render-return-undefined.md +++ b/docs/rules/no-render-return-undefined.md @@ -4,7 +4,7 @@ -> 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`. +> 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 From 0b56b095b9f9c8fd2d1b5870108c077056ef5a7e Mon Sep 17 00:00:00 2001 From: akulsr0 Date: Fri, 21 Jun 2024 08:11:50 +0530 Subject: [PATCH 12/12] refactor: sort rules index alphabetically --- lib/rules/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/index.js b/lib/rules/index.js index 14cc2546c7..054d676da2 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -73,10 +73,11 @@ module.exports = { 'no-is-mounted': require('./no-is-mounted'), 'no-multi-comp': require('./no-multi-comp'), 'no-namespace': require('./no-namespace'), - 'no-set-state': require('./no-set-state'), - 'no-string-refs': require('./no-string-refs'), 'no-redundant-should-component-update': require('./no-redundant-should-component-update'), + 'no-render-return-undefined': require('./no-render-return-undefined'), 'no-render-return-value': require('./no-render-return-value'), + 'no-set-state': require('./no-set-state'), + 'no-string-refs': require('./no-string-refs'), 'no-this-in-sfc': require('./no-this-in-sfc'), 'no-typos': require('./no-typos'), 'no-unescaped-entities': require('./no-unescaped-entities'), @@ -105,5 +106,4 @@ module.exports = { 'static-property-placement': require('./static-property-placement'), 'style-prop-object': require('./style-prop-object'), 'void-dom-elements-no-children': require('./void-dom-elements-no-children'), - 'no-render-return-undefined': require('./no-render-return-undefined'), };