diff --git a/.gitignore b/.gitignore index 61450cc..91cd5fb 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ +.idea +**/*.xml + # Logs logs *.log diff --git a/config/react.js b/config/react.js new file mode 100644 index 0000000..e69de29 diff --git a/index.js b/index.js new file mode 100644 index 0000000..e69de29 diff --git a/src/index.ts b/src/index.ts index bac8f24..0cee143 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,6 +22,7 @@ import noWatch from "./rules/no-watch/no-watch" import preferUseUnit from "./rules/prefer-useUnit/prefer-useUnit" import requirePickupInPersist from "./rules/require-pickup-in-persist/require-pickup-in-persist" import strictEffectHandlers from "./rules/strict-effect-handlers/strict-effect-handlers" +import useUnitDestructuring from "./rules/use-unit-destructuring/use-unit-destructuring" import { ruleset } from "./ruleset" const base = { @@ -45,6 +46,7 @@ const base = { "no-useless-methods": noUselessMethods, "no-watch": noWatch, "prefer-useUnit": preferUseUnit, + "use-unit-destructuring": useUnitDestructuring, "require-pickup-in-persist": requirePickupInPersist, "strict-effect-handlers": strictEffectHandlers, }, diff --git a/src/rules/use-unit-destructuring/use-unit-destructuring.md b/src/rules/use-unit-destructuring/use-unit-destructuring.md new file mode 100644 index 0000000..88d6d7e --- /dev/null +++ b/src/rules/use-unit-destructuring/use-unit-destructuring.md @@ -0,0 +1,145 @@ +# effector/use-unit-destructuring + +[Related documentation](https://effector.dev/en/api/effector-react/useunit/) + +Ensures that all units passed to useUnit are properly destructured to avoid unused subscriptions and implicit re-renders. + +## 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/use-unit-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/use-unit-destructuring/use-unit-destructuring.test.ts b/src/rules/use-unit-destructuring/use-unit-destructuring.test.ts new file mode 100644 index 0000000..de1eb69 --- /dev/null +++ b/src/rules/use-unit-destructuring/use-unit-destructuring.test.ts @@ -0,0 +1,165 @@ +import { RuleTester } from "@typescript-eslint/rule-tester" + +import rule from "./use-unit-destructuring" + +const ruleTester = new RuleTester({ + languageOptions: { + parserOptions: { + ecmaVersion: 2020, + sourceType: "module", + ecmaFeatures: { jsx: true }, + }, + }, +}) + +ruleTester.run("effector/use-unit-destructuring", rule, { + valid: [ + // All keys were destructured + { + code: ` + import { useUnit } from "effector-react"; + const { value, setValue } = useUnit({ + value: $store, + setValue: event, + }); + `, + }, + // All keys were destructured + { + code: ` + import { useUnit } from "effector-react"; + const [value, setValue] = useUnit([$store, event]); + `, + }, + // With one element in object-shape + { + code: ` + import { useUnit } from "effector-react"; + const { value } = useUnit({ value: $store }); + `, + }, + // With one element in array-shape + { + code: ` + import { useUnit } from "effector-react"; + const [value] = useUnit([$store]); + `, + }, + // Is not useUnit - no check + { + code: ` + const { value } = someOtherFunction({ + value: $store, + setValue: event, + }); + `, + }, + ], + + invalid: [ + // Object: not destructured + { + code: ` + import { useUnit } from "effector-react"; + const { value } = useUnit({ + value: $store, + setValue: event, + }); + `, + errors: [ + { + messageId: "unusedKey", + data: { key: "setValue" }, + }, + ], + }, + // Object: destructured, but key does not exist + { + code: ` + import { useUnit } from "effector-react"; + const { value, setValue, extra } = useUnit({ + value: $store, + setValue: event, + }); + `, + errors: [ + { + messageId: "missingKey", + data: { key: "extra" }, + }, + ], + }, + // Array: implicit subscription (not all elements were destructuring) + { + code: ` + import { useUnit } from "effector-react"; + const [setValue] = useUnit([event, $store]); + `, + errors: [ + { + messageId: "implicitSubscription", + data: { index: 1, name: "$store" }, + }, + ], + }, + // Array: several implicit subscriptions + { + code: ` + import { useUnit } from "effector-react"; + const [value] = useUnit([$store, event, $anotherStore]); + `, + errors: [ + { + messageId: "implicitSubscription", + data: { index: 1, name: "event" }, + }, + { + messageId: "implicitSubscription", + data: { index: 2, name: "$anotherStore" }, + }, + ], + }, + // Object: several unused keys + { + code: ` + import { useUnit } from "effector-react"; + const { value } = useUnit({ + value: $store, + setValue: event, + reset: resetEvent, + }); + `, + errors: [ + { + messageId: "unusedKey", + data: { key: "setValue" }, + }, + { + messageId: "unusedKey", + data: { key: "reset" }, + }, + ], + }, + // JSX component with object-shape + { + code: ` + 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: { key: "setValue" }, + }, + ], + }, + ], +}) diff --git a/src/rules/use-unit-destructuring/use-unit-destructuring.ts b/src/rules/use-unit-destructuring/use-unit-destructuring.ts new file mode 100644 index 0000000..3fc6397 --- /dev/null +++ b/src/rules/use-unit-destructuring/use-unit-destructuring.ts @@ -0,0 +1,138 @@ +import { AST_NODE_TYPES, type TSESTree } from "@typescript-eslint/utils" + +import { createRule } from "@/shared/create" + +type MessageIds = "unusedKey" | "missingKey" | "implicitSubscription" +type Options = [] + +export default createRule({ + name: "use-unit-destructuring", + meta: { + type: "problem", + docs: { + description: "Ensure destructured properties match the passed unit object/array", + }, + messages: { + unusedKey: 'Property "{{key}}" is passed but not destructured', + missingKey: 'Property "{{key}}" is destructured but not passed in the unit object', + implicitSubscription: + "Element at index {{index}} ({{name}}) is passed but not destructured, causing implicit subscription", + }, + schema: [], + }, + defaultOptions: [], + create(context) { + function handleObjectPattern( + objectArgument: TSESTree.ObjectExpression, + objectPattern: TSESTree.ObjectPattern, + ): void { + // Collect all keys from argument object + const argumentKeys = new Set( + objectArgument.properties + .filter( + (prop): prop is TSESTree.Property => + prop.type === AST_NODE_TYPES.Property && prop.key.type === AST_NODE_TYPES.Identifier, + ) + .map((prop) => (prop.key as TSESTree.Identifier).name), + ) + + // Collect destructured keys + const destructuredKeys = new Set( + objectPattern.properties + .filter( + (prop): prop is TSESTree.Property => + prop.type === AST_NODE_TYPES.Property && prop.key.type === AST_NODE_TYPES.Identifier, + ) + .map((prop) => (prop.key as TSESTree.Identifier).name), + ) + + // Check unused keys + for (const key of argumentKeys) { + if (!destructuredKeys.has(key)) { + context.report({ + node: objectArgument, + messageId: "unusedKey", + data: { key }, + }) + } + } + + // Check missing keys + for (const key of destructuredKeys) { + if (!argumentKeys.has(key)) { + context.report({ + node: objectPattern, + messageId: "missingKey", + data: { key }, + }) + } + } + } + + function handleArrayPattern(arrayArgument: TSESTree.ArrayExpression, arrayPattern: TSESTree.ArrayPattern): void { + const argumentElements = arrayArgument.elements + const destructuredElements = arrayPattern.elements + + const destructuredCount = destructuredElements.filter((el) => el !== null).length + const argumentCount = argumentElements.filter((el) => el !== null).length + + if (destructuredCount >= argumentCount) return + + // If undestructured elements exists + for (let i = destructuredCount; i < argumentCount; i++) { + const element = argumentElements[i] + if (!element || element.type === AST_NODE_TYPES.SpreadElement) continue + + let name = "unknown" + + if (element.type === AST_NODE_TYPES.Identifier) { + name = element.name + } else if (element.type === AST_NODE_TYPES.MemberExpression) { + name = context.sourceCode.getText(element) + } + + context.report({ + node: element, + messageId: "implicitSubscription", + data: { + index: i, + name, + }, + }) + } + } + + return { + CallExpression(node): void { + if ( + node.callee.type !== AST_NODE_TYPES.Identifier || + node.callee.name !== "useUnit" || + node.arguments.length === 0 + ) { + return + } + + const argument = node.arguments[0] + const parent = node.parent + + if ( + !parent || + parent.type !== AST_NODE_TYPES.VariableDeclarator || + argument?.type === AST_NODE_TYPES.SpreadElement + ) { + return + } + + // Shape is Object-like + if (argument?.type === AST_NODE_TYPES.ObjectExpression && parent.id.type === AST_NODE_TYPES.ObjectPattern) { + handleObjectPattern(argument, parent.id) + } + + // Shape is Array-like + if (argument?.type === AST_NODE_TYPES.ArrayExpression && parent.id.type === AST_NODE_TYPES.ArrayPattern) { + handleArrayPattern(argument, parent.id) + } + }, + } + }, +}) diff --git a/src/ruleset.ts b/src/ruleset.ts index b11fbd2..5c57ba4 100644 --- a/src/ruleset.ts +++ b/src/ruleset.ts @@ -28,6 +28,7 @@ const react = { "effector/enforce-gate-naming-convention": "error", "effector/mandatory-scope-binding": "error", "effector/prefer-useUnit": "error", + "effector/use-unit-destructuring": "warn", } satisfies TSESLint.Linter.RulesRecord const future = {