Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule: prefer-clean-model-structure #114

Open
sergeysova opened this issue Aug 10, 2022 · 7 comments
Open

Rule: prefer-clean-model-structure #114

sergeysova opened this issue Aug 10, 2022 · 7 comments
Labels

Comments

@sergeysova
Copy link
Member

sergeysova commented Aug 10, 2022

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 sergeysova added the RFC label Aug 10, 2022
@sergeysova
Copy link
Member Author

the committee decided it was useless

@sergeysova sergeysova closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2022
@igorkamyshev
Copy link
Member

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

@igorkamyshev igorkamyshev reopened this Aug 10, 2022
@ilyaagarkov
Copy link
Member

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
Copy link
Member

ilyaagarkov commented Aug 10, 2022

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)

@igorkamyshev
Copy link
Member

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.

@sergeysova
Copy link
Member Author

sergeysova commented Aug 11, 2022

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

@igorkamyshev
Copy link
Member

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 igorkamyshev changed the title Rule: 'prefer-clean-model-structure` Rule: prefer-clean-model-structure Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants