Is the return value from Store.Dispatch useful for anything?

[cmeeren]

Originally titled Middleware - no difference between `return next(action);` or `next(action); return null;`?. See second comment below for new info.

I came across this while implementing a middleware which sent the action off to async "listeners", see the next comment for details.

For simplicity's sake, consider the following middleware:

public static class SomeMiddleware
{
    public static Func<Dispatcher, Dispatcher> CreateMiddleware(IStore<RootState> store)
    {
        return next => action =>
        {
            return next(action);
        };
    }
}

The app seems to run exactly the same if I replace return next(action); with next(action); return null;:

public static class SomeMiddleware
{
    public static Func<Dispatcher, Dispatcher> CreateMiddleware(IStore<RootState> store)
    {
        return next => action =>
        {
            next(action);
            return null;
        };
    }
}

What is the significance of the return value from the middleware? How is it used in Redux.NET?

[cmeeren]

More info:

I came across this while trying to implement “listener middleware”, according to Alex Reardon's post The ‘middleware listener’ pattern: better asynchronous actions in Redux. To me this seems like a great way to implement async operations in Redux, because all async (and other side effect) operations are clearly separated from all other parts of the app. Basically, you create a “listener middleware” that first dispatches the action down the pipeline (by calling next(action)) so the state gets updated, and then sends the action off to all its registered listeners (which perform e.g. API calls, etc.) so that they can act on it.

For example, a click on a sign-in button will send a simple action SignInRequested {Username = "foo", Password = "bar"}, which makes a reducer set IsSigningIn = true in the state tree so a spinner can be displayed. The action is passed down through all middlewares as usual, including ListenerMiddleware, but ListenerMiddleware doesn't perform stuff and then call return next(action). First, it calls next(action) so the state gets updated, and then it proceeds to pass the action to a SignInListener which performs the relevant API calls asynchronously. The listener then dispatches a SignInCompleted {Token = "foo"} or SignInFailed {ErrorMessage = "bar"} when it completes.

After calling the listeners, I have to return something from the middleware in order to fulfill the "interface", but it doesn't seem to matter if I return null, action, or the result of next(action).

Here's a rough first draft of the listener middleware I created:

public class ListenerMiddleware
{
    public delegate Task Listener(IAction action, RootState state, Dispatcher dispatcher);

    private readonly Listener[] listeners;

    public ListenerMiddleware(params Listener[] listeners)
    {
        this.listeners = listeners;
    }

    public Func<Dispatcher, Dispatcher> CreateMiddleware(IStore<RootState> store)
    {
        return next => action =>
        {
            RootState preActionState = store.GetState();
            IAction ret = next(action);
            foreach (Listener listener in listeners)
            {
                listener(action, preActionState, store.Dispatch);
            }
            return ret;
        };
    }
}

[cmeeren]

After some more thought and investigation, allow me to partially answer my own question:

The return value from the middlewares “bubbles up” through the middleware stack, and is ultimately accessible as the return value of Store.Dispatch(). For example, if one of the middlewares returns null, the return value of Store.Dispatch() will also be null, whereas if all middlewares return next(action), then the return value will be the original action passed to Store.Dispatch().

My question now is, is this return value useful for anything?

[lilasquared]

@cmeeren the result from middlewares bubbling up is much more important if you are using redux with something like javascript that has no types. If you think of a middleware that might return a promise, you could then see yourself writing some code that looks like

store.dispatch(myActionThatHitsPromiseMiddleware).then(...)

here it is important that the store.dispatch(...) method returns the promise otherwise the .then(...) call will fail.

In the case here, I am not seeing any usefulness yet as the result of a dispatch will always yield an IAction that has no real behavior. I think it would be interesting to attempt to create the middlewares in such a way that the return value from dispatch was generic. It should be possible? But I haven't tried yet

@GuillaumeSalles do you think it is possible to implement the Dispatcher in such a way that its return value doesn't have to be IAction? If I wanted my middleware to do something async with a Task or something would it be possible?

