Skip to content

Commit

Permalink
Merge pull request #121 from solidjs-community/fix/func-name-check
Browse files Browse the repository at this point in the history
Make function name checking more resilient for #116.
  • Loading branch information
joshwilsonvu authored Dec 30, 2023
2 parents 87e2f9b + a69c97f commit 85c4f5a
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 9 deletions.
28 changes: 28 additions & 0 deletions docs/components-return-once.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,5 +145,33 @@ callback(() => {
return <div />;
});

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 <></>;
}

```
<!-- AUTO-GENERATED-CONTENT:END -->
4 changes: 2 additions & 2 deletions src/rules/components-return-once.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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" &&
Expand Down
17 changes: 10 additions & 7 deletions src/rules/reactivity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
trackImports,
isDOMElementName,
ignoreTransparentWrappers,
getFunctionName,
} from "../utils";

const { findVariable, getFunctionHeadLocation } = ASTUtils;
Expand Down Expand Up @@ -425,15 +426,17 @@ export default createRule<Options, MessageIds>({
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.
Expand Down
13 changes: 13 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
25 changes: 25 additions & 0 deletions test/rules/components-return-once.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,31 @@ export const cases = run("components-return-once", rule, {
}
return <div />;
});`,
`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
Expand Down

0 comments on commit 85c4f5a

Please sign in to comment.