feat(prefer-single-binding): add rule by Olovyannikov · Pull Request #176 · effector/eslint-plugin

Olovyannikov · 2025-12-14T10:28:20Z

Closes #116

Why is this important?

Performance

Each useUnit call creates its own subscription management overhead. Combining them reduces:

Number of hook calls
Subscription management overhead
Re-render coordination complexity

Code clarity

A single useUnit call makes it easier to:

See all dependencies at a glance
Understand component's reactive logic
Maintain and refactor code

Example

// correct
const Component = () => {
  const [value, setValue] = useUnit([$store, storeUpdated]);
}

// incorrect
const Component = () => {
  const [value] = useUnit([$store]);
  const [setValue] = useUnit([storeUpdated]);
}

Separate stores and events calls

If you want to separate stores and events as independent groups, you can use option separation

When set to allow, allows separate useUnit calls for stores and events, but still enforces combining multiple calls within each group.

Example

// correct
const Component = () => {
  const [value, setValue] = useUnit([$store, storeUpdated]);
}

// if `separation` flag is 'enforce':
const Component = () => {
  const [value, setValue] = useUnit([$store, storeUpdated]);
  
   // will be transformed to:
   
  const [value] = useUnit([$store]);
  const [setValue] = useUnit([storeUpdated])
}

// also correct
const Component = () => {
  const [value] = useUnit([$store]);
  const [setValue] = useUnit([storeUpdated]);
}

// incorrect
const Component = () => {
  const [value] = useUnit([$store]);
  const [setValue] = useUnit([storeUpdated]);
  const [setAnotherValue] = useUnit([thirdEvent]);
}

netlify · 2025-12-14T10:28:25Z

✅ Deploy Preview for eslint-plugin ready!

Name	Link
🔨 Latest commit	`3b4f69d`
🔍 Latest deploy log	https://app.netlify.com/projects/eslint-plugin/deploys/6a2fb597175de40008528fcf
😎 Deploy Preview	https://deploy-preview-176--eslint-plugin.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Olovyannikov · 2026-04-14T09:53:06Z

@kireevmp can you review this code please?
I did it similarly to #175. I removed the unit type detection using heuristics and replaced it with type-check. And updated the test cases at the same time.

kireevmp

Hey @Olovyannikov! I skimmed through the PR and before we dive into actual implementation specifics, let's work out what this rule should check for and what we'd consider a violation. Right now it seems there are a few edge cases that need considering.

kireevmp · 2026-04-16T13:04:05Z

+    schema: [
+      {
+        type: "object",
+        properties: {
+          allowSeparateStoresAndEvents: { type: "boolean", default: false },
+          enforceStoresAndEventsSeparation: { type: "boolean", default: false },
+        },
+        additionalProperties: false,
+      },
+    ],


allow and enforce settings are not actually independent, because you can't enforce separation while also disallowing it. These are better merged into a string flag, i.e. separation: "forbid" | "allow" | "enforce"

Yes, you're right. I made a string flag now.

You added three items here: forbid, allow and enforce like I suggested. Can you explain how allow is different during code generation from forbid and enforce 'separation"? And how would the user know clearly why allow works this way?

kireevmp · 2026-04-16T13:06:49Z

+        additionalProperties: false,
+      },
+    ],
+    fixable: "code",


The code fix output is not runtime equivalent to the input, so we shouldn't use autofix here. Since it modifies the runtime behavior, it must be a suggestion.

kireevmp · 2026-04-16T13:09:08Z

+    let services: ParserServices | null = null
+    try {
+      const s = ESLintUtils.getParserServices(context, true)
+      if (s.program) services = s
+    } catch {
+      services = null
+    }


The plugin generally requires TypeScript, and eslint-typescript guidelines recommend against rules that change behavior based on type information availability. So I suggest directly getting services as you need where you need without try/catch handling and other conditionals.

kireevmp · 2026-04-16T13:10:30Z

+      "FunctionDeclaration": onFunctionEnter,
+      "FunctionExpression": onFunctionEnter,
+      "ArrowFunctionExpression": onFunctionEnter,
+
+      "FunctionDeclaration:exit": onFunctionExit,
+      "FunctionExpression:exit": onFunctionExit,
+      "ArrowFunctionExpression:exit": onFunctionExit,


