From 001ee58de099f497b76f9a6621c15b7f5dddcb9c Mon Sep 17 00:00:00 2001 From: Hetu Nandu Date: Mon, 7 Oct 2024 18:38:47 +0530 Subject: [PATCH 1/2] Add named use effect --- .../packages/eslint-plugin/src/index.ts | 3 ++ .../src/named-use-effect/rule.test.ts | 22 +++++++++ .../src/named-use-effect/rule.ts | 46 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 app/client/packages/eslint-plugin/src/named-use-effect/rule.test.ts create mode 100644 app/client/packages/eslint-plugin/src/named-use-effect/rule.ts diff --git a/app/client/packages/eslint-plugin/src/index.ts b/app/client/packages/eslint-plugin/src/index.ts index e3e454c20e2..eda6deb6d34 100644 --- a/app/client/packages/eslint-plugin/src/index.ts +++ b/app/client/packages/eslint-plugin/src/index.ts @@ -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", }, }, }, diff --git a/app/client/packages/eslint-plugin/src/named-use-effect/rule.test.ts b/app/client/packages/eslint-plugin/src/named-use-effect/rule.test.ts new file mode 100644 index 00000000000..857c13a1cae --- /dev/null +++ b/app/client/packages/eslint-plugin/src/named-use-effect/rule.test.ts @@ -0,0 +1,22 @@ +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(){ }, [])", + }, + ], + invalid: [ + { + code: "useEffect(function (){ }, [])", + errors: [{ messageId: "useNamedUseEffect" }], + }, + { + code: "useEffect(() => {})", + errors: [{ messageId: "useNamedUseEffect" }], + }, + ], +}); diff --git a/app/client/packages/eslint-plugin/src/named-use-effect/rule.ts b/app/client/packages/eslint-plugin/src/named-use-effect/rule.ts new file mode 100644 index 00000000000..ad5246da4be --- /dev/null +++ b/app/client/packages/eslint-plugin/src/named-use-effect/rule.ts @@ -0,0 +1,46 @@ +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", + }, + }, + create(context) { + return { + CallExpression(node) { + // Check if the useEffect has a named function + if ( + node.callee.type === "Identifier" && + node.callee.name === "useEffect" + ) { + const firstArg = node.arguments[0]; + + if (firstArg.type === "ArrowFunctionExpression") { + context.report({ + node, + messageId: "useNamedUseEffect", + }); + } + + if (firstArg.type === "FunctionExpression") { + if (!firstArg.id) { + context.report({ + node, + messageId: "useNamedUseEffect", + }); + } + } + } + }, + }; + }, +}; From 334c68048e53d6414d8ece2cde017cac17d859a1 Mon Sep 17 00:00:00 2001 From: Hetu Nandu Date: Wed, 9 Oct 2024 15:08:40 +0530 Subject: [PATCH 2/2] Add new case and some comments --- .../src/named-use-effect/rule.test.ts | 5 ++- .../src/named-use-effect/rule.ts | 43 +++++++++++++------ 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/app/client/packages/eslint-plugin/src/named-use-effect/rule.test.ts b/app/client/packages/eslint-plugin/src/named-use-effect/rule.test.ts index 857c13a1cae..267997f9e4a 100644 --- a/app/client/packages/eslint-plugin/src/named-use-effect/rule.test.ts +++ b/app/client/packages/eslint-plugin/src/named-use-effect/rule.test.ts @@ -8,6 +8,9 @@ ruleTester.run("named-use-effect", namedUseEffectRule, { { code: "useEffect(function add(){ }, [])", }, + { + code: "React.useEffect(function add(){ }, [])", + }, ], invalid: [ { @@ -15,7 +18,7 @@ ruleTester.run("named-use-effect", namedUseEffectRule, { errors: [{ messageId: "useNamedUseEffect" }], }, { - code: "useEffect(() => {})", + code: "React.useEffect(function (){ }, [])", errors: [{ messageId: "useNamedUseEffect" }], }, ], diff --git a/app/client/packages/eslint-plugin/src/named-use-effect/rule.ts b/app/client/packages/eslint-plugin/src/named-use-effect/rule.ts index ad5246da4be..5732f04c926 100644 --- a/app/client/packages/eslint-plugin/src/named-use-effect/rule.ts +++ b/app/client/packages/eslint-plugin/src/named-use-effect/rule.ts @@ -11,30 +11,49 @@ export const namedUseEffectRule: TSESLint.RuleModule<"useNamedUseEffect"> = { schema: [], // No options messages: { useNamedUseEffect: - "The function inside the useEffect should be named for better readability", + "The function inside the useEffect should be named for better readability eg: useEffect(function mySideEffect() {...}, [])", }, }, create(context) { return { CallExpression(node) { - // Check if the useEffect has a named function - if ( - node.callee.type === "Identifier" && - node.callee.name === "useEffect" - ) { - const firstArg = node.arguments[0]; + // useEffect used directly + // eg + // import { useEffect } from "react"; + // ... + // useEffect(() => {}, []) + const isDirectCall = + node.callee.type === "Identifier" && node.callee.name === "useEffect"; - if (firstArg.type === "ArrowFunctionExpression") { + // 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, + node: callbackArg, messageId: "useNamedUseEffect", }); } - if (firstArg.type === "FunctionExpression") { - if (!firstArg.id) { + // Function Expressions can be unnamed. This is also discouraged + if (callbackArg.type === "FunctionExpression") { + if (!callbackArg.id) { context.report({ - node, + node: callbackArg, messageId: "useNamedUseEffect", }); }