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

An alternative side effect model based on Generators and Sagas #1139

Closed
yelouafi opened this Issue Dec 14, 2015 · 36 comments

Comments

8 participants
@yelouafi

yelouafi commented Dec 14, 2015

There were many discussions (most of them are closed some time ago) about side effects and their relations to the pure Redux model based on actions/reducers. Here i'd like to present an alternative model for Side Effects based on the ideas discussed on the older posts.

redux-saga is a middleware centered around the notion of Saga (inspired but not sure if strictly conform to the Saga pattern). So just as stated on the docs : reducers are used for state transition, and Sagas aer used to orchestrate the flow of operations.

The model tries the gather some pertinent ideas from the precedent discussions

  • Declarative Side Effects makes the code easily testable provided they do not introduce a significative boilerplate (dedicated constants, actions, effect handlers)
  • Generators are the simplest/most powerful way to describe asynchronous operations using the well known control-flow statements (while, for, if).

So basically a Saga is generator function that yields effects and gets the corresponding responses. I know this was already explored and discussed, but the plus of the model is the definition of the Effects themselves: Side Effects are not simply server updates, dom storage, navigation ...etc. A side effect is anything that needs to be done imperatively at a specific point of time (i.e. is all about ordering things), so waiting for user actions, and dispatching actions to the store are also considered side effects in the model.

function* incrementAsync(io) {
  while(true) {
    // wait for each INCREMENT_ASYNC action  
    const nextAction = yield io.take(INCREMENT_ASYNC)

    // call delay : Number -> Promise
    yield delay(1000)

    // dispatch INCREMENT_COUNTER
    yield io.put( increment() )
  }
}

The point of using Generators is not simply to provide some syntactical benefits; it's that in the asynchronous world we're sill in the goto age. Nobody would say now that Structured programming just provides Syntactical benefits over the old goto style (although some people used to say that in those older times).

another important point i that Generators/Sagas are composables (either via the builtin yield* or the generic middleware composition mechanism) which means you can create reusable Effects and compose them with other effects (timeouts, future actions...) using parallel or race combinators.

There were some side discussions on whether we should embed side effect code inside the pure code, or the inverse; I think both are 2 different views of the same principle behind side effects which is the order/interleaving of things: the application starts with a side effect, then the rest must be a well defined order of pure/effectful computation. The essential is to keep the 2 separated, and clearly define your breakpoints (actions, state updates, ...)

I'm still experimenting with the model and how to extend it, like adding monadic operations (merging, zipping or concatenating 2 Generators, just like those found in Reactive Streams). Any comments/critics are welcome of course

The project emerged actually from a lengthy discussion; The idea of Saga was introduced by @slorber. I was initially enthusiast about the Elm model but ended up with a conviction that code that must manage the order of things (operations) should be kept separated from code that compute the app state

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Dec 14, 2015

Contributor

I don't have the time to look into this closely but on a first glance this sounds very sane. Can you try porting the Redux examples in this repo to this model so we can see how it works in real apps and compare the code?

Contributor

gaearon commented Dec 14, 2015

I don't have the time to look into this closely but on a first glance this sounds very sane. Can you try porting the Redux examples in this repo to this model so we can see how it works in real apps and compare the code?

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Dec 14, 2015

Contributor

Maybe @jlongster, @ashaffer, and @acdlite might want to take a look at this too.

Contributor

gaearon commented Dec 14, 2015

Maybe @jlongster, @ashaffer, and @acdlite might want to take a look at this too.

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 14, 2015