Consider merging selectors into a single "logical" one FunctionDeclaration, FunctionExpression, ArrowFunctionExpression

Indeed, you can simply list them separated by commas. Did it, thanks.

kireevmp · 2026-04-16T13:15:23Z

  "effector/no-unnecessary-duplication": "warn",
  "effector/no-useless-methods": "error",
  "effector/no-watch": "warn",
+  "effector/prefer-single-binding": "warn",


This is a react rule, not recommended one – it only concerns React users and not Vue users, for example

Since this is a stylistic rule (enforces code style, not correctness), we can't really include it into react preset either

How about the style group?

That'd be something close to react-stylistic preset, yeah. But I'm not sure we want to make a new preset just for one rule. Let's ponder on this once implementation is close to completion.

kireevmp · 2026-04-16T13:27:00Z

+  meta: {
+    type: "suggestion",
+    docs: {
+      description: "Recommend using a single useUnit call instead of multiple",


I'm a bit concerned with rule expectation mismatch here. It declares that it "recommends a single useUnit call", however

This rule does work with about non-destructuring calls const a = useUnit($a) and non-combining calls const { a, b } = useUnit(model) with property access

There are no tests illustrating how this rule works with Effects

There are no tests that combines both array form and object form in a single component

There are no tests that play with variable positioning, for example where the unit consumed is not defined statically
const { a } = useUnit({ a: $a }) const { $b } = useContext(MyModel) const { b } = useUnit({ b: $b })

This rule does not declare how it works with @@unitShape (i.e. routers, queries)

With a proper setting, it actually enforces 2 useUnit calls with separation, so not a "single" call

I've also not found tests demonstrating how this rule works with aliasing
const { a } = useUnit({ a: $a }); const { a: b } = useUnit({ a: $b })

My question here is – what are the expectations in these cases? Is there a sound "fix" we can provide for all of them? And should we actually check and report those cases?

Yes, I will add more cases.

Can we take a step back and work on this in comments first? It would be much easier to discuss conceptual solutions without diving into code other than examples. I'd love your opinions on what solutions, if any, are available for every listed case

Aliasing works - I've added tests for this.
Also, we can't offer a fix for all cases; for such cases, we'll simply mark it as a warning.
Regarding effects, there's an issue suggesting creating a rule that prohibits working with effects directly. If that rule is included in the recommended rule, you can omit that case here, but by default, I currently separate calls when using separation enforce.

What about UnitShape - I'm not sure what the correct approach would be - highlight it as a warning? Or skip it, since it operates according to special rules?

Examples:

separation: 'forbid'

// Store + Event // ESLint: Multiple useUnit calls detected. Consider combining them into a single call for better performance. (effector/prefer-single-binding) const products = useUnit(CartModel.$cart); const onProductRemoveFromCart = useUnit(AddToCartModel.cartProductTotalRemoved);

// Store + Event Array Shaped // ESLint: Multiple useUnit calls detected. Consider combining them into a single call for better performance. (effector/prefer-single-binding) // Suggest to merge this calls included const [products] = useUnit([CartModel.$cartProducts]); const [onProductRemoveFromCart] = useUnit([AddToCartModel.cartProductTotalRemoved]);

// Store + Event Object Shaped // ESLint: Multiple useUnit calls detected. Consider combining them into a single call for better performance. (effector/prefer-single-binding) // Suggest to merge this calls included const { products } = useUnit({ products: CartModel.$cartProducts, }); const { onProductRemoveFromCart } = useUnit({ onProductRemoveFromCart: AddToCartModel.cartProductTotalRemoved, });

// Mixed shapes - Array + Object // ESLint: Multiple useUnit calls detected. Consider combining them into a single call for better performance. (effector/prefer-single-binding) const [products] = useUnit([CartModel.$cartProducts]); const { onProductRemoveFromCart } = useUnit({ onProductRemoveFromCart: AddToCartModel.cartProductTotalRemoved, });

// Using alias // ESLint: Multiple useUnit calls detected. Consider combining them into a single call for better performance. (effector/prefer-single-binding) const { products } = useUnitEffector({ products: CartModel.$cartProducts, }); const [onProductRemoveFromCart] = useUnitEffector([AddToCartModel.cartProductTotalRemoved]);

// named destructuring // ESLint: Multiple useUnit calls detected. Consider combining them into a single call for better performance. (effector/prefer-single-binding) const { a } = useUnitEffector({ a: CartModel.$cartProducts, }); const { a: b } = useUnitEffector({ a: AddToCartModel.cartProductTotalRemoved, });

With both @@unitShape and a pre-build useUnit(model) where the argument is not a unit or shape of units, I'd suggest we fully exclude them from calculation. These can never be soundly "combined" so they're logically not subject to rule enforcement / separation.

With Store + Event basic case, this is actually the most common example of "invalid" code when developers duplicate useUnit calls without using destructuring, common copy-pasting. So we ought to provide a suggestion for this simplest case.

With mixed shapes - Array + Object case we can also provide a suggestion by treating

const [products] = useUnit([CartModel.$cartProducts]); // as equivalent to const { products } = useUnit({ products: CartModel.$cartProducts });

and merging with other destructuring patterns.

This case will always provide a suggestion unless either a rest property is used or variable names are duplicated in scope.

Using aliases adds a layer of complexity, however the accepting identifier is bound to be unique due to scoping rules, so

const { a: b } = useUnitEffector({ a: CartModel.$cartProducts }); // can be treated as const { b } = useUnitEffector({ b: CartModel.$cartProducts });

for the sake of suggestion generation.

So the most notably here are the cases we aren't able to provide a suggestion:

@@unitShape and useUnit(model) – ignored by the rule due to unsound combination,

duplicate identifiers to prevent invalid JS from being generated, or

rest property and spread operator since we can't track property -> unit mapping.

Other cases should theoretically provide a valid suggestion.

There's also a useUnit ordering problem since it can be applied to dynamically acquired Units, see example above with useContext. So a question is how would we treat the following code?

const a = useUnit($a) // (1) const $b = useContext(MyModel) // (2) const b = useUnit($b) // (3)

Should those be kept separate, suppressing the warning, or do we generate a suggestion for

const $b = useContext(MyModel) const { a, b } = useUnit({ a: $a, b: $b })

And if we do so, how do we track if a has been used between (1) and (3)?

It seems even for basic cases this rule would also unfortunately require variable scope analysis to detect if units are declared in a current scope or a surrounding scope to prevent issues with temporal dead zone.

kireevmp · 2026-04-16T13:27:54Z

+  },
+} as const
+
+function getTypeFromChecker(node: Node.Expression, services: ParserServices): UnitType {


Let's use a shared typecheck helper is in this rule instead of custom checks?

Cool! Found it, using it! :)

