From a69c97fa3ea5fcc5cf1c21aabd11d8b5da72996c Mon Sep 17 00:00:00 2001 From: Josh Wilson Date: Sat, 30 Dec 2023 15:43:53 -0500 Subject: [PATCH] Make function name checking more resilient for #116. --- docs/components-return-once.md | 28 +++++++++++++++++++++++ src/rules/components-return-once.ts | 4 ++-- src/rules/reactivity.ts | 17 ++++++++------ src/utils.ts | 13 +++++++++++ test/rules/components-return-once.test.ts | 25 ++++++++++++++++++++ 5 files changed, 78 insertions(+), 9 deletions(-) diff --git a/docs/components-return-once.md b/docs/components-return-once.md index 36b91a9..1c1f4eb 100644 --- a/docs/components-return-once.md +++ b/docs/components-return-once.md @@ -145,5 +145,33 @@ callback(() => { return
; }); +function Component() { + const renderContent = () => { + if (false) return <>; + return <>; + }; + return <>{renderContent()}; +} + +function Component() { + function renderContent() { + if (false) return <>; + return <>; + } + return <>{renderContent()}; +} + +function Component() { + const renderContent = () => { + const renderContentInner = () => { + // ifs in render functions are fine no matter what nesting level this is + if (false) return; + return <>; + }; + return <>{renderContentInner()}; + }; + return <>; +} + ``` diff --git a/src/rules/components-return-once.ts b/src/rules/components-return-once.ts index eef993d..7838e18 100644 --- a/src/rules/components-return-once.ts +++ b/src/rules/components-return-once.ts @@ -1,5 +1,5 @@ import { TSESTree as T, ESLintUtils } from "@typescript-eslint/utils"; -import type { FunctionNode } from "../utils"; +import { getFunctionName, type FunctionNode } from "../utils"; const createRule = ESLintUtils.RuleCreator.withoutDocs; @@ -63,8 +63,8 @@ export default createRule({ const onFunctionExit = (node: FunctionNode) => { if ( - (node.type === "FunctionDeclaration" && node.id?.name?.match(/^[a-z]/)) || // "render props" aren't components + getFunctionName(node)?.match(/^[a-z]/) || node.parent?.type === "JSXExpressionContainer" || // ignore createMemo(() => conditional JSX), report HOC(() => conditional JSX) (node.parent?.type === "CallExpression" && diff --git a/src/rules/reactivity.ts b/src/rules/reactivity.ts index 911d147..ef80db4 100644 --- a/src/rules/reactivity.ts +++ b/src/rules/reactivity.ts @@ -15,6 +15,7 @@ import { trackImports, isDOMElementName, ignoreTransparentWrappers, + getFunctionName, } from "../utils"; const { findVariable, getFunctionHeadLocation } = ASTUtils; @@ -425,15 +426,17 @@ export default createRule({ const onFunctionExit = (currentScopeNode: ProgramOrFunctionNode) => { // If this function is a component, add its props as a reactive variable if (isFunctionNode(currentScopeNode)) { - markPropsOnCondition( - currentScopeNode, - (props) => + markPropsOnCondition(currentScopeNode, (props) => { + if ( !isPropsByName(props.name) && // already added in markPropsOnEnter - currentScope().hasJSX && + currentScope().hasJSX + ) { + const functionName = getFunctionName(currentScopeNode); // begins with lowercase === not component - (currentScopeNode.type !== "FunctionDeclaration" || - !currentScopeNode.id?.name?.match(/^[a-z]/)) - ); + if (functionName && !/^[a-z]/.test(functionName)) return true; + } + return false; + }); } // Ignore sync callbacks like Array#forEach and certain Solid primitives. diff --git a/src/utils.ts b/src/utils.ts index c3ec25e..aacc45f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -97,6 +97,19 @@ export const isProgramOrFunctionNode = ( node: T.Node | null | undefined ): node is ProgramOrFunctionNode => !!node && PROGRAM_OR_FUNCTION_TYPES.includes(node.type); +export const getFunctionName = (node: FunctionNode): string | null => { + if ( + (node.type === "FunctionDeclaration" || node.type === "FunctionExpression") && + node.id != null + ) { + return node.id.name; + } + if (node.parent?.type === "VariableDeclarator" && node.parent.id.type === "Identifier") { + return node.parent.id.name; + } + return null; +}; + export function findInScope( node: T.Node, scope: ProgramOrFunctionNode, diff --git a/test/rules/components-return-once.test.ts b/test/rules/components-return-once.test.ts index c0ea2f6..e8ac466 100644 --- a/test/rules/components-return-once.test.ts +++ b/test/rules/components-return-once.test.ts @@ -24,6 +24,31 @@ export const cases = run("components-return-once", rule, { } return
; });`, + `function Component() { + const renderContent = () => { + if (false) return <>; + return <>; + } + return <>{renderContent()}; + }`, + `function Component() { + function renderContent() { + if (false) return <>; + return <>; + } + return <>{renderContent()}; + }`, + `function Component() { + const renderContent = () => { + const renderContentInner = () => { + // ifs in render functions are fine no matter what nesting level this is + if (false) return; + return <>; + }; + return <>{renderContentInner()}; + }; + return <>; + }`, ], invalid: [ // Early returns