eslint-plugin icon indicating copy to clipboard operation
eslint-plugin copied to clipboard

Rule: `prefer-clean-model-structure`

Open sergeysova opened this issue 3 years ago • 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;
}

sergeysova avatar Aug 10 '22 11:08 sergeysova

the committee decided it was useless

sergeysova avatar Aug 10 '22 11:08 sergeysova

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

igorkamyshev avatar Aug 10 '22 12:08 igorkamyshev

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

ilyaagarkov avatar Aug 10 '22 13:08 ilyaagarkov

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)

ilyaagarkov avatar Aug 10 '22 13:08 ilyaagarkov

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.

igorkamyshev avatar Aug 11 '22 07:08 igorkamyshev

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

sergeysova avatar Aug 11 '22 10:08 sergeysova

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.

igorkamyshev avatar Aug 11 '22 11:08 igorkamyshev