kireevmp · 2026-04-16T13:32:27Z

+  }
+}
+
+function scoreTypes(types: UnitType[]): UnitType {


I'm not sure why we need to score something here and pre-group? Would it be possible to just collect the list of mappings unit -> local ident that is grouped later, with no regard of who was first or second, completely regenerating all useUnit calls?

I agree, it looks unreliable. I'll see what I can do.

… and plain calls

kireevmp

Hey @Olovyannikov – thanks for taking the time to come back to polish. I see good progress here, I think there's a solid foundation now with normalization concept in place. What's left is to apply it consistently everywhere across the rule, to actually use that structure. The mental model here seems to be correct, which is great news.

If you're up to do another pass, I'd appreciate you also reviewing the test suite. Due to large rule size, under-employed normalization and ultimately duplicated code paths, the coverage is below 80% and the suite may not cover every corner case, something we really need here to be sure rule is sound.

kireevmp · 2026-06-12T20:17:55Z

+type Fixer = RuleFixer
+type SourceCode = Context["sourceCode"]
+type Scope = ReturnType<SourceCode["getScope"]>


Why do we need to compute these aliases when plain imports are available?

Fixer => RuleFixer SourceCode => TSESLint.SourceCode Scope => TSESLint.Scope.Scope

Let's strive to use built-ins as much as possible, without reinventing the wheel 🙏

kireevmp · 2026-06-12T20:18:50Z

+    shape: "ObjectExpression.arguments",
+    list: "ArrayExpression.arguments",
+  },
+} as const


Why did you think to add as const? It does not seem to be relevant at all for type system here.

kireevmp · 2026-06-12T20:23:27Z