[GuillaumeSalles]

Thanks a lot @cmeeren and @lilasquared for your comments. You definitely put a lot of thoughts on the subject!

@lilasquared is totally right about the purpose of the Dispatcher's return value. I tried to make the Dispatcher generic one year ago to dispatch but if I remember correctly the C# type system is a too restrictive to achieve what you want to do.

Let's imagine we transform the Dispatcher delegate from this :

public delegate IAction Dispatcher(IAction action);

to this :

public delegate TAction Dispatcher<TAction>(TAction action);

The IStore interface becomes IStore<TState,TAction> and it force us to define what kind of TAction can be dispatched at the store creation. It can be hard to imagine this limitation without coding, but feel free to refactor the Store class to see it by yourself. The refactoring can be done really quickly. If you find a better solution to solve this issue, don't hesitate to make a PR. I would be glad to discuss it with you.

However, I totally agree that the IAction markup interface is useless.

The listeners middleware seems to be an interesting pattern but I have the feeling that this additional indirection can be make difficult to follow the execution flow on a large app. But maybe it's just an impression since I never tried it.

I solved the async issue with an extension method on IStore :
https://github.com/GuillaumeSalles/redux.NET/blob/master/examples/async/Redux.Async/StoreExtensions.cs

I doesn't follow the redux philosophy so I did not add it to the nuget package.

[cmeeren]

The listeners middleware seems to be an interesting pattern but I have the feeling that this additional indirection can be make difficult to follow the execution flow on a large app. But maybe it's just an impression since I never tried it.

I'm also entirely new at this, but to me it seems like a great pattern, because the logic performing async actions are completely separated from the rest of the code. Say you have a login page. In the view's codebehind, all you have is SignInButtonOnClicked handler which simply dispatches a SignInRequestedAction. Then a SignInListener acts on this and sends SignInCompletedAction(string token) or SignInFailedAction(string errorMessage) based on the result.

If this logic was instead kept in the view's codebehind, then you're back to something akin to the MVVM problem of potentially large viewmodels, and furthermore you'd have to duplicate this logic if you make platform-specific UIs.

I do agree that the control flow can be a tad bit more obscure, but this hasn't been a problem for me yet; I find that as long as I define the right actions, everything in the “UI” project is more or less just dispatching actions based on UI events, and in the PCL all the control flow logic is in the reducers and the listeners (or other middleware). The listeners frequently have dependencies (API calls, view navigation, dialog messages, etc.) which I inject using DI (I therefore create all my listeners, other middleware and the Store in the composition root).

---

