Skip to content

Commit

Permalink
chore: Add named-use-effect custom eslint rule (#36725)
Browse files Browse the repository at this point in the history
## Description

Adds a custom es lint rule that would add a warning when a useEffect is
used without a named function inside it


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced a new ESLint rule to enforce the use of named functions
within the `useEffect` hook, promoting better code readability.
- Updated recommended ESLint configuration to include the new rule with
a warning level.

- **Tests**
- Added a comprehensive test suite for the new `namedUseEffectRule`,
covering both valid and invalid usage scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
hetunandu authored Oct 9, 2024
1 parent 6e59db2 commit 71963b5
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 0 deletions.
3 changes: 3 additions & 0 deletions app/client/packages/eslint-plugin/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import { objectKeysRule } from "./object-keys/rule";
import { namedUseEffectRule } from "./named-use-effect/rule";

const plugin = {
rules: {
"object-keys": objectKeysRule,
"named-use-effect": namedUseEffectRule,
},
configs: {
recommended: {
rules: {
"@appsmith/object-keys": "warn",
"@appsmith/named-use-effect": "warn",
},
},
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { TSESLint } from "@typescript-eslint/utils";
import { namedUseEffectRule } from "./rule";

const ruleTester = new TSESLint.RuleTester();

ruleTester.run("named-use-effect", namedUseEffectRule, {
valid: [
{
code: "useEffect(function add(){ }, [])",
},
{
code: "React.useEffect(function add(){ }, [])",
},
],
invalid: [
{
code: "useEffect(function (){ }, [])",
errors: [{ messageId: "useNamedUseEffect" }],
},
{
code: "React.useEffect(function (){ }, [])",
errors: [{ messageId: "useNamedUseEffect" }],
},
],
});
65 changes: 65 additions & 0 deletions app/client/packages/eslint-plugin/src/named-use-effect/rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import type { TSESLint } from "@typescript-eslint/utils";

export const namedUseEffectRule: TSESLint.RuleModule<"useNamedUseEffect"> = {
defaultOptions: [],
meta: {
type: "suggestion",
docs: {
description: "Warns when useEffect hook has an anonymous function",
recommended: "warn",
},
schema: [], // No options
messages: {
useNamedUseEffect:
"The function inside the useEffect should be named for better readability eg: useEffect(function mySideEffect() {...}, [])",
},
},
create(context) {
return {
CallExpression(node) {
// useEffect used directly
// eg
// import { useEffect } from "react";
// ...
// useEffect(() => {}, [])
const isDirectCall =
node.callee.type === "Identifier" && node.callee.name === "useEffect";

// useEffect used via React object
// eg
// import React from "react";
// ...
// React.useEffect(() => {}, [])
const isMemberExpressionCall =
node.callee.type === "MemberExpression" &&
node.callee.object.type === "Identifier" &&
node.callee.object.name === "React" &&
node.callee.property.type === "Identifier" &&
node.callee.property.name === "useEffect";

if (isDirectCall || isMemberExpressionCall) {
// Get the first argument which should be a function
const callbackArg = node.arguments[0];

// Arrow function are never named so it is discouraged
if (callbackArg.type === "ArrowFunctionExpression") {
context.report({
node: callbackArg,
messageId: "useNamedUseEffect",
});
}

// Function Expressions can be unnamed. This is also discouraged
if (callbackArg.type === "FunctionExpression") {
if (!callbackArg.id) {
context.report({
node: callbackArg,
messageId: "useNamedUseEffect",
});
}
}
}
},
};
},
};

0 comments on commit 71963b5

Please sign in to comment.