diff --git a/.changeset/slow-rocks-refuse.md b/.changeset/slow-rocks-refuse.md new file mode 100644 index 0000000..9fcdec3 --- /dev/null +++ b/.changeset/slow-rocks-refuse.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-effector": minor +--- + +Add new rule `enforce-exhaustive-useUnit-destructuring` diff --git a/.gitignore b/.gitignore index 61450cc..e9ffb1f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ +.idea/ + # Logs logs *.log diff --git a/src/index.ts b/src/index.ts index 2787ff8..21eba7d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,6 +3,7 @@ import type { ESLint, Linter } from "eslint" import { name, version } from "../package.json" import enforceEffectNamingConvention from "./rules/enforce-effect-naming-convention/enforce-effect-naming-convention" +import enforceExhaustiveUseUnitDestructuring from "./rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring" import enforceGateNamingConvention from "./rules/enforce-gate-naming-convention/enforce-gate-naming-convention" import enforceStoreNamingConvention from "./rules/enforce-store-naming-convention/enforce-store-naming-convention" import keepOptionsOrder from "./rules/keep-options-order/keep-options-order" @@ -29,6 +30,7 @@ const base = { meta: { name, version, namespace: "effector" }, rules: { "enforce-effect-naming-convention": enforceEffectNamingConvention, + "enforce-exhaustive-useUnit-destructuring": enforceExhaustiveUseUnitDestructuring, "enforce-gate-naming-convention": enforceGateNamingConvention, "enforce-store-naming-convention": enforceStoreNamingConvention, "keep-options-order": keepOptionsOrder, diff --git a/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.md b/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.md new file mode 100644 index 0000000..49a6445 --- /dev/null +++ b/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.md @@ -0,0 +1,157 @@ +--- +description: Ensure all units passed to useUnit are properly destructured to avoid unused subscriptions and implicit re-renders. +--- + +# effector/enforce-exhaustive-useUnit-destructuring + +[Related documentation](https://effector.dev/en/api/effector-react/useunit/) + +## Rule Details + +This rule enforces that: + +- All properties passed in an object to useUnit must be destructured to prevent implicit subscriptions; +- All elements passed in an array to useUnit must be destructured to prevent implicit subscriptions also. + +### Object shape + +When using useUnit with an object, you must destructure all keys that you pass. Otherwise, unused units will still +create subscriptions and cause unnecessary re-renders. +TypeScript + +```ts +// 👍 correct - all properties are destructured +const { value, setValue } = useUnit({ + value: $store, + setValue: event, +}); +``` + +```ts +// 👎 incorrect - setValue is not destructured but still creates subscription +const { value } = useUnit({ + value: $store, + setValue: event, // unused but subscribed! +}); +``` + +```ts +// 👎 incorrect - extra is destructured but not passed +const { + value, + setValue, + extra // extra is missing - will be undefined +} = useUnit({ + value: $store, + setValue: event, +}); +``` + +### Array shape + +When using useUnit with an array, you must destructure all elements. Elements that are not destructured will still +create subscriptions, leading to implicit re-renders. +TypeScript + +```ts +// 👍 correct - all elements are destructured +const [value, setValue] = useUnit([$store, event]); +``` + +```ts +// 👎 incorrect - $store is not destructured but creates implicit subscription +const [setValue] = useUnit([event, $store]); +// Component will re-render when $store changes, even though you don't use it! +``` + +```ts +// 👎 incorrect - event and $anotherStore cause implicit subscriptions +const [value] = useUnit([$store, event, $anotherStore]); +// Component re-renders on $store, event, and $anotherStore changes +``` + +## Why is this important? + +Implicit subscriptions can lead to: + +- Performance issues: unnecessary re-renders when unused stores update +- Hard-to-debug behavior: component re-renders for unclear reasons +- Memory leaks: subscriptions that are never cleaned up properly + +## Examples + +### Real-world example + +```tsx +import React, { Fragment } from "react"; +import { createEvent, createStore } from "effector"; +import { useUnit } from "effector-react"; + +const $store = createStore("Hello World!"); +const event = createEvent(); + +// 👎 incorrect +const BadComponent = () => { + const { value } = useUnit({ + value: $store, + setValue: event, // ❌ not used but subscribed! + }); + + return {value}; +}; + +// 👍 correct +const GoodComponent = () => { + const { value, setValue } = useUnit({ + value: $store, + setValue: event, + }); + + return ; +}; +``` + +```tsx +import React, { Fragment } from "react"; +import { createEvent, createStore } from "effector"; +import { useUnit } from "effector-react"; + +const $store = createStore("Hello World!"); +const event = createEvent(); + +// 👎 incorrect - implicit subscription to $store +const BadComponent = () => { + const [setValue] = useUnit([event, $store]); // ❌ $store not used but subscribed! + + return ; +}; + +// 👍 correct - explicit destructuring +const GoodComponent = () => { + const [value, setValue] = useUnit([$store, event]); + + return ; +}; + +// 👍 also correct - only pass what you need +const AlsoGoodComponent = () => { + const [setValue] = useUnit([event]); // ✅ no implicit subscriptions + + return ; +}; +``` + +### When Not To Use It + +If you intentionally want to subscribe to a store without using its value (rare case), you can disable this rule for +that line: + +```tsx +// eslint-disable-next-line effector/enforce-exhaustive-useUnit-destructuring +const { value } = useUnit({ + value: $store, + trigger: $triggerStore, // intentionally subscribing without using +}); +``` + +However, in most cases, you should refactor your code to avoid implicit subscriptions. \ No newline at end of file diff --git a/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.test.ts b/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.test.ts new file mode 100644 index 0000000..9cdde9c --- /dev/null +++ b/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.test.ts @@ -0,0 +1,238 @@ +import { RuleTester } from "@typescript-eslint/rule-tester" +import { parser } from "typescript-eslint" + +import { tsx } from "@/shared/tag" + +import rule from "./enforce-exhaustive-useUnit-destructuring" + +const ruleTester = new RuleTester({ + languageOptions: { + parser, + parserOptions: { + projectService: { allowDefaultProject: ["*.tsx"], defaultProject: "tsconfig.fixture.json" }, + ecmaFeatures: { jsx: true }, + }, + }, +}) + +ruleTester.run("enforce-exhaustive-useUnit-destructuring", rule, { + valid: [ + { + name: "object: all keys were destructured", + code: tsx` + import { useUnit } from "effector-react" + const { value, setValue } = useUnit({ + value: $store, + setValue: event, + }) + `, + }, + { + name: "array: all keys were destructured", + code: tsx` + import { useUnit } from "effector-react" + const [value, setValue] = useUnit([$store, event]) + `, + }, + { + name: "object: with one element", + code: tsx` + import { useUnit } from "effector-react" + const { value } = useUnit({ value: $store }) + `, + }, + { + name: "array: with one element", + code: tsx` + import { useUnit } from "effector-react" + const [value] = useUnit([$store]) + `, + }, + { + name: "nocheck: is not useUnit", + code: tsx` + const { value } = someOtherFunction({ + value: $store, + setValue: event, + }) + `, + }, + { + name: "alias: all keys destructured", + code: tsx` + import { useUnit as useEffectorUnit } from "effector-react" + const { value, setValue } = useEffectorUnit({ + value: $store, + setValue: event, + }) + `, + }, + { + name: "array: all elements destructured with no holes", + code: tsx` + import { useUnit } from "effector-react" + const [a, b, c] = useUnit([$a, $b, $c]) + `, + }, + ], + + invalid: [ + { + name: "object: key is passed but not destructured", + code: tsx` + import { useUnit } from "effector-react" + const { value } = useUnit({ + value: $store, + setValue: event, + }) + `, + errors: [ + { + messageId: "unusedKey", + data: { name: "setValue" }, + line: 2, + column: 27, + endLine: 5, + endColumn: 2, + }, + ], + }, + { + name: "object: key is destructured but does not exist in passed object", + code: tsx` + import { useUnit } from "effector-react" + const { value, setValue, extra } = useUnit({ + value: $store, + setValue: event, + }) + `, + errors: [ + { + messageId: "missingKey", + data: { name: "extra" }, + }, + ], + }, + { + name: "array: implicit subscription when not all elements are destructured", + code: tsx` + import { useUnit } from "effector-react" + const [setValue] = useUnit([event, $store]) + `, + errors: [ + { + messageId: "unusedKey", + data: { name: "$store" }, + line: 2, + column: 28, + endLine: 2, + endColumn: 43, + }, + ], + }, + { + name: "array: several implicit subscriptions", + code: tsx` + import { useUnit } from "effector-react" + const [value] = useUnit([$store, event, $anotherStore]) + `, + errors: [ + { + messageId: "unusedKey", + data: { name: "event" }, + }, + { + messageId: "unusedKey", + data: { name: "$anotherStore" }, + }, + ], + }, + { + name: "object: several keys are passed but not destructured", + code: tsx` + import { useUnit } from "effector-react" + const { value } = useUnit({ + value: $store, + setValue: event, + reset: resetEvent, + }) + `, + errors: [ + { + messageId: "unusedKey", + data: { name: "setValue" }, + }, + { + messageId: "unusedKey", + data: { name: "reset" }, + }, + ], + }, + { + name: "object: JSX component: key is passed but not destructured", + code: tsx` + import React, { Fragment } from "react" + import { useUnit } from "effector-react" + + const ObjectShapeComponent = () => { + const { value } = useUnit({ + value: $store, + setValue: event, + }) + return {value} + } + `, + errors: [ + { + messageId: "unusedKey", + data: { name: "setValue" }, + }, + ], + }, + { + name: "alias: key is passed but not destructured", + code: tsx` + import { useUnit as useEffectorUnit } from "effector-react" + const { value } = useEffectorUnit({ + value: $store, + setValue: event, + }) + `, + errors: [ + { + messageId: "unusedKey", + data: { name: "setValue" }, + }, + ], + }, + { + name: "array: implicit subscription on skipped hole in pattern", + code: tsx` + import { useUnit } from "effector-react" + const [a, , c] = useUnit([$a, $b, $c]) + `, + errors: [ + { + messageId: "unusedKey", + data: { name: "$b" }, + }, + ], + }, + { + name: "object: string literal key is passed but not destructured", + code: tsx` + import { useUnit } from "effector-react" + const { value } = useUnit({ + value: $store, + setValue: event, + }) + `, + errors: [ + { + messageId: "unusedKey", + data: { name: "setValue" }, + }, + ], + }, + ], +}) diff --git a/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.ts b/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.ts new file mode 100644 index 0000000..892393c --- /dev/null +++ b/src/rules/enforce-exhaustive-useUnit-destructuring/enforce-exhaustive-useUnit-destructuring.ts @@ -0,0 +1,149 @@ +import { type TSESTree as Node, AST_NODE_TYPES as NodeType } from "@typescript-eslint/utils" + +import { createRule } from "@/shared/create" +import { PACKAGE_NAME } from "@/shared/package" + +type Options = [] + +type MessageIds = "unusedKey" | "missingKey" +type ValueNode = Node.Expression | Node.DestructuringPattern | null + +type ShapeCall = Node.VariableDeclarator & { + init: Node.CallExpression & { + callee: Node.Identifier + arguments: [Node.ObjectExpression] + } + id: Node.ObjectPattern +} + +type ListCall = Node.VariableDeclarator & { + init: Node.CallExpression & { + callee: Node.Identifier + arguments: [Node.ArrayExpression] + } + id: Node.ArrayPattern +} + +function toName(key: string | number, node: ValueNode): string { + if (!node) return `` + if (node.type === NodeType.Identifier) return node.name + if (node.type === NodeType.Literal) return String(node.value) + if (node.type === NodeType.MemberExpression && node.property.type === NodeType.Identifier) { + return `${toName(key, node.object)}.${node.property.name}` + } + return `` +} + +function* check( + provided: Map, + consumed: Map, +): Generator<{ type: "unused" | "missing"; name: string }> { + for (const [key, node] of provided) { + if (!consumed.has(key)) yield { type: "unused", name: toName(key, node) } + } + for (const [key, node] of consumed) { + if (!provided.has(key)) yield { type: "missing", name: toName(key, node) } + } +} + +function toKey(prop: Node.Property): string | number | null { + if (prop.computed) return null + if (prop.key.type === NodeType.Identifier) return prop.key.name + if (prop.key.type === NodeType.Literal) return prop.key.value + return null +} + +function shapeToKeyMap(shape: Node.ObjectPattern | Node.ObjectExpression): Map | null { + const map = new Map() + + for (const prop of shape.properties) { + if (prop.type !== NodeType.Property) return null + + const key = toKey(prop) + + if (key === null) return null + + map.set(key, prop.key) + } + + return map +} + +function listToKeyMap(list: Node.ArrayPattern | Node.ArrayExpression): Map | null { + const map = new Map() + + for (const [index, element] of list.elements.entries()) { + if (element === null) continue + if (element.type === NodeType.RestElement || element.type === NodeType.SpreadElement) return null + + map.set(index, element as ValueNode) + } + + return map +} + +const selector = { + import: `ImportDeclaration[source.value=${PACKAGE_NAME.react}] > ImportSpecifier[imported.name=useUnit]`, + variable: { + shape: "VariableDeclarator[id.type=ObjectPattern]", + list: "VariableDeclarator[id.type=ArrayPattern]", + }, + call: "CallExpression.init[arguments.length=1][callee.type=Identifier]", + arg: { + shape: "ObjectExpression.arguments", + list: "ArrayExpression.arguments", + }, +} as const + +export default createRule({ + name: "enforce-exhaustive-useUnit-destructuring", + meta: { + type: "problem", + docs: { + description: "Ensure all units passed to useUnit are properly destructured.", + }, + messages: { + unusedKey: 'Property "{{name}}" is passed but not destructured.', + missingKey: 'Property "{{name}}" is destructured but not passed in the unit object.', + }, + schema: [], + defaultOptions: [], + }, + create(context) { + const importedAs = new Set() + + return { + [selector.import]: (node: Node.ImportSpecifier) => void importedAs.add(node.local.name), + + [`${selector.variable.shape}:has(> ${selector.call}:has(${selector.arg.shape}))`](node: ShapeCall): void { + if (!importedAs.has(node.init.callee.name)) return + + const provided = shapeToKeyMap(node.init.arguments[0]) + const consumed = shapeToKeyMap(node.id) + + if (provided === null || consumed === null) return + + for (const { type, name } of check(provided, consumed)) { + if (type === "unused") + context.report({ node: node.init.arguments[0], messageId: "unusedKey", data: { name } }) + else context.report({ node: node.id, messageId: "missingKey", data: { name } }) + } + }, + + [`${selector.variable.list}:has(> ${selector.call}:has(${selector.arg.list}))`](node: ListCall): void { + if (!importedAs.has(node.init.callee.name)) return + + const provided = listToKeyMap(node.init.arguments[0]) + const consumed = listToKeyMap(node.id) + + if (provided === null || consumed === null) return + + for (const { type, name } of check(provided, consumed)) { + if (type === "unused") + context.report({ node: node.init.arguments[0], messageId: "unusedKey", data: { name } }) + else context.report({ node: node.id, messageId: "missingKey", data: { name } }) + } + }, + } + }, +}) diff --git a/src/ruleset.ts b/src/ruleset.ts index 25fc617..64c19dd 100644 --- a/src/ruleset.ts +++ b/src/ruleset.ts @@ -26,6 +26,7 @@ const scope = { const react = { "effector/enforce-gate-naming-convention": "error", + "effector/enforce-exhaustive-useUnit-destructuring": "warn", "effector/mandatory-scope-binding": "error", "effector/no-units-spawn-in-render": "error", "effector/prefer-useUnit": "error",