+/** Unit expression nodes of a call, regardless of whether it can be normalized (used for hoisting analysis). */
+function unitExpressionNodes(call: UseUnitCall): Node.Expression[] {
+  const argument = call.init.arguments[0]
+  if (!argument || argument.type === NodeType.SpreadElement) return []


Consider narrowing the selector on ESLint side to only include useUnit calls with Object, Array expressions or Identifier as first argument. May save couple of ifs across the rule.

kireevmp · 2026-06-12T20:24:29Z

+  if (argument.type === NodeType.ArrayExpression) {
+    return argument.elements.filter((el): el is Node.Expression => el !== null && el.type !== NodeType.SpreadElement)
+  }
+  if (argument.type === NodeType.ObjectExpression) {
+    return argument.properties
+      .filter((p): p is Node.Property => p.type === NodeType.Property)
+      .map((p) => p.value as Node.Expression)
+  }


Let's avoid explicit type guards since TS can faithfully infer those now, modern style. These now only add extra noise.

kireevmp · 2026-06-12T20:25:43Z

+  if (argument.type === NodeType.ObjectExpression) {
+    return argument.properties
+      .filter((p): p is Node.Property => p.type === NodeType.Property)
+      .map((p) => p.value as Node.Expression)


Can we avoid a type cast? Looking at code, there's no guarantee made that non-expression arguments are excluded, so this is not sound.

kireevmp · 2026-06-12T21:15:27Z

+  if (argument.type === NodeType.ObjectExpression) {
+    const properties = argument.properties.filter((p): p is Node.Property => p.type === NodeType.Property)
+
+    const byType = {
+      store: properties.filter((p) => getNodeType(p.value as Node.Expression) === "store"),
+      event: properties.filter((p) => getNodeType(p.value as Node.Expression) === "event"),
+      effect: properties.filter((p) => getNodeType(p.value as Node.Expression) === "effect"),
+    }
+
+    const toKeyName = (p: Node.Property) =>
+      p.key.type === NodeType.Identifier ? p.key.name : sourceCode.getText(p.key)
+
+    const lines = Object.values(byType)
+      .filter((group) => group.length > 0)
+      .map(
+        (group) =>
+          `const { ${group.map(toKeyName).join(", ")} } = ${calleeName}({ ${group.map((p) => sourceCode.getText(p)).join(", ")} })`,
+      )
+
+    return [fixer.replaceText(call.statement, lines.join(`\n${indent}`))]


This seems to eventually just drop unknown properties from useUnit, potentially generating invalid destructive code. Consider, for some reason, we couldn't detect type of a unit (let's say it resolved to any). Under no circumstances can we delete useUnit($somethingThatIsAny) – best-case we ought to filter it out and keep as-is, worst-case – do nothing with this useUnit.

kireevmp · 2026-06-12T21:30:13Z

+      ...(mergeable
+        ? {
+            suggest: [
+              {
+                messageId: "multipleUseUnit",
+                fix: (fixer) => generateMergeFix(fixer, captured, context),
+              },
+            ],
+          }
+        : {}),


Suggested change

...(mergeable

? {

suggest: [

{

messageId: "multipleUseUnit",

fix: (fixer) => generateMergeFix(fixer, captured, context),

},

],

}

: {}),

suggest: mergeable

? [{ messageId: "multipleUseUnit", fix: (fixer) => generateMergeFix(fixer, captured, context) }]

: undefined,

kireevmp · 2026-06-12T21:31:49Z

+        if (!arg || arg.type === NodeType.SpreadElement) return
+        // Only non-destructuring calls over a single unit can be merged; a collection argument
+        // (`useUnit([...])` / `useUnit({...})`) or a non-unit argument (e.g. @@unitShape) is ignored.
+        if (arg.type === NodeType.ArrayExpression || arg.type === NodeType.ObjectExpression) return


Consolidate paths - if you're triggering on all calls after all, there's no reason to have three selectors if one suffices.

kireevmp · 2026-06-12T21:32:33Z

    "prefer-useUnit": preferUseUnit,
+    "prefer-single-binding": preferSingleBinding,


Ordering

Suggested change

"prefer-useUnit": preferUseUnit,

"prefer-single-binding": preferSingleBinding,

"prefer-single-binding": preferSingleBinding,

"prefer-useUnit": preferUseUnit,

kireevmp · 2026-06-12T21:33:34Z

+const style = {
+  "effector/prefer-single-binding": "warn",
+} satisfies TSESLint.Linter.RulesRecord


Let's postpone adding a whole new style preset for now, please. Adding a preset is easy, removing not so much. I'd rather be cautious here, if you allow that.

…inding Collapse the three parallel traversals (extractBindings, unitExpressionNodes, generateSeparationFix) into one analyze() pass that yields a single primitive: an identifier bound to the unit expression passed to useUnit. Every helper now consumes that structure, and source text is read only while building a fix. Resolve the soundness issues raised in review: - module units declared below the component are no longer a false TDZ hazard; only locals declared at/after the anchor block hoisting - multi-declarator statements, let/var and type-annotated bindings are reported without a suggestion instead of producing a crashing or invalid fix - the split fix keeps units whose type is undetermined instead of dropping them - duplicate destructuring keys no longer silently drop a binding - merging finds mergeable calls below one that cannot be hoisted - enforce now merges same-type calls as well as splitting mixed ones Drop the computed type aliases, the unsound `as Expression` cast and the redundant `as const`/type guards in favour of the built-in ts-eslint types. Tests cover the new edge cases; branch coverage rises from 79% to 89%.

…e cases Add a comparison of forbid/allow/enforce code generation, spell out which calls are ignored (@@unitShape, non-unit args, temporal dead zone) and which are reported without a suggestion (multi-declarator, let, type annotations, etc.).

prefer-single-binding stays available as effector/prefer-single-binding but is no longer bundled in a preset — removing a preset after release is painful, so hold off on adding one. Also order prefer-single-binding before prefer-useUnit.

Olovyannikov force-pushed the new/prefer-single-binding branch 4 times, most recently from 7edd618 to 15906fe Compare December 14, 2025 11:20

Olovyannikov force-pushed the new/prefer-single-binding branch 2 times, most recently from e12cac1 to 82797c1 Compare March 4, 2026 08:10

Olovyannikov force-pushed the new/prefer-single-binding branch from 82797c1 to 89651b2 Compare April 10, 2026 08:26

kireevmp mentioned this pull request Apr 13, 2026

feat(enforce-exhaustive-useUnit-destructuring): add new rule for destructured units #175

Merged

Olovyannikov changed the title ~~feat(prefer-single-binding): add rule~~ [WIP] feat(prefer-single-binding): add rule Apr 13, 2026

Olovyannikov added 5 commits April 14, 2026 11:05

feat(prefer-single-binding): add rule

3f52c18

feat(prefer-single-binding): add separate events and stores flag

41d0274

docs(prefer-single-binding): update docs

a5954d1

feat(prefer-single-binding): update to v17

8f2502d

fix(files): remove legacy empty files

98afa07

Olovyannikov force-pushed the new/prefer-single-binding branch from 89651b2 to 25440ef Compare April 14, 2026 09:46

feat: add type-checker for detect unit type

c157176

Olovyannikov force-pushed the new/prefer-single-binding branch from 25440ef to c157176 Compare April 14, 2026 09:48

Olovyannikov changed the title ~~[WIP] feat(prefer-single-binding): add rule~~ feat(prefer-single-binding): add rule Apr 14, 2026

kireevmp reviewed Apr 16, 2026

View reviewed changes

Olovyannikov requested a review from kireevmp April 16, 2026 18:27

refactor: add string flag, merge selectors, create new group 'style'

2898b5b

Olovyannikov force-pushed the new/prefer-single-binding branch from a7b21d9 to 2898b5b Compare April 17, 2026 08:11

feat(prefer-single-binding): detect local names doubles, tdz, aliases…

36e3e17

… and plain calls

kireevmp reviewed Jun 12, 2026

View reviewed changes

Olovyannikov added 3 commits June 15, 2026 11:18

Olovyannikov requested a review from kireevmp June 15, 2026 08:19

		"prefer-useUnit": preferUseUnit,
		"prefer-single-binding": preferSingleBinding,

Conversation

Olovyannikov commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why is this important?

Performance

Code clarity

Example

Separate stores and events calls

Example

Uh oh!

netlify Bot commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for eslint-plugin ready!

Uh oh!

Olovyannikov commented Apr 14, 2026

Uh oh!

kireevmp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kireevmp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Olovyannikov commented Dec 14, 2025 •

edited

Loading

netlify Bot commented Dec 14, 2025 •

edited

Loading