effector / eslint-plugin

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

Rule: `no-mutation-in-pure-functions` #6

Open igorkamyshev opened 3 years ago

igorkamyshev commented 3 years ago

Some functions in effector API should be pure:

We cannot prove the purity functions by ESLint rule, but we can forbid mutations in these cases.

AlexandrHoroshih commented 3 years ago

I'm not sure i understand - how it is supposed to work?

For e.g., i think, this kind of mutation would be entierly correct:

(state, payload) => {
 const copyState = deepCopy(state);
 const copyPayload = deepCopy(payload);

 copyPayload.foo = "bar";
 copyState.some = copyPayload;

return copyState;
}

state & payload are available for other reducers and stores - mutating them is not correct, because this change will be observable. But by creating a own copy in reducer this is no longer problem, since mutation of copy is not observable from the outside.

Another problem is that there is no way to proof that this copy = deepCopy(state) actually does not mutate the state 🤔

Looks like, it is only options that are secured from false warnings, are:

  1. Direct mutations of arguments (since reducer is not the only consumer of store state and event payload)
  2. Mutations of external objects
AlexandrHoroshih commented 3 years ago

Also there is another case that is affects "purity" of functions - reading from outer state that can be changed during app lifetime. This is a side-effect and can lead to observable changes in function behaviour, basically, by making it stateful

const box = { ... };
const RED = 'red';

$store.on(event, (state, payload) => {
 // all of this this is reading from outer state which is side-effect
 // and may lead to observable changes in function behaviour
 const date = new Date()
 const otherState = $another.getState()

 // `box` is not readonly or frozen object, we cannot prove that is not mutable
 // should be warned, i guess
 const someData = box[payload];

 // RED is a immutable primitive constant, will never change
 // this should not be warned, i guess
 const nextState = {...state, color: RED}

})