FormidableLabs / freactal

Clean and robust state management for React and React-like libs.
MIT License
1.65k stars 46 forks source link

API change Proposal #61

Closed etienne-dldc closed 5 years ago

etienne-dldc commented 7 years ago

Hi ✋ ,

I've been thinking about my problem (see #59 ) and I have a proposal to solve this.

Some vocabulary

First I think it might be useful to clarify some vocabulary :

modifier : (prevState) => nextState

A modifier take a state and return a state ¯_(ツ)_/¯.

Example :

const counterIncrement = (state) => ({ ...state, counter: state.counter + 1 })

softModifier

It might be useful to create a helper like this :

const softModifier = (newStateCreator) => (state) => ({ ...state, ...(newStateCreator(state)) })

// And then to use it :

const counterIncrement = softModifier(state => ({ counter: state.counter + 1 }))

modifierCreator : (...params) => modifier

A modifierCreator is just a function that return a modifier.
Example :

const counterIncrementBy = (amount) => (state) => ({ ...state, counter: state.counter + amount })

// same as

const counterIncrementBy = (amount) => softModifier(state => ({ counter: state.counter + amount }))

effect : (effects, state) => Promise(modifier)

An effect would be similar to what it's now but it will not accept additional arguments and instead it will take the current state as second argument. Just like in current API the result would be wrapped in a Promise if necessary so we don't have to create a promise for synchronous effects.

Example :

const incrementCounter = (effects, state) => counterIncrement;

// same as
const incrementCounter = (effects, state) => (state) => ({ ...state, counter: state.counter + 1 });

// you can also use `modifierCreator` :
const incrementCounterBy2 = (effects, state) => counterIncrementBy(2);

effectCreator = (...params) => affect

Finally an effectCreatoris a function that return a effect. This is what you would use to pass argument to effect. Example:

const updateCounterBy = (amount) => (effects, state) => counterIncrementBy(amount);

// but we can omit effects and state here
const updateCounterBy = (amount) => () => counterIncrementBy(amount);

API Change Proposal

The idea is that instead of declaring effects like in the current API we would declare effectCreator.

// we would replace this :
updateCounterBy: (effects, addVal) => state => ({ ...state, counter: state.counter + addVal })

// by this
updateCounterBy: (addVal) => () => state => ({ ...state, counter: state.counter + addVal })

A note about composition

An interesting consequence of modifiers is that it's easy to compose then :

const incrementTwice = () => () => compose(counterIncrement, counterIncrement);
// we might want some helper so we can write
const incrementTwice = modifierEffect(compose(counterIncrement, counterIncrement));

Waiting for you feedback about this 🤔 I could create a PR if you like the idea 😃

divmain commented 7 years ago

I'm going to think a bit more about this. You bring up some interesting ideas but, although your proposed change can handle synchronous state transformations just fine, it would introduce issues with async state transformations.

I do have a couple of other ideas for how to expose state to the effects in natural ways. I'm going to leave this issue open as a place-holder.

etienne-dldc commented 7 years ago

For me, it could introduce issues with async state transformations only if the state provided by the effect is used in a modifier but that would be a miss use because modifierCreator should be isolated.

To illustrate

// Wrong
myEffect: () => (effects, state) => () => ({ ...state }}

// Correct
myEffect: () => (effects, oldState) => (state) => ({ ...state }}

// Or if you need something from the oldState
myEffect: () => (effects, oldState) => createMyModifierWithOldState(oldState)

I agree that this might be tricky and easy to make mistake but maybe we can find a way to prevent then or make good documentation (not convince myself ^^).

Another solution would be to declare modifierCreators in provideState :

provideState({
  modifiers: {
    // modifiers are modifiersCreators
    incrementBy: (amount) => (state) => ({ ...state, counter: state.counter + amount }),
    // you can access other modifiers here
    increment: () => (state, modifiers) => modifiers.incrementBy(1)
  },
  effects: {
    // effects are effectCreators
    incrementFromApi: () => (effects, modifier, state) => (
      api.howManyDoINeedToIncrement().then((amount) => modifier.incrementBy(amount))
    )
  }
})

One more example using state provided by effect :

provideState({
  identity: () => (state) => state,
  initialState: () => ({ counters: [0, 0, 0], currentCounterIndex: 2 }),
  modifiers: {
    incrementBy: (counterIndex, amount) => (state) => ({
      ...state,
      counter: state.counter.map(
        (val, index) => (index === counterIndex) ? val + amount : val) }
      )
    }),
    incrementCurrentBy: (oldCounterIndex, amount) => (state, modifiers) => (
      // if state.currentCounterIndex is no longer oldCounterIndex do nothing
      state.currentCounterIndex === oldCounterIndex ? modifier.incrementBy(state.currentCounterIndex, amount) : modifier.identity()
    )
  },
  effects: {
    // Note that we don't pass any parameters to this, no need to know what is the currentCounter !
    incrementCurrentCounterFromApi: () => (effects, modifiers, state) => (
      api.howManyDoINeedToIncrement(state.currentCounterIndex).then(
        (amount) => modifier.incrementCurrentBy(state.currentCounterIndex, amount)
      )
    )
  }
})
bingomanatee commented 6 years ago

this might seem creepy but: what if the "this" in effects was the state? Because in the current API it doesn't have a deterministic notion of "this" you wouldn't have to change the signature of an effect but you could still pull state values from "this" into effects going forward.