@gaearon there are already 2 examples ported : counter and shopping-cart; although there is no complex operations in the examples (added a simple example of onBoarding to the counter example; i'll try to port other examples later; it was on the roadmap anyway to provide more examples.

I think we need some use cases involving non trivial operations (i mean other than simple reactions) that still can fit into small examples, perhaps some use case that someone found difficult to implement using thunks.

yelouafi commented Dec 14, 2015

@gaearon there are already 2 examples ported : counter and shopping-cart; although there is no complex operations in the examples (added a simple example of onBoarding to the counter example; i'll try to port other examples later; it was on the roadmap anyway to provide more examples.

I think we need some use cases involving non trivial operations (i mean other than simple reactions) that still can fit into small examples, perhaps some use case that someone found difficult to implement using thunks.

@ashaffer

This comment has been minimized.

Show comment
Hide comment
@ashaffer

ashaffer Dec 14, 2015

Contributor

Hmm...i'm not sure I understand what advantages this approach confers over say, redux-thunk or redux-effects. It seems like it's somewhere in between the two and also uses generators. (redux-gen is a composition middleware for redux-effects that does the same, and so would give you similar syntactic benefits over the bind syntax of redux-effects.

Would you mind providing a comparison of redux-saga to each of these? @yelouafi

EDIT: In thinking about it a bit more, it seems like the core opinion of this approach is that action creators are the wrong place for effectful logic (even if the actual effects are handled elsewhere)?

Contributor

ashaffer commented Dec 14, 2015

Hmm...i'm not sure I understand what advantages this approach confers over say, redux-thunk or redux-effects. It seems like it's somewhere in between the two and also uses generators. (redux-gen is a composition middleware for redux-effects that does the same, and so would give you similar syntactic benefits over the bind syntax of redux-effects.

Would you mind providing a comparison of redux-saga to each of these? @yelouafi

EDIT: In thinking about it a bit more, it seems like the core opinion of this approach is that action creators are the wrong place for effectful logic (even if the actual effects are handled elsewhere)?

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 14, 2015

@ashaffer the main difference as you said is that sagas are not fired in an action creator. Instead they are like daemon processes that pull the future actions.

The other important difference is the broader definition of side effects. Like I said effects are more about 'things that needs to happen at a specific moment or in a specific order'. So there is no difference beteween a future action , a future timeout or the result of a dispatch call. The saga itself is an effect. So you can compose all those things together

As a consequence you can implement logic that spans across multiple actions. With action creators you ll have to implement a kind of state machine and explicitly store the AC state even if it is irrelevant to the view. With generators the state is either implicit by the control flow statements or explicit but local to the saga.

As redux-effects, redux-saga favors a declarative approach by separating effect creation from effect execution. But tries to not get in the way. You can use io.call(fn, ...args) to create function call effects without using the middleware handler approach. But you can also follow the redux-effects approach by yielding io.put(effectAsAction) and automatically handling the promise response from the handler middleware. You can also yield promises directly if you want. I choose to not place any restriction here and leave the choice to the developper.

yelouafi commented Dec 14, 2015

@ashaffer the main difference as you said is that sagas are not fired in an action creator. Instead they are like daemon processes that pull the future actions.

The other important difference is the broader definition of side effects. Like I said effects are more about 'things that needs to happen at a specific moment or in a specific order'. So there is no difference beteween a future action , a future timeout or the result of a dispatch call. The saga itself is an effect. So you can compose all those things together

As a consequence you can implement logic that spans across multiple actions. With action creators you ll have to implement a kind of state machine and explicitly store the AC state even if it is irrelevant to the view. With generators the state is either implicit by the control flow statements or explicit but local to the saga.

As redux-effects, redux-saga favors a declarative approach by separating effect creation from effect execution. But tries to not get in the way. You can use io.call(fn, ...args) to create function call effects without using the middleware handler approach. But you can also follow the redux-effects approach by yielding io.put(effectAsAction) and automatically handling the promise response from the handler middleware. You can also yield promises directly if you want. I choose to not place any restriction here and leave the choice to the developper.

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 16, 2015

ported the async(reddit) example last night; a small but an interesting case because of concurrent requests. A quick comparison with the original thunk-based version

  • The initial fetch: In the thunk based version, is fired from inside in App's componentDidMount. In the saga based version, it's handled by the startup Saga which yields to the sub-(co)routine fetchPosts then finishes.
  • reddit selection changes: In the thunk based version, is handled within App's componentwillReceiveProps which checks if the selected reddit has changed then fires the async AC fetchPostsIfNeeded. In the saga based version, App just dispatches the SELECT_REDDIT action, which is watched by the nextRedditChange saga.
  • invalidateReddit watches for INVALIDATE_REDDIT events then yields to the fetchPosts subroutine. In the thunk based version it's in the App's handleRefreshClick callback which fires 2 consecutive actions: invalidateReddit then fetchPostsIfNeeded
  • Moved out didInvalidate state from the store since it's only used by the fetchPostsIfNeeded AC to handle its control flow so no relevant to the view.
  • nextRedditChange calls io.race([fetchPosts, true]) to both yield the fetch request and resumes immediately (because true will be resolved right away). This is a kind of a spawn, otherwise it'll be blocked on the request call and will miss in-between SELECT_REDDIT events; this is a subtle detail because in the thunk version, async AC are called automatically on each event and don't miss any one; but on the other hand, this makes some tasks like handling out-of-order response more difficult.

yelouafi commented Dec 16, 2015

ported the async(reddit) example last night; a small but an interesting case because of concurrent requests. A quick comparison with the original thunk-based version

  • The initial fetch: In the thunk based version, is fired from inside in App's componentDidMount. In the saga based version, it's handled by the startup Saga which yields to the sub-(co)routine fetchPosts then finishes.
  • reddit selection changes: In the thunk based version, is handled within App's componentwillReceiveProps which checks if the selected reddit has changed then fires the async AC fetchPostsIfNeeded. In the saga based version, App just dispatches the SELECT_REDDIT action, which is watched by the nextRedditChange saga.
  • invalidateReddit watches for INVALIDATE_REDDIT events then yields to the fetchPosts subroutine. In the thunk based version it's in the App's handleRefreshClick callback which fires 2 consecutive actions: invalidateReddit then fetchPostsIfNeeded
  • Moved out didInvalidate state from the store since it's only used by the fetchPostsIfNeeded AC to handle its control flow so no relevant to the view.
  • nextRedditChange calls io.race([fetchPosts, true]) to both yield the fetch request and resumes immediately (because true will be resolved right away). This is a kind of a spawn, otherwise it'll be blocked on the request call and will miss in-between SELECT_REDDIT events; this is a subtle detail because in the thunk version, async AC are called automatically on each event and don't miss any one; but on the other hand, this makes some tasks like handling out-of-order response more difficult.
@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 16, 2015

@ashaffer

Would you mind providing a comparison of redux-saga to each of these?

after looking at the mentioned project i'd say

  • redux-gen improves over redux-thunk with the linear/structured control flow provided by generators. But it suffers from the same limitations of being fired from within Action Creators. So, to handle logic that spans multiple actions it has to store its control state inside the store (the FSM approach).
  • redux-effects seems to focus more on the declarative approach to side effects than the control flow itself, which can be delegated to other libs like declarative-promise or redux-gen. IMO this is a good point, because of the ability to pre-process effects-as-objects before they get executed by the dedicated middlewares, as well as better testability.

But having actually discussed with @tomkis1 who already tried this approach in real production; it seems there are some issues inherent to this approach

I think redux-effects and redux-saga address 2 complementary concerns. I think redux-saga can also be used as a composition middleware for redux-effects (unless I'm mistaken) because it automatically resolve responses from dispatch calls.

In thinking about it a bit more, it seems like the core opinion of this approach is that action creators are the wrong place for effectful logic (even if the actual effects are handled elsewhere)?

To be honest that was my first opinion (when i was still enthusiast about the elm model). My initial prototype used Sagas that were fired on each action; but I quickly realized that this won't allow expressing logic over multiples actions. Actually I'm less opinionated, I'm even thinking about adding support for simple Sagas that can be fired directly from Action Creators, thus providing something like redux-gen

yelouafi commented Dec 16, 2015

@ashaffer

Would you mind providing a comparison of redux-saga to each of these?

after looking at the mentioned project i'd say

  • redux-gen improves over redux-thunk with the linear/structured control flow provided by generators. But it suffers from the same limitations of being fired from within Action Creators. So, to handle logic that spans multiple actions it has to store its control state inside the store (the FSM approach).
  • redux-effects seems to focus more on the declarative approach to side effects than the control flow itself, which can be delegated to other libs like declarative-promise or redux-gen. IMO this is a good point, because of the ability to pre-process effects-as-objects before they get executed by the dedicated middlewares, as well as better testability.

But having actually discussed with @tomkis1 who already tried this approach in real production; it seems there are some issues inherent to this approach

I think redux-effects and redux-saga address 2 complementary concerns. I think redux-saga can also be used as a composition middleware for redux-effects (unless I'm mistaken) because it automatically resolve responses from dispatch calls.

In thinking about it a bit more, it seems like the core opinion of this approach is that action creators are the wrong place for effectful logic (even if the actual effects are handled elsewhere)?

To be honest that was my first opinion (when i was still enthusiast about the elm model). My initial prototype used Sagas that were fired on each action; but I quickly realized that this won't allow expressing logic over multiples actions. Actually I'm less opinionated, I'm even thinking about adding support for simple Sagas that can be fired directly from Action Creators, thus providing something like redux-gen

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Dec 16, 2015

Contributor
    yield io.race([
      shouldFetch ? fetchPosts(io, newReddit) : null,
      true // avoid blocking on the fetchPosts call
    ])

seems a bit hard to read, is there a better way to express not blocking?

Contributor

gaearon commented Dec 16, 2015

    yield io.race([
      shouldFetch ? fetchPosts(io, newReddit) : null,
      true // avoid blocking on the fetchPosts call
    ])

seems a bit hard to read, is there a better way to express not blocking?

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 16, 2015

I was thinking the same thing. Maybe a helper function io.spawn which encapsulates the code above.

yelouafi commented Dec 16, 2015

I was thinking the same thing. Maybe a helper function io.spawn which encapsulates the code above.

@tomkis

This comment has been minimized.

Show comment
Hide comment
@tomkis

tomkis Dec 17, 2015

Contributor

@yelouafi

The project emerged actually from a lengthy discussion; The idea of Saga was introduced by @slorber. I was initially enthusiast about the Elm model but ended up with a conviction that code that must manage the order of things (operations) should be kept separated from code that compute the app state
...
To be honest that was my first opinion (when i was still enthusiast about the elm model). My initial prototype used Sagas that were fired on each action; but I quickly realized that this won't allow expressing logic over multiples actions. Actually I'm less opinionated, I'm even thinking about adding support for simple Sagas that can be fired directly from Action Creators, thus providing something like redux-gen

I am advocating the Elm approach ever since the first time I encountered Flux and after 3 successful production projects (1 Flux, 2 Redux) I am still convinced that it's it the only right way to think about side effects and I am really convinced that I won't change my opinion in near future.

My arguments are strongly based on some principal similarities between CQRS/ES and functional unidirectional data flow front end architecture (redux/flux/elm...). The way we think about CQRS/ES on the server and the client should be different because the actor is different. Computer program (client) is communicating with the server (Client<->Server architecture) and all the interactions with the system are by its nature Command based on the other hand User<->Client communication is different because user is emitting Events, things they did (very important past tense here because Command is action to be executed) with the UI, not Commands.

This conceptual idea leads to the very important fact that UI is Event based, not Command based and because there are no Commands there are no Command handlers therefore all the logic should be within the reducer itself, if we treat any interaction with the UI as Event it's pretty easy, we will also get ultimate replay experience for free, because mouse clicks/moves/whatever are facts which can't simply be denied!

I really believe that service and domain layer should be separated. Intention to do some side effect is business rule its actual execution is not and we can consider this some kind of service interaction. redux-thunk mixes those two things together

I blogged about it. And the reason that we need to reduce side effects somehow led me to the need of implementing something like redux-side-effects.

Contributor

tomkis commented Dec 17, 2015

@yelouafi

The project emerged actually from a lengthy discussion; The idea of Saga was introduced by @slorber. I was initially enthusiast about the Elm model but ended up with a conviction that code that must manage the order of things (operations) should be kept separated from code that compute the app state
...
To be honest that was my first opinion (when i was still enthusiast about the elm model). My initial prototype used Sagas that were fired on each action; but I quickly realized that this won't allow expressing logic over multiples actions. Actually I'm less opinionated, I'm even thinking about adding support for simple Sagas that can be fired directly from Action Creators, thus providing something like redux-gen

I am advocating the Elm approach ever since the first time I encountered Flux and after 3 successful production projects (1 Flux, 2 Redux) I am still convinced that it's it the only right way to think about side effects and I am really convinced that I won't change my opinion in near future.

My arguments are strongly based on some principal similarities between CQRS/ES and functional unidirectional data flow front end architecture (redux/flux/elm...). The way we think about CQRS/ES on the server and the client should be different because the actor is different. Computer program (client) is communicating with the server (Client<->Server architecture) and all the interactions with the system are by its nature Command based on the other hand User<->Client communication is different because user is emitting Events, things they did (very important past tense here because Command is action to be executed) with the UI, not Commands.

This conceptual idea leads to the very important fact that UI is Event based, not Command based and because there are no Commands there are no Command handlers therefore all the logic should be within the reducer itself, if we treat any interaction with the UI as Event it's pretty easy, we will also get ultimate replay experience for free, because mouse clicks/moves/whatever are facts which can't simply be denied!

I really believe that service and domain layer should be separated. Intention to do some side effect is business rule its actual execution is not and we can consider this some kind of service interaction. redux-thunk mixes those two things together

I blogged about it. And the reason that we need to reduce side effects somehow led me to the need of implementing something like redux-side-effects.

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 17, 2015

@tomkis1

I am advocating the Elm approach ever since the first time I encountered Flux and after 3 successful production projects (1 Flux, 2 Redux) I am still convinced that it's it the only right way to think about side effects

IMO Redux, despite some similarities, is not Elm architecture; in Elm a component encapsulates all its logic (render UI, update, actions) while Redux, by its nature, only encapsulates actions/update (reducer) and is totally unaware of the UI. When combined with an UI layer like React, we make a kind of Vertical Separation: the hierarchy of reducers doesn't necessarily reflect the hierarchy of components; and we establish connections at various levels between the 2 hierarchies (i.e. connect).

What the Elm model provides, IMO, is a pure model to build reusable UI components (Data Grids, Dialogs, Date Pickers ...); while in the couple React-Redux, those are actually implemented as stateful React components. So if I'd compare Elm with the couple React-Redux, i'd say that the Elm model offers an alternative UI model to build Redux apps (e.g. you can build a View layer using only React pure components).

But still. If by some mean we would use the Elm model to build a pure View layer for Redux; I think,and that's only an opinion, w'd do better if we separate state transitions from effect outputs.

yelouafi commented Dec 17, 2015

@tomkis1

I am advocating the Elm approach ever since the first time I encountered Flux and after 3 successful production projects (1 Flux, 2 Redux) I am still convinced that it's it the only right way to think about side effects

IMO Redux, despite some similarities, is not Elm architecture; in Elm a component encapsulates all its logic (render UI, update, actions) while Redux, by its nature, only encapsulates actions/update (reducer) and is totally unaware of the UI. When combined with an UI layer like React, we make a kind of Vertical Separation: the hierarchy of reducers doesn't necessarily reflect the hierarchy of components; and we establish connections at various levels between the 2 hierarchies (i.e. connect).

What the Elm model provides, IMO, is a pure model to build reusable UI components (Data Grids, Dialogs, Date Pickers ...); while in the couple React-Redux, those are actually implemented as stateful React components. So if I'd compare Elm with the couple React-Redux, i'd say that the Elm model offers an alternative UI model to build Redux apps (e.g. you can build a View layer using only React pure components).

But still. If by some mean we would use the Elm model to build a pure View layer for Redux; I think,and that's only an opinion, w'd do better if we separate state transitions from effect outputs.

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 17, 2015

w'd do better if we separate state transitions from effect outputs.

That, of course, if you don't consider triggering state transitions as a form of Side Effect itself

yelouafi commented Dec 17, 2015

w'd do better if we separate state transitions from effect outputs.

That, of course, if you don't consider triggering state transitions as a form of Side Effect itself

@tomkis

This comment has been minimized.

Show comment
Hide comment
@tomkis

tomkis Dec 17, 2015

Contributor

@yelouafi

IMO Redux, despite some similarities, is not Elm architecture

Absolutely agree! I didn't say we should do it the same way like Elm but I was specifically talking about Elm's effect handling. It was just a reference.

Anyway

Redux, by its nature, only encapsulates actions/update (reducer) and is totally unaware of the UI

I am really glad that you wrote it because it's just confirming that we shouldn't care about complex architectures (Elm) but we should rather think about simple concepts (CQRS/ES/DDD).

Strictly speaking, Redux is Event Sourcing and nothing else. Now it depends only on you if you want to combine that with CQRS (side effects in action creators with redux-thunk) or you better re-think the way we treat UI interaction now - Just keep events to be Events :-)

Redux is a predictable state container for JavaScript apps.

People who are claiming that Flux/Redux is CQRS are wrong because Redux is about state management and CQRS does not have anything to do with state management, it's responsibility of Event Sourcing.

Your approach is interesting yet I am afraid that it suffers the same pain points like redux-thunk:

  1. Unit testing UI interaction as defined by use cases in your domain model is not possible
  2. Mixing service and domain layer
Contributor

tomkis commented Dec 17, 2015

@yelouafi

IMO Redux, despite some similarities, is not Elm architecture

Absolutely agree! I didn't say we should do it the same way like Elm but I was specifically talking about Elm's effect handling. It was just a reference.

Anyway

Redux, by its nature, only encapsulates actions/update (reducer) and is totally unaware of the UI

I am really glad that you wrote it because it's just confirming that we shouldn't care about complex architectures (Elm) but we should rather think about simple concepts (CQRS/ES/DDD).

Strictly speaking, Redux is Event Sourcing and nothing else. Now it depends only on you if you want to combine that with CQRS (side effects in action creators with redux-thunk) or you better re-think the way we treat UI interaction now - Just keep events to be Events :-)

Redux is a predictable state container for JavaScript apps.

People who are claiming that Flux/Redux is CQRS are wrong because Redux is about state management and CQRS does not have anything to do with state management, it's responsibility of Event Sourcing.

Your approach is interesting yet I am afraid that it suffers the same pain points like redux-thunk:

  1. Unit testing UI interaction as defined by use cases in your domain model is not possible
  2. Mixing service and domain layer
@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 17, 2015

  1. Unit testing UI interaction as defined by use cases in your domain model is not possible
  2. Mixing service and domain layer

Not sure if I correctely understood it; can you elaborate ?

yelouafi commented Dec 17, 2015

  1. Unit testing UI interaction as defined by use cases in your domain model is not possible
  2. Mixing service and domain layer

Not sure if I correctely understood it; can you elaborate ?

@tomkis

This comment has been minimized.

Show comment
Hide comment
@tomkis

tomkis Dec 17, 2015

Contributor

Could you write a unit test for this use case:

When user clicks the button and condition (some flag in app state) is met, loading spinner is displayed and specific API call is executed.

Contributor

tomkis commented Dec 17, 2015

Could you write a unit test for this use case:

When user clicks the button and condition (some flag in app state) is met, loading spinner is displayed and specific API call is executed.

@tomkis

This comment has been minimized.

Show comment
Hide comment
@tomkis

tomkis Dec 17, 2015

Contributor

Here is my naive example:

it('should display loading spinner and execute API call FOO', () => {
  let {appState, effects} = unwrap(reducer({loading: false, condition: true}, {type: 'SOME_ACTION'}));

  assert.isTrue(appState.loading); // loading spinner is displayed
  assert.equal(effects.length, 1);
  assert(effects[0].calledWith('FOO')); // specific API call was executed (remember, this is just a thunk, we don't care about implementation)

  let {appState, effects} = unwrap(reducer({loading: false, condition: false}, {type: 'SOME_ACTION'}));

  assert.isFalse(appState.loading); // loading spinner is not displayed
  assert.equal(effects.length, 0); // No effect gets executed
});
Contributor

tomkis commented Dec 17, 2015

Here is my naive example:

it('should display loading spinner and execute API call FOO', () => {
  let {appState, effects} = unwrap(reducer({loading: false, condition: true}, {type: 'SOME_ACTION'}));

  assert.isTrue(appState.loading); // loading spinner is displayed
  assert.equal(effects.length, 1);
  assert(effects[0].calledWith('FOO')); // specific API call was executed (remember, this is just a thunk, we don't care about implementation)

  let {appState, effects} = unwrap(reducer({loading: false, condition: false}, {type: 'SOME_ACTION'}));

  assert.isFalse(appState.loading); // loading spinner is not displayed
  assert.equal(effects.length, 0); // No effect gets executed
});
@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 17, 2015

what's wrong with ?

it('should display loading spinner and execute API call FOO', () => {
  const appState = {loading: false};

  const state = reducer(appState, {type: 'SOME_ACTION'});
  const effect = saga(appState, {type: 'SOME_ACTION'}).next().value;


  assert.isTrue(appState.loading); // loading spinner is displayed
  assert.deepEqual(effect,  [apiCall, 'FOO']); // apiCall('FOO') was yielded
});

yelouafi commented Dec 17, 2015

what's wrong with ?

it('should display loading spinner and execute API call FOO', () => {
  const appState = {loading: false};

  const state = reducer(appState, {type: 'SOME_ACTION'});
  const effect = saga(appState, {type: 'SOME_ACTION'}).next().value;


  assert.isTrue(appState.loading); // loading spinner is displayed
  assert.deepEqual(effect,  [apiCall, 'FOO']); // apiCall('FOO') was yielded
});
@tomkis

This comment has been minimized.

Show comment
Hide comment
@tomkis

tomkis Dec 17, 2015

Contributor

It's not unit test (your domain logic is in the test instead of actual code), you simply can't be sure that those two pieces which should be together: mutating the app state and some intention for side effect are actually called.

A minor off-topic, but why did you use generators? In my opinion async/await is doing exactly the same thing that you want to achieve by using generators. I used generators in my implementation because I somehow needed to to distinguish between application state and side effects but I guess you are using them only for achieving asynchronous workflow and that's exactly the reason why we have async/await

Contributor

tomkis commented Dec 17, 2015

It's not unit test (your domain logic is in the test instead of actual code), you simply can't be sure that those two pieces which should be together: mutating the app state and some intention for side effect are actually called.

A minor off-topic, but why did you use generators? In my opinion async/await is doing exactly the same thing that you want to achieve by using generators. I used generators in my implementation because I somehow needed to to distinguish between application state and side effects but I guess you are using them only for achieving asynchronous workflow and that's exactly the reason why we have async/await

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 17, 2015

It's not unit test (your domain logic is in the test instead of actual code)

I understand; that's why you emphasized the world 'unit'. I can surely setup an integration test which connects the reducer and saga to some dispatcher, dispatches the action then checks the result of the 2 but of course this is not as simple as unit testing. Sure, pure functions will be always easier to test.

On the other hand a separate generator makes implementing multi-step logic easier (which somewhat was the main purpose of this middleware). While in the ad-hoc reducer approach you'll have to implement some state machine for complex workflow. You'll likely also pollute your view state with some data that is only intended to manage the control flow (the memory of the 'effect driver')

also the @slorber concern is to be considered.

A minor off-topic, but why did you use generators? In my opinion async/await is doing exactly the same thing

Logic inside generators can be simply tested because you can step through yielded results using next; since Sagas don't yield promises but just descriptions of the desired effect, you don't have to run the effect, just test it and then resume the generator with a dummy response. With await it's more complicated, first you have to return a promise, and more importantly an async function with multiple awaits inside is like a black box. You can mock all the triggered side effects and resolve the successive results with dummy responses. But that's introduce unnecessary complications. Generators may not be as easier to test as pure functions but still they are easier to test than blackboxed async functions.

Another reason is that Generators makes other features possible; actually I'm thinking on how to express non-blocking calls (which right now could be 'hacked' using yield race([effect, true]) using something similar to unix/node process but much simpler;

function saga(io) {
  while( io.take(GET_ALL_PRODUCTS) ) {
     // don't block on this call, we don't want to miss in-between events
    const task = yield io.fork( fetchPosts, '/products' )
    /* 
      if needed
      const result = io.join(task)
   */
  }
}

yelouafi commented Dec 17, 2015

It's not unit test (your domain logic is in the test instead of actual code)

I understand; that's why you emphasized the world 'unit'. I can surely setup an integration test which connects the reducer and saga to some dispatcher, dispatches the action then checks the result of the 2 but of course this is not as simple as unit testing. Sure, pure functions will be always easier to test.

On the other hand a separate generator makes implementing multi-step logic easier (which somewhat was the main purpose of this middleware). While in the ad-hoc reducer approach you'll have to implement some state machine for complex workflow. You'll likely also pollute your view state with some data that is only intended to manage the control flow (the memory of the 'effect driver')

also the @slorber concern is to be considered.

A minor off-topic, but why did you use generators? In my opinion async/await is doing exactly the same thing

Logic inside generators can be simply tested because you can step through yielded results using next; since Sagas don't yield promises but just descriptions of the desired effect, you don't have to run the effect, just test it and then resume the generator with a dummy response. With await it's more complicated, first you have to return a promise, and more importantly an async function with multiple awaits inside is like a black box. You can mock all the triggered side effects and resolve the successive results with dummy responses. But that's introduce unnecessary complications. Generators may not be as easier to test as pure functions but still they are easier to test than blackboxed async functions.

Another reason is that Generators makes other features possible; actually I'm thinking on how to express non-blocking calls (which right now could be 'hacked' using yield race([effect, true]) using something similar to unix/node process but much simpler;

function saga(io) {
  while( io.take(GET_ALL_PRODUCTS) ) {
     // don't block on this call, we don't want to miss in-between events
    const task = yield io.fork( fetchPosts, '/products' )
    /* 
      if needed
      const result = io.join(task)
   */
  }
}
@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 17, 2015

Another reason is that Generators makes other features possible; actually I'm thinking on how to express non-blocking calls

actually the above use case was not quite expressive, you can also do

function saga(io) {
  while(await take(GET_ALL_PRODUCTS) ) {
     // don't block on this call, we don't want to miss in-between events
    const task = fetchPosts( '/products' )
    // if needed
    const result = await task
  }
}

but not things like

yield io.cancel(task)
yield io.pause(task)
task.isRunning()

yelouafi commented Dec 17, 2015

Another reason is that Generators makes other features possible; actually I'm thinking on how to express non-blocking calls

actually the above use case was not quite expressive, you can also do

function saga(io) {
  while(await take(GET_ALL_PRODUCTS) ) {
     // don't block on this call, we don't want to miss in-between events
    const task = fetchPosts( '/products' )
    // if needed
    const result = await task
  }
}

but not things like

yield io.cancel(task)
yield io.pause(task)
task.isRunning()
@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 18, 2015

seems a bit hard to read, is there a better way to express not blocking?

added support for non blocking calls using fork and join

if( shouldFetch )
  yield fork( fetchPosts, newReddit )

Also got rid of the io argument, all effect creators (call, race, fork ...) are exported from the main package (same usage in code and tests).

yelouafi commented Dec 18, 2015

seems a bit hard to read, is there a better way to express not blocking?

added support for non blocking calls using fork and join

if( shouldFetch )
  yield fork( fetchPosts, newReddit )

Also got rid of the io argument, all effect creators (call, race, fork ...) are exported from the main package (same usage in code and tests).

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Dec 18, 2015

Contributor

API is shaping up nicely, great work. 👍

Contributor

gaearon commented Dec 18, 2015

API is shaping up nicely, great work. 👍

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 18, 2015

API is shaping up nicely, great work

Thanks. Happy you liked it

yelouafi commented Dec 18, 2015

API is shaping up nicely, great work

Thanks. Happy you liked it

@ashaffer

This comment has been minimized.

Show comment
Hide comment
@ashaffer

ashaffer Dec 18, 2015

Contributor

@yelouafi I like what you're doing here as well. I think you are right that the most correct place for all this logic is in the middleware, so that you have a pristine log of intent. I'm excited to see how this shapes up.

Contributor

ashaffer commented Dec 18, 2015

@yelouafi I like what you're doing here as well. I think you are right that the most correct place for all this logic is in the middleware, so that you have a pristine log of intent. I'm excited to see how this shapes up.

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 18, 2015

@ashaffer thanks. That's nice to hear

yelouafi commented Dec 18, 2015

@ashaffer thanks. That's nice to hear

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 21, 2015

Just ported the real-world example to redux-saga.

main changes related to the original example

  • moved out the redux-router: (my initial plan was to use redux-simple-router but it doesn't keep the params inside the store). I created my own router reducer which handles a UPDATE_ROUTER_STATE action. The action is triggered from within App`s lifecycle callbacks. Everything seems working but maybe I missed something.
  • App also triggers a NAVIGATE action which is handled by watchNavigate Saga. It does so by calling the history service to trigger the navigation. The same history service is also used by the Router.
  • Moved out the API middleware and kept a simple service which expose the fetch functions (fetchUser, fetchRepo ...).
  • RepoPage and UserPage trigger each an action upon receiving new data from the router. Actions get handled by the watchLoadUserPage and watchLoadRepoPage sagas. In the original version, the logic was split between containers (fetch on route change) and action creators (decision to fetch or not). All the async code in action creators is handled by different Sagas (fetching users, repos, ... including loading decisions) using the api service

BTW updated the example to babel 6

yelouafi commented Dec 21, 2015

Just ported the real-world example to redux-saga.

main changes related to the original example

  • moved out the redux-router: (my initial plan was to use redux-simple-router but it doesn't keep the params inside the store). I created my own router reducer which handles a UPDATE_ROUTER_STATE action. The action is triggered from within App`s lifecycle callbacks. Everything seems working but maybe I missed something.
  • App also triggers a NAVIGATE action which is handled by watchNavigate Saga. It does so by calling the history service to trigger the navigation. The same history service is also used by the Router.
  • Moved out the API middleware and kept a simple service which expose the fetch functions (fetchUser, fetchRepo ...).
  • RepoPage and UserPage trigger each an action upon receiving new data from the router. Actions get handled by the watchLoadUserPage and watchLoadRepoPage sagas. In the original version, the logic was split between containers (fetch on route change) and action creators (decision to fetch or not). All the async code in action creators is handled by different Sagas (fetching users, repos, ... including loading decisions) using the api service

BTW updated the example to babel 6

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 22, 2015

Everything seems working but maybe I missed something.

And the thing i missed is ... Devtools. Which my poorman solution neither redux-router seems to handle. And which redux-simple-router

yelouafi commented Dec 22, 2015

Everything seems working but maybe I missed something.

And the thing i missed is ... Devtools. Which my poorman solution neither redux-router seems to handle. And which redux-simple-router

@yelouafi yelouafi closed this Dec 22, 2015

@yelouafi yelouafi reopened this Dec 22, 2015

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 22, 2015

(Sorry didnt mean to close it. Little phone screen).

Only redux-simple-router seems to handle devtools time travel correctly.

yelouafi commented Dec 22, 2015

(Sorry didnt mean to close it. Little phone screen).

Only redux-simple-router seems to handle devtools time travel correctly.

@kimjoar

This comment has been minimized.

Show comment
Hide comment
@kimjoar

kimjoar Dec 22, 2015

@yelouafi Yeah, we needed to handle some edge-cases in redux-simple-router to handle devtools properly. Hopefully it should work as expected.

Btw, instead of this:

function mapStateToProps(state) {
  return {
    errorMessage: state.errorMessage,
    inputValue: state.router.pathname.substring(1)
  }
}

you could rely on the params from react-router instead:

function mapStateToProps(state, props) {
  return {
    errorMessage: state.errorMessage,
    inputValue: props.params.something
  }
}

And change the something to the right param, of course.

The same [here](https://github.com/yelouafi/redux-saga/blob/master/examples/real-world/containers/RepoPage.js, where you can change to:

function mapStateToProps(state, props) {
  const { login, name } = props.params
  // ...

(i.e. for most cases I don't think redux-simple-router needs to store the params — just use the params from react-router instead)

kimjoar commented Dec 22, 2015

@yelouafi Yeah, we needed to handle some edge-cases in redux-simple-router to handle devtools properly. Hopefully it should work as expected.

Btw, instead of this:

function mapStateToProps(state) {
  return {
    errorMessage: state.errorMessage,
    inputValue: state.router.pathname.substring(1)
  }
}

you could rely on the params from react-router instead:

function mapStateToProps(state, props) {
  return {
    errorMessage: state.errorMessage,
    inputValue: props.params.something
  }
}

And change the something to the right param, of course.

The same [here](https://github.com/yelouafi/redux-saga/blob/master/examples/real-world/containers/RepoPage.js, where you can change to:

function mapStateToProps(state, props) {
  const { login, name } = props.params
  // ...

(i.e. for most cases I don't think redux-simple-router needs to store the params — just use the params from react-router instead)

@tomkis

This comment has been minimized.

Show comment
Hide comment
@tomkis

tomkis Dec 22, 2015

Contributor

@yelouafi

On the other hand a separate generator makes implementing multi-step logic easier (which somewhat was the main purpose of this middleware). While in the ad-hoc reducer approach you'll have to implement some state machine for complex workflow. You'll likely also pollute your view state with some data that is only intended to manage the control flow (the memory of the 'effect driver')

You took my words. Saga is a great pattern for orchestrating long living transactions without need of complex state machine, you should definitely mention that in the README. Because I feel like there is some misunderstanding that people think that Saga is an another approach for solving side effects, which is not true.

Contributor

tomkis commented Dec 22, 2015

@yelouafi

On the other hand a separate generator makes implementing multi-step logic easier (which somewhat was the main purpose of this middleware). While in the ad-hoc reducer approach you'll have to implement some state machine for complex workflow. You'll likely also pollute your view state with some data that is only intended to manage the control flow (the memory of the 'effect driver')

You took my words. Saga is a great pattern for orchestrating long living transactions without need of complex state machine, you should definitely mention that in the README. Because I feel like there is some misunderstanding that people think that Saga is an another approach for solving side effects, which is not true.

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Dec 31, 2015

@kjbekkelund

Yeah, we needed to handle some edge-cases in redux-simple-router to handle devtools properly.

Updating the app location seems to me much like updating the DOM with react-redux. Something that should handled automatically maybe.

yelouafi commented Dec 31, 2015

@kjbekkelund

Yeah, we needed to handle some edge-cases in redux-simple-router to handle devtools properly.

Updating the app location seems to me much like updating the DOM with react-redux. Something that should handled automatically maybe.

@kurtharriger

This comment has been minimized.

Show comment
Hide comment
@kurtharriger

kurtharriger Jan 3, 2016

I do agree that side effects in redux is currently a little clunky, but I'm a little concerned about using generators. Generators are stateful but this state is hidden inside the generator function rather than stored within the global state object. This makes it nearly impossible to snapshot and restore the state of an application and my gut reaction tells me that storing state in generators might be a step backwards.

kurtharriger commented Jan 3, 2016

I do agree that side effects in redux is currently a little clunky, but I'm a little concerned about using generators. Generators are stateful but this state is hidden inside the generator function rather than stored within the global state object. This makes it nearly impossible to snapshot and restore the state of an application and my gut reaction tells me that storing state in generators might be a step backwards.

@despairblue

This comment has been minimized.

Show comment
Hide comment
@despairblue

despairblue Jan 3, 2016

That's exactly what I thought. How would you hot swap these?

On Sun, Jan 3, 2016, 16:38 Kurt Harriger notifications@github.com wrote:

I do agree that side effects in redux is currently a little clunky, but
I'm a little concerned about using generators. Generators are stateful but
this state is hidden inside the generator function rather than stored
within the global state object. This makes it nearly impossible to snapshot
and restore the state of an application and my gut reaction tells me that
storing state in generators might be a step backwards.


Reply to this email directly or view it on GitHub
https://github.com/rackt/redux/issues/1139#issuecomment-168506113.

Q: Why is this email five sentences or less?
A: http://five.sentenc.es

despairblue commented Jan 3, 2016

That's exactly what I thought. How would you hot swap these?

On Sun, Jan 3, 2016, 16:38 Kurt Harriger notifications@github.com wrote:

I do agree that side effects in redux is currently a little clunky, but
I'm a little concerned about using generators. Generators are stateful but
this state is hidden inside the generator function rather than stored
within the global state object. This makes it nearly impossible to snapshot
and restore the state of an application and my gut reaction tells me that
storing state in generators might be a step backwards.


Reply to this email directly or view it on GitHub
https://github.com/rackt/redux/issues/1139#issuecomment-168506113.

Q: Why is this email five sentences or less?
A: http://five.sentenc.es

@yelouafi

This comment has been minimized.

Show comment
Hide comment
@yelouafi

yelouafi Jan 3, 2016

If you look at the redux-saga examples repo. You ll see that there is no less state in the store than in the thunk based version. Actually the state inside a saga is control state not app state. But if you want you can also store it in the store by using put.

Actually there are arguments in favor of the 2 approaches. See redux-saga/redux-saga#8

yelouafi commented Jan 3, 2016

If you look at the redux-saga examples repo. You ll see that there is no less state in the store than in the thunk based version. Actually the state inside a saga is control state not app state. But if you want you can also store it in the store by using put.

Actually there are arguments in favor of the 2 approaches. See redux-saga/redux-saga#8

@slorber

This comment has been minimized.

Show comment
Hide comment
@slorber

slorber Jan 4, 2016

Contributor

@kurtharriger @despairblue this is another discussion related: redux-saga/redux-saga#13 (comment)

Contributor

slorber commented Jan 4, 2016

@kurtharriger @despairblue this is another discussion related: redux-saga/redux-saga#13 (comment)

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Mar 18, 2016

Contributor

Relevant new discussion: #1528

Contributor

gaearon commented Mar 18, 2016

Relevant new discussion: #1528

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment