effector/eslint-plugin

Rule: `prefer-clean-model-structure`

sergeysova opened this issue · 7 comments

This rule should improve the readability of the code.

The rule should check the order of definitions of units:

  1. First of all define domains
  2. Define events
  3. After events define derived events
  4. Next define stores
  5. After simple stores define derived stores
  6. Next define effects
  7. After effects define derived effects (attach)
  8. After all definitions write logic on sample's
  9. Do not mix custom mapper/functions with samples and definitions, move them down

This rule should ban using .on and .reset etc. methods on stores immediately after definition. The same for domains, effects, and events.

This rule should not be auto-fixable, because it's affects business-logic.

FAIL

// prefer-clean-model-structure: 'error'

const someHappened = createEvent()
const $data = createStore(0)
  .on(someHappened, (c) => c + 1)

const runMeFx = createEffect(() => {})

function calculate(a, b) {
  return a + b;
}

sample({ clock: someHappened, target: runMeFx })

export const anotherFx = attach({
  source: $data,
  async effect(data, arg) {
    return calculate(data, arg)
  },
})

sample({ clock: anotherFx.doneData, target: someHappened })

OKAY

// prefer-clean-model-structure: 'error'

const someHappened = createEvent()

const $data = createStore(0)

const runMeFx = createEffect(() => {})

export const anotherFx = attach({
  source: $data,
  async effect(data, arg) {
    return calculate(data, arg)
  },
})

$data.on(someHappened, (c) => c + 1)

sample({ clock: someHappened, target: runMeFx })

sample({ clock: anotherFx.doneData, target: someHappened })

function calculate(a, b) {
  return a + b;
}

the committee decided it was useless

In my humble opinion, this rule can be useful in numerous instances. I suggest implementing in and waiting for community feedback.

It looks like you missed one selction between (or I am missing )

6. After effects define derived effects (attach)
7. After all definitions write logic on sample's

stores subscribing on events/effect section should be there

btw threre also could be spliing to exported/no-exported inside each section
It could improve readability of public API

bad

const firstEvent = createEvent();
export const firstExporedEvent = createEvent();
const secondEvent = createEvent();
export const secondExportedEvent = createEvent();

const store1 = createStore(0)
export const exportedStore1 = createStore(0)
const store2 = createStore(0)
export const exportedStore2 = createStore(0)

good

export const firstExporedEvent = createEvent();
export const secondExportedEvent = createEvent();
const firstEvent = createEvent();
const secondEvent = createEvent();

export const exportedStore1 = createStore(0)
export const exportedStore2 = createStore(0)
const store1 = createStore(0)
const store2 = createStore(0)

btw threre also could be spliing to exported/no-exported inside each section It could improve readability of public API

In my view, export preferences does not relate to this rule, I suppose it's a lot ESLint rules to define where developers should write their exports.

a lot ESLint rules to define where developers should write their exports.

It is not about "where" write exports, but about how to sort regular unit definitions and exported definitions

I still do not get it. How does it relate with Effector?

We can use something like https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/group-exports.md for this purpose.

I mean, it is a good idea not to mix exported and non-exported entities in any case, not only Effector-units.