As a side note, I have in fact made my own Redux implementation inspired by Redux.NET (cmeeren/yarni, maybe I'll put it on Nuget sometime) with a few changes:

1. Any object can be an action.
2. I use a normal event for state changes (with overridden add which calls new handlers immediately upon subscription). No dependency on any reactive stuff (it's probably pretty neat if you need it, but I don't).
3. I use a property Store.State instead of a method Store.GetState() (why do you use a method?)
4. Store.Dispatch returns void, which also means that middleware doesn't have to return anything.
5. I have bundled a ListenerMiddleware which first passes the action down the chain (so the state gets updated) and then sends it off to all listeners. The listeners have signature public delegate Task AsyncListener<TState>(object action, TState state, Dispatcher dispatch) (they are semantically event handlers, so void return type would be fine even when they're async, but async void can be a pain to unit test, and since many of them would be async I've opted for async Task).

Let me know if you spot any glaring mistakes, though it currently works fine and I think I'm not too far off target.

[lilasquared]

@cmeeren i will definitely check it out but i would recommend to make the middleware you've bundled with it its own package if you post to nuget simply for the sake of separating concerns. redux does not ship with any middleware at all.

If store.dispatch returns void then does that mean you cannot return anything from it? So the case of chaining async actions wouldn't work in the case of the redux-promise-middleware correct?

Have either of you used Mediatr? I wonder if these two can work together or if they are too different.

[cmeeren]

i would recommend to make the middleware you've bundled with it its own package if you post to nuget simply for the sake of separating concerns. redux does not ship with any middleware at all.

Thanks, I've considered that and will probably do that if I make it available on Nuget.

If store.dispatch returns void then does that mean you cannot return anything from it? So the case of chaining async actions wouldn't work in the case of the redux-promise-middleware correct?

Correct. Returning void means you can't chain anything, because there's nothing to chain. This doesn't matter to me, because I use the listener pattern instead of async actions or other similar methods. Having to return things from my middleware just felt like noise.

Have either of you used Mediatr?

Never heard of it.

[GuillaumeSalles]

The action creators logic in the async example are in the PCL projects too.
https://github.com/GuillaumeSalles/redux.NET/tree/master/examples/async

I see Redux (javascript and .NET) as a low level library that allow the users to build higher level extensions to consume it the way they want. I thinks this extensibility is one of the root cause of its popularity in the javascript community. So listeners are definitely is a great opinionated approach to dispatch async actions.

Never heard of MediatR but I can see the similarity with listeners and how it can be used to dispatch async actions.

Yarni seems great! If I had to release a v3 of Redux.NET (to remove the Rx dependencies), the code would probably be really close.

[cmeeren]

The action creators logic in the async example are in the PCL projects too.

Oh, now I see. I hadn't looked closely enough at this example. A quick comparison:

Async listener	Async action creator
Enabled via specialized middleware	Enabled via extension method on Store
An action-dispatching “middleware” that acts on all incoming actions	An action-dispatching function that is run using the extension method
Logic placed in separate class (listener)	Logic placed in separate class (action creator)
Logic can be placed in PCL	Logic can be placed in PCL
Part of the middleware chain	Not part of the middleware chain

Potential pitfalls/tradeoffs of AsyncActionCreator as implemented in the sample:

• The async action does not go through middleware, but is executed immediately by the Dispatch overload extension. This can be a problem if, say, you have some kind of ActionFilterMiddleware that stops actions that are illegal under the current state (e.g. stops a SignInRequestedAction if state.IsSigningIn == true). This means that the action creator must itself determine whether it is allowed to run. This is of course a perfectly valid way to do it, but a dedicated middleware for stopping illegal actions could conceivably provide a bit more separation of concerns (in my mind).

  It can also be a problem if you have e.g. middleware that logs all actions. If the async action creator does not immediately send off an action such as SignInRequestedAction (e.g. if you don't have an activity indicator that needs this), then if an error occurs before SignInCompletedAction is sent, it will be harder to know what went wrong since you wouldn't know that the sign-in process was started. Whereas for the async listener pattern, all actions are simple “records” and go through all middleware.

  In other words, with the async listener pattern, the entire app is just about dispatching and reacting to simple actions, whereas with the async action creator pattern, things get more complex.

• Considering the above, it's confusing that the action creator is run using an extension method on the store (and one called Dispatch at that). The Store.Dispatch extension method is not really dispatching an action, it's just calling a function which itself is (probably) dispatching actions at some point.

  In other words, I don't really see how passing the async action creator to the extension method on Store is any different from just calling any old method containing the same logic. Since the extension method simply calls the action creator, it seems to have nothing to do with the store.

• In the current example implementation, getState is passed instead of the actual state, meaning that the state might change during the async action. This could be dangerous if you're not expecting it. It might be more prudent to pass a snapshot of the state at the time when the action creator is called. (Then again, having access to an updated state throughout the async action might be just what you want – I might just be influenced by the async listener article.)

Downsides of the current implementation of Yagni.AsyncListener:

• Each listener must remember to guard against irrelevant actions (e.g. if (!(action is SignInRequestedAction actn)) return;). Otherwise, you'll be stuck in an infinite loop where the actions dispatched by the listener are picked up again and the process repeats. This is not really a significant problem IMHO, and the guard also doubles as a cast as shown above using C# 7 (after that line, actn can be used and is a SignInRequestedAction). And the alternative would be generic listeners, which I was not able to implement in a practical way due to C#'s type system.

• The control flow can be is slightly more obscure (though not necessarily; currently, I kind of like how the whole control flow is based on reacting to simple action descriptions).

In the end, everything's a tradeoff. Right now, I'm leaning towards async listeners.

[lilasquared]

@cmeeren @GuillaumeSalles I've been working on this for some time now and I think that I have a pretty good implementation going. I studied the redux source code and ported it almost one for one. The only thing I'm missing right now is the $$observable implemenation - mainly because I haven't used it and I don't know what it does. I am very open to feedback and would be interested to hear what you guys have to think since you both have your own implementations. You can find mine at https://github.com/lilasquared/NRedux or you can install it from nuget https://www.nuget.org/packages/NRedux/0.5.1.
I am working on examples and middleware now but you can find thunk inside of the test helpers https://github.com/lilasquared/NRedux/blob/master/NRedux.Test/Helpers.cs#L135. Let me know what you think!

[GuillaumeSalles]

@cmeeren Great analysis! Just to be clear, I'm not too fond of my implementation of async action creators... (That's why the extension method is not in a nuget package.) I tried to follow the advices of the official documentation (http://redux.js.org/docs/advanced/AsyncActions.html#async-action-creators) like @lilasquared did.
I tried to implement redux-thunk as a middleware in the past but I think there is a major flaw with the type system. As we mentioned earlier, the return type of Dispatch depends on the middlewares used to create the store. It forces you to cast the Dispatch return value every time a thunk is dispatched like @lilasquared did in his tests.

var promise = store.Dispatch(Helpers.AddTodoAsync("Use Redux")) as Task;

We can't guess the return type of the dispatch without knowing which middleware has been injected in the store.

The dispatch async action creator is just a patch I used before with my team.

@lilasquared : Your implementation is great ! It's awesome that you even made StoreEnhancer and CombineReducer. The exception to avoid dispatch from a reducer is cool too. I clearly remember the redux javascript code source while going through your code. :) Are you coming from a javascript background?

Some comments :

• The @cmeeren version is more concise because it uses C# events instead of keeping the listeners in a list.
• I think you can have multithread issue with your implementation. Take a look at the lock in our code (https://github.com/GuillaumeSalles/redux.NET/blob/master/src/Redux/Store.cs#L64) and the test (https://github.com/GuillaumeSalles/redux.NET/blob/master/src/Redux.Tests/StoreTests.cs#L105)
• It's kind of weird that you use both async/await and ContinueWith in your tests. E.g: You can replace this by :

await (Task)(store.Dispatch(Helpers.AddTodoAsync("Maybe")));
Assert.Equal(expectedState, store.GetState());

[lilasquared]

@GuillaumeSalles i have a background in both equally but I am a huge fan of javascript. I pretty much read through the redux code and did my best to port it to C#.

I agree that the forced casting for the results of dispatch is a little messy. Since the action creators define which type of action will be created and in a way tell the dispatcher which middleware to use, at design time you kind of do know what the return type should be. It was the only way I could get it to work that seemed like it was not a huge deal. It would be possible to abstract it to a boundActionCreator that would always return a typed task so that you wouldn't have to cast each time its used. I can try to mock up something like that as well.

I really appreciate the feedback - I will have to check out the events instead of the listeners - would that limit in any way what the listeners could be? I was trying to keep it as vanilla as possible to resemble the actual redux source.

Thanks for the test example for multithreading - i will add one and see how it performs.

I think for the unit test I got so excited that it actually worked that i didn't think about making the test more concise.

Do you think any of this is translatable to Redux.net? I pulled yours down and started trying to implement some things but I felt like I was basically rewriting the whole thing. I would totally be willing to help you out if you are interested.

[GuillaumeSalles]

Do you think any of this is translatable to Redux.net? I pulled yours down and started trying to implement some things but I felt like I was basically rewriting the whole thing. I would totally be willing to help you out if you are interested.

Sure! We can continue to talk about it here #51