Let effects return the `actions` object

jorgebucaran commented 7 years ago

Actions are functions you call like actions.myAction(data). When you call an action, HyperApp checks if it belongs to effects or reducers, then:

for reducers
- applies the reducer to update the model, newModel = reducer(oldModel, data)
for effects
- calls the effect effect(data)

Finally, the action function returns (undefined).

The proposal is, amend the previously described behavior, and return the actions object instead.

This would allow chaining effects / reducers.

Effects

Effects, can be asynchronous, so we must return a Promise to make sure they are executed in order.

actions.myReducer1().myReducer2().myEffect1().then(actions.myEffect2).then(actions.myEffect3)

app({
    effects: {
       bigEffect: async (model, actions, data) => {
           actions.myReducer1().myReducer2()
           await actions.myEffect1()
           await actions.myEffect2()
           actions.myReducer3().myReducer4()
        }
    }
})

Reducers

Reducers are sync, so there's no need to return a promise in this case.

actions.myReducer1().myReducer2()

Notes

Chaining reducers should be limited to reducers, otherwise you'd end up using them like an effect which is wrong. Why? Because reducers cause the view to be re-rendered.
There's no need to re-render after each reducer, so this mechanism should try to be efficient. Also related: https://github.com/hyperapp/hyperapp/issues/90

FlorianWendelborn commented 7 years ago

I see no downside in doing this. I'd say it's better to allow it for both effects and reducers though since the line between both is quite blurry and in my experience you often call a single effect which then calls multiple reducers.

tunnckoCore commented 7 years ago

i'm +1 for allow it just for reducers, it feels good, but not for effects since they are async and dont make any sense

FlorianWendelborn commented 7 years ago

@tunnckoCore they might be async or just call other actions synchronously.

Might be nice to return a promise instead so you can .then it. Would even work for reducers.

jorgebucaran commented 7 years ago

I am starting to agree with @tunnckoCore here.

But, I'd still like to play a bit with the original idea to see where that takes me.

tunnckoCore commented 7 years ago

dodekeract, not make sense to call other actions synchronously, it just won't feels good and will be against elm's architecture principles and will lead to more problems. it's easier to just say that they are async actions and nothing more, no unexpected and different behaviours. they just must strictly be async.

and if that is architecture design and pattern of elm, it is for reason for sure. it is better to try to match it as much as possible in js land with no key diffs.

one more point is that, that architecture and style is kinda selling point of choo and hyperapp. users fall in love for very short time because its simplicity and clean design.

also, it feels pretty natural.

i'm still agree to enable it for reducers only, since it leads to nothing different and won't have any significant impact

rbiggs commented 7 years ago

I think sticking as closely as possible to the Elm Architecture is the biggest selling point, otherwise it will become like CycleJS.

jorgebucaran commented 7 years ago

@rbiggs Hehe.

@tunnckoCore The main benefit I saw here, was the oneliner actions.reducer1().reducer2()...reducerN(). Like you point out, it doesn't make sense for effects without some hacky internal juggling.

Why not just actions.reducer1() && actions.reducer2() && actions.reducerN() then?

lukejacksonn commented 7 years ago

Will this mean that running actions.reducer1().reducer2() will only cause the model to update once?

jorgebucaran commented 7 years ago

It shouldn't if we're using requestAnimationFrame, but TBH I don't know if I'd like to proceed with this feature, it seems it creates more of an anti-pattern than anything else.

The thing, sometimes you'd like to run several reducers, so instead of:

actions.one()
actions.two()
actions.three()

you could do:

actions.one().two().three()

Effect actions, return what your effect function returns, so you can return a promise which is fine.

tunnckoCore commented 7 years ago

will only cause the model to update once?

@lukejacksonn don't think so, it will update it two times, i believe.

jorgebucaran commented 7 years ago

This landed in @0.6.0 so closing.

jorgebucaran / hyperapp