effector / eslint-plugin

Enforcing best practices for Effector
https://eslint.effector.dev
MIT License
90 stars 16 forks source link

Rule: `no-obvious-cycle` #89

Open igorkamyshev opened 2 years ago

igorkamyshev commented 2 years ago

I've found a code in some codebases like this:

forward({ from: submit, to: submit });

Of course, it leads to bug. We can detect such cases in linter.

ainursharaev commented 2 years ago

I've discussed this rule in the effector's chat, and many people voiced their concerns that this rule is not entirely worth implementing.

  1. Simple cases like forward({ from: submit, to: submit }); are easy to detect at the early stages.
  2. Not all cases can lead to a cycle, for example sample({ clock: foo, filter, fn, target: foo })
  3. Some cases are really hard to detect, for example
const foo = createEvent();
const $bar = createStore("bar");

const foobar = sample({
  clock: foo,
  source: $bar
})

// somewhere in another module

sample({
  clock: foobar,
  target: foo
})
igorkamyshev commented 2 years ago

I do not get it 🤔

Original example (which I saw in real codebase) in the issue is definitely a buggy code, and we can detect it statically.

forward({ from: submit, to: submit });

Why should we remove this rule? Of course, it cannot detect any bug, as well as any other rule. For me, it is not a reason to remove the whole rule.

This plugin does not aim to fix only "smart" bugs, it is trying to help developers introduce fewer bugs. So, this rule would help to find "stupid" bug earlier, event before build.