Skip to content
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

Future of Redux.NET / version 3.0 #51

Open
6 tasks
GuillaumeSalles opened this issue Mar 27, 2017 · 29 comments
Open
6 tasks

Future of Redux.NET / version 3.0 #51

GuillaumeSalles opened this issue Mar 27, 2017 · 29 comments

Comments

@GuillaumeSalles
Copy link
Owner

GuillaumeSalles commented Mar 27, 2017

I open this issue to know what kind of features people would want in a v3.

I don't currently use Redux.NET in my day to day job so I can't really build something useful without feedback.

What improvements I currently have in mind :

cc @cmeeren, @lilasquared, @dcolthorp :

You definitely put a lot of time to think about Redux or even use it in production. I think we can benefit from our different experiences to shape a new version of Redux that we will help everyone. Your feedback is more than welcome and I'm looking for some people that want to become core contributors of Redux.NET since I don't always have the time to work on it anymore.

@lilasquared
Copy link

lilasquared commented Mar 27, 2017

  1. I think that having IObservable isn't necessarily a bad thing but it should be implemented as an optional feature similar to how redux is. Or possibly a middleware?
  2. The enhancer is a cool feature because it allows for more enhancers than just ApplyMiddleware - As for drawbacks perhaps we can use the bindActionCreators to mitigate drawbacks during design time.
  3. Agreed - in mine I allow actions to be anything except primitive types. I think redux does the same.
  4. Not sure what that is will have to look into it.
  5. Combine reducers is key to splitting up the code to be maintainable. With my implementation of it the type safety remains. Since at design time you should have a defined type for your state tree it shouldn't be too bad. There might be some more run time checks we can do to mitigate problems with the reflection.
  6. Moar samples is always good

I would also add that if we do implement these features we separate them out similar to how I have to help with maintenance. You can see how I set up the partial classes to keep the code corresponding to the different public api methods in their own files.

The part I don't know about is how others feel using something like this. I chose to keep the feel of redux in my public api since i have the experience with redux in javascript. By that I mean that I opted for the public static CreateStore() method rather than new Store() constructor. IMO it feels more like redux that way. I would be totally willing to help implement some of these things for v3.

@cmeeren
Copy link

cmeeren commented Mar 27, 2017

First, let me state clearly how I stand:

  • In general, when I decide which libraries to use, three requirements are central to me: 1) It should work, 2) it should have the features I need, and 3) it should be simple and elegant and not require too much boilerplate for my needs. Naturally, my own Redux implementation, Yarni, satisfies all three (that's why I made it in the first place).

  • I have no significant background in JavaScript or modern web development. Writing .NET/C# has been my day job for 8 months, though I consider myself to be fairly knowledgeable about the language relative to my short experience.

  • I don't see the point of trying to mimic the original JavaScript Redux API one-to-one. There might be a slightly lower transition cost for original JS devs, but C# is an entirely different language, and one should use its strengths and idioms as far as possible. When I made Yarni, I tried to get down to the bare bones of what Redux actually is and tries to do, and write a C# library for doing that.

    Translating APIs to idiomatic C# is nothing new. Take for example Xamarin (where I'm sure many will use this library): Xamarin.Android wraps the native Java API, but things have been "C#-ified" where possible. For example, properties are used instead of fields with explicit getter and setter methods, and events are used instead of sending callbacks to subscribe methods.

    I might be able to help out with maintaining Redux.NET, but I don't have a whole lot of time on my hands and hence I would really have to like the way Redux.NET is going. Otherwise I'll just use Yarni for my own purposes.

With this in mind, here's my feedback:

  • One of the great things about the current Redux.NET implementation (and Yarni), is how short and concise it is. It's simple to understand and doesn't contain a lot of places for bugs to hide.

  • Use a simple StateChanged event on the store for subscribing to state changes. Events are very common in C# and should be well known to most .NET devs. This has benefits in edge cases e.g. for multithreading/concurrency and exceptions, where the events and handlers will behave as expected to anyone familiar with events (e.g. throwing an exception from a handler will make the subsequent handlers not get called. etc.). With a custom Subscribe() implementation, these things would be more up in the air and require more investment on the part of the developer to read documentation, not to mention put more work on us to be thorough in explaining how the code behaves in all edge cases (an impossible task since we can't foresee them all, and bound to result in bug reports).

    • In Yarni, the StateChanged handlers simply get the updated state. This is adequate for my purposes, but for a "real" implementation available on NuGet and intended to be used by the rest of the world, it would perhaps be more prudent to use a StateChangedEventArgs. This could have a State field with the updated state, and could also be extended to include OldState and other relevant fields later on without breaking the API. Though I'd actually argue against using EventHandler<StateChangedEventArgs> and instead opt for Action<StateChangedEventArgs> or similar, because the former supplies event handles with object sender in addition to the event args, and passing the store to the handler sounds unnecessary and fishy – it should be available to the handlers via other means anyway.
  • Use a Store.State property instead of a Store.GetState() method.

  • I suggest that the store's initialState should be a non-optional parameter, because then you can use the [NotNull] annotation attribute to enhance ReSharper's excellent nullability analysis (this is just for analysis, no enforcement). If the initial state is optional and hence nullable, ReSharper will warn about a possible NullReferenceException every time you use Store.State, which is counterproductive since in all real apps the root state will almost never be null (if so, then only temporarily because you might set the initial state through the reducers, but then you can just ignore a single warning when passing initialState = null).

  • I know nothing about any reactive stuff, don't need it (AFAIK), and wouldn't be interested in maintaining such code (nor would I be the right person to) . It might be useful to some, however. If included, it should be in a separate NuGet package to avoid unnecessary dependencies.

  • Looking at NRedux (and the original JS Redux for that matter), I don't really understand the StoreEnhancer stuff. Could I see some example of how it's used and in which situations it can be useful? Is it worth the added boilerplate on top of a simple Store constructor?

  • Why go out of our way to prohibit primitive types as actions? In most apps this would not be possible or feasible anyway, but in the few cases it is, why should we spend 10-20 lines to prohibit it? Redux is not really particularly opinionated, and I don't think Redux.NET should be either (apart from evident best C# practices). IMHO this belongs in the documentation, not in the code.

  • I'm also a bit wary of CombineReducers since it's using reflection. I'd rather not have any possibility at all for runtime errors. How is it used? Could someone provide a sample?

    In general, it's not that much boilerplate to create higher-level states. In my app, I have three different branches in my state:

    public class RootState
    {
        public RootState(AuthState authState, ProjectState projectState, UiSignInState uiSignInState)
        {
            this.AuthState = authState;
            this.ProjectState = projectState;
            this.UiSignInState = uiSignInState;
        }
    
        public AuthState AuthState { get; }
    
        public ProjectState ProjectState { get; }
    
        public UiSignInState UiSignInState { get; }
    }

    This is my RootState reducer:

    public static RootState Execute(RootState previousState, object action)
    {
       return new RootState(
           AuthStateReducer.Execute(previousState.AuthState, action),
           ProjectStateReducer.Execute(previousState.ProjectState, action),
           UiSignInStateReducer.Execute(previousState.UiSignInState, action));
    }

    For the record, here's the ProjectState, which contains actual data:

    public class ProjectState
    {
        public ProjectState([CanBeNull] IImmutableList<Project> projects, bool isRefreshingProjects, bool isCheckingInOutProject)
        {
            this.Projects = projects;
            this.IsRefreshingProjects = isRefreshingProjects;
            this.IsCheckingInOutProject = isCheckingInOutProject;
        }
    
        [CanBeNull]
        public IImmutableList<Project> Projects { get; }
    
        public bool IsRefreshingProjects { get; }
    
        public bool IsCheckingInOutProject { get; }
    }

    The following is not really relevant to Redux.NET itself, but as you can see, to make the state more immutable, I have get-only properties and constructors with all required parameters. This means that if I add more state properties and constructor parameters, I'll get compiler errors I have to fix, which ensures that I don't add/move a state property and forget to handle it in a reducer (which could happen with get-set-properties). The coming Record Types feature (C# 8?) would completely eliminate all the boilerplate here.

  • Returning actions from dispatch/middleware means the implementation is more flexible as regards how one handles async actions. It it not something I'm personally a fan of - I rather enjoy the async listener middleware pattern I use in Yarni, very similar in concept to redux-saga, which seems to be the most popular way of handling async actions in Redux. Therefore, having to return actions from my middleware just felt like added boilerplate in my case. Furthermore, in order to use it (no matter how), one would need to explicitly cast (probably safely and with null-checking?) the return value from Dispatch() due to the type system. Again, more boilerplate.

In short, I'm happy to discuss implementation details, but as for actually maintaining the code, that would first and foremost require Redux.NET to be as easy and simple to use for my purposes as Yarni, otherwise I'll probably just use that.

@dcolthorp
Copy link

I can see that we may each have differing goals with respect to how we would prefer to integrate this pattern into .net apps.

Like @cmeeren, I lean toward an approach more native to C#, as opposed to trying to reimplement the Javascript API.

To that end, having redux based on events seems reasonable, but the reason the pattern is appealing in the first place is because of how nicely it interplays with Rx. I'm really only interested in Redux+Rx, so there'd need to still be a good story there.

I don't have a strong opinion about your other points, but do have thoughts on whether to include CombineReducers. While this is possible in C# with reflection (as @lilasquared has shown), I tend to agree with @cmeeren that it's not my preferred approach. CombineReducers is fundamentally a dynamic solution to the problem, and C# is a static language. I think lenses or something like them are a more natural solution for a static language. See, for example, aether in F#.

For my own purposes, I've been using a simple higher order function to define reducers in terms of typed substructure.

// Register handler which target a BleState substructure and map them to the right part of State.
private static void Register<T>(AppStore store, RefAction<BleState, T> bleHandler) where T : IEvent
{
    store.RegisterHandler((ref State state, T evt) => bleHandler(ref state.BleState, evt));
}

Lenses provide a general solution to the problem of reading/writing substructure in statically typed functional languages without the need for reflection. Given that redux is fundamentally a functional pattern, I think this is a fruitful place to look for inspiration on this front. My example above is simply targeted at how to write reducers focused on a substructure, but there's an option to go further to make reducers easier to write as well.

One goal I have is setting up a batteries-included, prepackaged "starter kit" we can use for xamarin and WPF apps. I see a redux-like pattern as part of the solution, but my focus is more on how the library API and principles will facilitate an architecture, as opposed to e.g. developing a very extensible library.

In particular, I'm keen to have good solutions to:

  1. asynchronous process handling (e.g. sagas),
  2. integration testing via dispatching events and awaiting resulting state changes,
  3. a good threading model that resists programmer error.

As pointed out in my blog post, I'm looking to draw inspiration from redux-sagas for #1.

For #2, I think it's important to be able to dispatch an action and await the completion of any resulting asynchronous tasks before moving on to the next line of the test. I want to be able to test my application core with zero chance of a timing-related failure or time wasted polling for some change to the application state.

A threading model should probably be pluggable, but redux is a pattern for stateful client apps and nearly every platform has a main thread restriction. A threading model is a blindspot for redux in javascript, because javascript doesn't have threads.

I think a good default is for events to be broadcast on the main thread, regardless of where they're dispatched. In most cases state changes will be bound into the UI. Handling Store events on any other thread is, to me, really the special case. IMO, a pattern requiring devs to remember to e.g. ObserveOn the main thread will tend to increase complexity and bug count. Granted, others may not agree with this. 😄

Redux.NET need not solve all of these challenges, but hopefully the extension mechanisms you mentioned above could be designed in a way to facilitate these things.

@lilasquared
Copy link

@cmeeren and @dcolthorp Thanks for your feedback. I'm sure a lot of dot net developers feel the same way you do - a dot net library should feel like dot net. I created my implementation purely to see if it was possible. Because of my background in javascript and existing patterns used in redux it felt familiar to me. That being said I have no problem keeping redux.net feeling more like dot net, and in fact I think it should stay that way if it will be more familiar for other dot net developers to adopt.

I do think that if we are going to call it redux then it should have the same features as traditional redux. One such feature is the enhancer. This is the critical point for redux to allow extensions. I want to be clear and say that the enhancer method is not only for middleware. Redux itself ships with an applyMiddleware enhancer but this is not the only way to enhance the store. If you would like to see other examples for enhancers you can check out redux-persist. It allows for the store to be persisted and auto rehydrated as needed.

For combine reducers I think that what @cmeeren showed with the root reducer that just passes the action down the chain is super simple and super clean. As I said before I created mine more as a challenge to see if it would be possible - also to have the same feel as redux. It was a learning experience for me and I think keeping redux.net more dot net like is better.

As for the middleware it is going to be tough to have similar functionality as redux. Explicitly casting any time you want to do something with the result from a dispatch is not ideal. I solved part of this by providing a method with the middleware that did the casting so that the developer would not be responsible for doing the casting. Here is an example.

public class LiteDbMiddleware {
    public static Middleware<TState> CreateMiddleware<TState>(String databaseName) {
        var db = new LiteDatabase(databaseName);
        var continuum = db.GetCollection<PointInTime<TState>>("spacetime");

        return store => {
            var pointsInTime = continuum.FindAll().ToList();
            pointsInTime.ForEach(point => store.Dispatch(point.LastAction));

            return next => action => {
                if (action is GetHistoryAction) {
                    return continuum.FindAll().ToList();
                }
                var result = next(action);
                continuum.Insert(new PointInTime<TState> { LastAction = action, State = store.GetState() });
                return result;
            };
        };
    }

    public static IEnumerable<PointInTime<TState>> GetHistory<TState>(Dispatcher dispatch) {
        return dispatch(new GetHistoryAction()) as IEnumerable<PointInTime<TState>>;
    }
}

this is a simple middleware that I created that stores every action into a LiteDB (simple bson file) and then reads all the actions back out and applies them to the store when the app starts. Its a super basic event-sourcing example. In the example you can see that i also bundled with it an action GetHistoryAction and a method GetHistory. GetHistory dispatches the internal action to the store - which is intercepted by the middleware and returns a list of all the actions. With this it would be possible to call into the middleware to retrieve a list of all the actions that have been applied to the store and view them as history, along with the state of the application at the time. I think that the redux dev tools do something similar for the time travel debugging. I don't want to get caught up in the example because its probably riddled with holes, but the concept is that the return values from the middleware can have a use - so there should be some thought put into a good way to do it.

Also I would point out that in response to

Why go out of our way to prohibit primitive types as actions? In most apps this would not be possible or feasible anyway, but in the few cases it is, why should we spend 10-20 lines to prohibit it? Redux is not really particularly opinionated, and I don't think Redux.NET should be either (apart from evident best C# practices). IMHO this belongs in the documentation, not in the code.

redux does explicitly prohibit this which can be seen in this unit test, which is why I did it in my port. Like you say it would not really be feasible to do and would be considered bad practice, so explicitly disallowing it makes sense.

I'm really glad that there is a lot of discussion going on and I appreciate all the time put into the comments here. I think that redux is awesome and i'm glad to be a part of the community helping bring it to dot net

@cmeeren
Copy link

cmeeren commented Mar 27, 2017

It seems to me that we do actually agree on most core issues. Here's a rough spec sheet based on the current feedback:

  • Use idiomatic C# - the primary focus is on familiarity to .NET devs, not JS devs. Design to push devs into best-practices C#.

  • IObservable should be supported, but preferably not mandatory (it would be ideal if we can find a way to have both IObservable and events without too much mess in the library code).

  • Allow arbitrary objects (not primitives) to be actions without needing to inherit anything.

  • Support store enhancers

  • No CombineReducers - it's a dynamic solution that does not really fit a static language. Manually splitting state is simple and robust (though, of course, any brilliant ideas are always welcome).

  • Support awaiting completion of async stuff in integration tests (i.e., send an action, await on something, and check the result). I'm a bit fuzzy on exactly what would be awaited and how to solve this.

  • Robust threading model. (Multithreading-safe, events broadcast on UI thread, etc.)

@lilasquared
Copy link

@cmeeren nice work simplifying what we have talked about. I like it. I have a question about your reducer example that you've given. In the root reducer Execute method you take in the previous state and action and return a new root state. It looks like that happens no matter what action is dispatched - which means that the root state object reference will change every time any action is dispatched even if none of the reducers are listening for that action. Do you think that should be the expected behavior? I would imagine that the state should remain the same if an action is dispatched that doesn't match. I haven't tried this out yet but i'm curious if there would be unnecessary listeners or events fired. Thoughts?

@cmeeren
Copy link

cmeeren commented Mar 27, 2017

@lilasquared Okay, here's a few thoughts on that:

  1. Your observation is entirely correct: Any action will trigger the creation of a new root state instance.

  2. Always avoid premature optimizations. In my specific app, the leaves of my state tree is one ImmutableCollection, a couple of strings and some bools. There really isn't much to optimize with regards to the state.

  3. This is intentional, to keep things simple. See also point 8.

  4. The root state reference changes, but that doesn't mean that the actual "contents" of the state change. If no reducers act on the action, only the higher-level states change: There will be a new root state instance, and it will reference three new "sub-state" instances, but these three instances will reference the same actual data (all collections, strings, etc.), which constitutes the actual state of the app, so there should be no significant memory overhead. The higher-level states are essentially just "pointers" to the actual data.

  5. I would imagine that the state should remain the same if an action is dispatched that doesn't match.

    I'd argue that the state is the same, even though you create new "containers". The state, being in practice an immutable object, is semantically the same even if the state references a new object. With record types (which I hope we'll see in C# 8) the objects would AFAIK truly be the same. As it is now, Store.State will point to another object, but the actual data - the "leaves" of the state - will stay the same, and they are what ultimately constitutes the actual state of the app.

  6. You might seem to imply that the fact that State points to a new reference will trigger an unnecessary StateChanged event. If this is the case, let me clarify: In Yarni, the triggering of the StateChanged event is not related to whether or not the reference changes. Any dispatch will create a new state object and trigger an event, but one does not cause the other - the store is responsible for both.

  7. If your aim is to avoid redrawing UI elements that have not changed, that must be handled on the UI end (e.g. using DistinctUntilChanged as in the Redux.NET readme, or by comparing manually with the existing value, or by giving the subscribers access to both the old and the new state to allow comparison between them).

And finally,

  1. Do you think that should be the expected behavior?

    As far as I can see, this is all related to how I structure my state and reducers, and is not related to the Redux implementation. The store cannot and should not know anything about which actions the reducers might or might not listen to, so any optimizations of the kind you speak of would be the responsibility of the reducers themselves. I also can't see how to optimize for this without throwing in a lot of boilerplate code or deep-equal checks in the reducers, which would kind of eliminate the optimization (and with deep-equal checks, you'd have created the new objects anyway). Remember, code should be 1) easy to maintain, 2) run correctly, and 3) run fast, in that order (I can't remember who made the case for putting maintainability before correctness, but it's a good point, because you'll have a much easier time fixing well-structured code than fixing spaghetti code).

@GuillaumeSalles
Copy link
Owner Author

Wow, I was not expecting that much feedback! Really appreciated!

Use idiomatic C# - the primary focus is on familiarity to .NET devs, not JS devs. Design to push devs into best-practices C#.

I agree. A StateChanged event is a lower entry barrier than an Observable.

IObservable should be supported, but preferably not mandatory (it would be ideal if we can find a way to have both IObservable and events without too much mess in the library code).

I see 3 ways of doing it

  1. ObservableStore class wrapper that take an IStore as a parameter (See this

  2. ObserveState extension method

public static IObservable<TState> ObserveState(this IStore<TState> store)
{
    return Observable.FromEvent<TState>(
        h => store.StateChanged += h,
        h => store.StateChanged -= h);
}
  1. Let the Subscribe method inside the Store class and implement it on top of StateChanged event without Rx code. (IObservable does not belong to Rx, it's a core interface)

1 and 2 introduce breaking changes, 3 increase the surface API and force store enhancers to implement the Subscribe method.

Allow arbitrary objects (not primitives) to be actions without needing to inherit anything.

I agree. See #53 Thanks a lot @lilasquared ! :)

Support store enhancers

Store enhancers are definitely a great extension point. However, if we want to lower the entry barrier for C# developers, I'm not sure supporting the store enhancers exactly like redux.js does is a good idea. IMO, even if C# has more and more functional features, a store enhancer could be replaced by an OO decorator. What do you think about it?

No CombineReducers - it's a dynamic solution that does not really fit a static language. Manually splitting state is simple and robust (though, of course, any brilliant ideas are always welcome).

I completely agree.

Robust threading model. (Multithreading-safe, events broadcast on UI thread, etc.)

Broadcast events on the UI thread will be difficult to integrate inside Redux because it's a platform specific task. I think it can be done via platform specific store enhancer but it forces to maintain multiple nugets. (Like Rx for scheduling).

Support awaiting completion of async stuff in integration tests (i.e., send an action, await on something, and check the result). I'm a bit fuzzy on exactly what would be awaited and how to solve this.

@dcolthorp I suppose the need to "Support awaiting completion of async stuff in integration tests" is a consequence of broadcasting on the UI thread. If the store enhancer expose a DispatchAndPublishAsync method, you would be able to write tests like in your article right?

Regarding the reducer structure, I agree with @cmeeren. It worked well for me when I was working on an app with a large state.

@dcolthorp I didn't know about Lenses. Really interesting! Thanks!

@lilasquared
Copy link

@GuillaumeSalles I think you have touched on one of the main points of concern that I have with the way the events / observables are being implemented or proposed to be implemented. You mention that:

Broadcast events on the UI thread will be difficult to integrate inside Redux because it's a platform specific task. I think it can be done via platform specific store enhancer but it forces to maintain multiple nugets.

And you are 100% correct. Platform specific things should not exist within redux. If you were to use redux with any modern javascript library you would most likely use redux and a redux-helper to integrate with your platform. (see react-redux, ng-redux, backbone-redux you get the idea). Redux was designed to have low-level api that is easy to extend.

With that being said there are quite a few more reasons to be able to easily extend in javsacript. I am wondering if those reasons exist in dot net too. I am totally behind leveraging the dot net best practices to create those extensions in a way that is easily consumable. (referring to the decorators comment) However I am pretty sure redux doesn't behave the way it does simply because it is javascript and not C#.

I can think of at least two extension points / helper libraries. One for MVC, and one for WPF, Maybe one for univeral apps? I'm not too sure. It may even be possible to create a helper that makes it easier to use with the command line. I like the flexibility that redux has so I guess my question for this project is: Is the intention to create something low level that can be extended, or something that is single use everywhere. The guys who made and maintain redux have put a lot of thought into all of the features and you can read up about why they behave the way they do.

One example is the discussion about not returning the state in the subscriber callback. They thought about doing it at one point until this comment:

OK I'll accept a PR passing the parameter. One tiny requirement is to make sure it works correctly when Redux store is instrumented by Redux DevTools. I have a bad feeling that every store extension that “lifts” getState will also have to take special care of this parameter. Which is not fun and feels like the price you pay for making low-level APIs more consumer friendly.

(source : reduxjs/redux#303)

If you continue scrolling you can see exactly why they decided to keep it the way it is because creating extensions becomes harder. So back to the original question which is do we feel like there will be need for extensions and should it be easier to create those by leaving the low level api of redux orthogonal

@cmeeren
Copy link

cmeeren commented Mar 30, 2017

1 and 2 introduce breaking changes, 3 increase the surface API and force store enhancers to implement the Subscribe method.

Since this is a major upgrade (2.x → 3.0), breaking changes are perfectly fine (see SemVer if you're not familiar with it; all libraries should adhere to semantic versioning IMHO). We should lay the foundation for a solid library going forward, and that will likely include several breaking changes.

Store enhancers are definitely a great extension point. However, if we want to lower the entry barrier for C# developers, I'm not sure supporting the store enhancers exactly like redux.js does is a good idea. IMO, even if C# has more and more functional features, a store enhancer could be replaced by an OO decorator. What do you think about it?

No strong feelings on my part, but that's mostly because I lack the experience/knowledge to fully grasp the implications (I'm not that familiar with functional concepts, and while I know of the decorator pattern, I haven't used it myself yet). Whichever solution we arrive at should be robust, extensible, and simple to use.

One example is the discussion about not returning the state in the subscriber callback. They thought about doing it at one point until this comment:

Interesting, thanks. I'm leaning towards making the "raw" store low-level/orthogonal and having separate higher-level extensions (whether as functional store enhancers or OO decorators) to e.g. serve the current (and possibly previous) state to subscribers. I would prefer the most important of these extensions to be included with Redux.NET according to what @dcolthorp said about a a batteries-included, prepackaged "starter kit".

Regarding the UI thread stuff: I haven't done anything such with Yarni, yet everything works perfectly. Under which circumstances would this be a problem? (In my app, all subscribers are defined in the view codebehind and update the view directly; I don't use bindings. Perhaps that's why things work for me?)

Regarding the "awaiting completion" stuff: What about middleware? Middleware may dispatch actions forward (ultimately reaching the store) and then do other stuff that the store can't possibly know about. See e.g. my ListenerMiddleware which sends the action down and then calls all listeners (subscribers), which will perform necessary async tasks and often dispatch actions of their own. So again, I'm not sure what exactly @dcolthorp intends to be awaited.

@cmeeren
Copy link

cmeeren commented Mar 30, 2017

I took a closer look at @dcolthorp's blog post now, and I must say I find the projection philosophy quite compelling, where Store.State is not public (I guess private?) and state is only surfaced through projections defined as static functions passed to Store.Observe() (and Store.Project()).

We might consider implementing and encouraging this design, because reorganizing the app state (as is necessary as the app grows) is a pain if you're using the state directly (I spent an hour today doing that, most of it fixing unit tests, and the state was fairly small).

@dcolthorp, you provide the following example in your blog post:

App.Store.Observe(Projections.Todos.FilteredTodos)
	.Subscribe(visibleTodos => TodosItemsControl.ItemsSource = visibleTodos);

What if there's several UI components you want to update? I immediately see at least two ways of doing it:

Alternative 1 – calling App.Store.Observe several times:

App.Store.Observe(Projections.Todos.FilteredTodos)
	.Subscribe(visibleTodos => TodosItemsControl.ItemsSource = visibleTodos);
App.Store.Observe(Projections.Todos.FilterState)
	.Subscribe(filter => ShowAllControl.Checked = filter == TodoFilter.ShowAll);
App.Store.Observe(Projections.Todos.AllTodos)
	.Subscribe(allTodos => EmptyMessage.IsVisible = !allTodos.Any());

Alternative 2 – having a projection that returns something like a UI-specific state:

App.Store.Observe(Projections.TodoListState)
	.Subscribe(state => {
            TodosItemsControl.ItemsSource = state.visibleTodos;
            ShowAllControl.Checked = state.filterState == TodoFilter.ShowAll;
            EmptyMessage.IsVisible = !state.allTodos.Any();
        });

@dcolthorp
Copy link

dcolthorp commented Mar 30, 2017

Hi everyone!

A few points of followup.

Projections

I'm glad you like the projections idea @cmeeren! We've been doing one observe per property, but with a couple of twists on your example above:

First, we're using a MVVM-first approach that allows us to bind observables to a property. So in our view model we'd have something like ShowEmptyMessage = ReactiveProperty.Create(store.Observe(Projections.Todos.IsEmpty)). ReactiveProperty is a small class that lets us data bind to an observable in our XAML views. However, I plan to look more closely at http://reactiveui.net for future projects.

Second, one of the reasons I like the term "projection" rather than "selector", is that selector implies substructure extraction, whereas I try to define projections that are high level and semantic in intent. So as you can see in my ReactiveProperty example in the proceeding paragraph, I'd simply define a high level projection that lets me ask the semantic question "Do I have any todos", rather than calling allTodos.Any() in the view/model implementation. In this way, the view layer is asking domain questions and the process (allTodos.Any()) is up to the domain layer.

Rx really becomes helpful when combining these two approaches. Sometimes you simply need to invert a boolean or run a projection's domain-level result through a presentation function. Linq really solves this problem well, allowing you to do things such as

MyProperty = ReactiveProperty.Create(store.Observer(Projections.SomeDomainProjection)
                       .Select(myPresentationFunction))

Linq/Rx therefore provides the high level glue we can use to bring together domain projections and view-level concerns in a declarative way.

Threading

Totally agree that the threading shouldn't be fixed within redux, but important to support the model. I've parameterized my implementation with a Action<Action> called DispatchStrategy, an implementation based on a main thread IScheduler (from Rx) in my app, and a threadpool IScheduler in my tests. (This may need adjustment as I implement the below.)

Asynchronous actions

I hope to get heads down on this pretty soon, and maybe blog about where I end up. For asynchronous actions, my thinking was to be able to define functions of type async (IDispatch) => Task with particular action types. I should then be able to await store.Dispatch(myAction), and have the asynchronous call return only when the asynchronous action handler finishes.

These asynchronous handlers could do e.g. HTTP requests, and would then dispatch actions back into the store to reflect progress, or even trigger subsequent async actions. (Redux sagas introduces some additional terminology I'm leaving out here.) These sagas can run in any thread, as dispatching to the store is safe from any thread. Observers are still notified on the main thread, so any state changes that happen during the process can be safely bound to the UI regardless of where they were dispatched.

Crucially, awaiting the Dispatch of the original action should return only when the original async action as well as any async actions it triggered (recursively) are complete. Therefore, await store.Dispatch(new AttemptLoginAction(...)) in a test should return only when any server communication is complete and any observers have been notified by resulting state changes, so you could immediately do an assertion against a projection or e.g. mainPageViewModel.IsLoggedIn and see the result. (Infinitely running processes would obviously not be defined using this mechanism.)

I think this structure would give you a clean, well-factored way to deal with asynchrony on the one hand, but also solve a long-standing issue with system tests – namely, slowness due to e.g. UI polling and inconsistent results due to timing issues. A system constructed in this way would track the precise information about when domain model changes have occurred, allowing end-to-end system testing of the application core in a high level way. That's the hope, at least.

@cmeeren
Copy link

cmeeren commented Mar 30, 2017

@dcolthorp, could you share the implementation of Store.Observe and Store.Project? I'm trying to experiment with creating a store extension, but I don't know much about reactive programming and have become stuck.

I actually just have the basic signature right, I think:

internal static class StoreExtensions
{
    public static IObservable<TProjection> Observe<TState, TProjection>(
            this Store<TState> store,
            Func<TState, TProjection> projection)
    {
        // create and return an Observable<TProjection>
    }
}

@cmeeren
Copy link

cmeeren commented Mar 30, 2017

On another note, I'm kind of thinking maybe we should just have (on the "base" store or an enhancer we ship) a Subscribe method like now. We would need it anyway for reactive support, it is just as easy to use as events (just without the += syntax), and there are several disadvantages to events, see the bullet points near the top here.

@GuillaumeSalles
Copy link
Owner Author

GuillaumeSalles commented Mar 30, 2017

@dcolthorp, could you share the implementation of Store.Observe and Store.Project? I'm trying to experiment with creating a store extension, but I don't know much about reactive programming and have become stuck.

If I understand the projection concept correctly it should be :

With redux.net 2.0.0

public static IObservable<TProjection> Observe<TState, TProjection>(
            this Store<TState> store,
            Func<TState, TProjection> projection)
    {
        return store.Select(projection);
    }

With an event based store

public static IObservable<TProjection> Observe<TState, TProjection>(
            this Store<TState> store,
            Func<TState, TProjection> projection)
    {
        return Observable.FromEvent<TState>(
             h => store.StateChanged += h,
             h => store.StateChanged -= h)
             .Select(projection);
    }

I fail to see the difference between @dcolthorp Observe and the IObservable.Select method. If I understand correctly, the difference is the context in which is used. Observe for domain concern and Select for view concern.

@cmeeren
Copy link

cmeeren commented Mar 30, 2017

Thanks! If it's that short and concise, @dcolthorp, what's the point of having an Observe method and calling Store.Observe(Projections.MyProjection) if you can just do Store.Select(Projections.MyProjection)?

@dcolthorp
Copy link

dcolthorp commented Mar 30, 2017

Ah, sorry. Observe is just what we called Select on our Store implementation (not redux.net). The main goal is to not expose the core IObservable<State> directly. I wasn't intentionally suggesting a name change.

We also have a T Project<T>(Func<State,T>) function which returns an instantaneous snapshot, for those times when you just to know the value right now. It accepts a projection function and returns the result of its application to the current store value.

@dcolthorp
Copy link

For what it's worth, here's a current snapshot of our Store implementation, which is built on top of a Ref type which implements most of the data management and signals. Store wraps that up and provides the core action handler registration and dispatch semantics.

The key thing to note is that Dispatch is currently called Handle, at the suggestion of a colleague who thought it would be more C#-ish. I plan to rename this to Dispatch, but the current name is what it is.

Also note that all of the RefFunc etc. stuff is a consequence of the choice to use structs for organizing the store. Switching to a fully immutable State type would allow us to do everything with normal lambdas.

This code is very much a snapshot of a work in progress and not released anywhere, just extracted from the xamarin app I'm working on. It's a playground for ideas, and extensibility etc. is currently not a goal. 😀

@cmeeren
Copy link

cmeeren commented Mar 30, 2017

Thanks, that clears it up.

I'm leaning towards exposing the state (so that we don't force added layers/complexity, e.g. projections, on people who have simple needs). However, we might consider hiding it in an enhancer or subclass or similar, which instead surfaces Select (whatever the name) and Project. Thoughts?

@dcolthorp
Copy link

I have no problem with that. :-) My aim is figuring out the API I want to program to, not designing an API everyone else should use. I'm happy to wrap a lower-level implementation (like I did with Ref).

P.S. One difference between Select and my Observe is that Observe does a DistinctUntilChanged so that observing a projection doesn't kick off spurious values when other parts of the store change. My plan would be to add a lower level accessor for the cases where you really want the raw stream, but the default assumption is that each observation of a projection should vary only when the logical value changes.

@cmeeren
Copy link

cmeeren commented Mar 30, 2017

the default assumption is that each observation of a projection should vary only when the logical value changes.

I agree that this would be the expected behavior.

@GuillaumeSalles
Copy link
Owner Author

GuillaumeSalles commented Mar 30, 2017

With that being said there are quite a few more reasons to be able to easily extend in javsacript. I am wondering if those reasons exist in dot net too. I am totally behind leveraging the dot net best practices to create those extensions in a way that is easily consumable. (referring to the decorators comment) However I am pretty sure redux doesn't behave the way it does simply because it is javascript and not C#.

IMHO, the truth lies in between. Redux.js API is reduced to the bare minimum thanks to the ton of feedback from the community. As @lilasquared noted, there is some differences between the C# version and the js version.

  • Redux .NET calls the observer on subscribe and the js version don't. I think this behavior is implemented outside of redux.js by view engine specific extension (e.g. react-redux for react, ng-redux for angular). We could do this outside of Redux.NET but it's a trade of between maintaining a "Connect" nuget (like react-redux) and let this behavior inside with the risk that it does not satisfy a use case.

  • Redux.js uses a @@redux/INIT action to create the initial state instead. I think that's pretty smart because if the reducer is composed by smaller reducers, it become the responsibility of the smaller reducers to expose a valid initial state. It easier to refactor the state shape.

  • Redux.js does let you dispatch from a reducer.

However, for the Redux.js Subscribe method vs C# events, I think there is no semantic differences.

Since this is a major upgrade (2.x → 3.0), breaking changes are perfectly fine (see SemVer if you're not familiar with it; all libraries should adhere to semantic versioning IMHO). We should lay the foundation for a solid library going forward, and that will likely include several breaking changes.

I know semver and I want to follow it with Redux.NET. But if the breaking change is to huge and does not offer a clear upgrade path, people tend to be angry... (E.g angular 1.4 -> angular 2.0). But it's probably bikeshedding because Redux.NET is clearly not as popular as Angular :D ! I don't think so much people are using redux.NET (but I don't really know...) so I'm open to break things heavily. Upgrade documentation should be enough.

No strong feelings on my part, but that's mostly because I lack the experience/knowledge to fully grasp the implications (I'm not that familiar with functional concepts, and while I know of the decorator pattern, I haven't used it myself yet). Whichever solution we arrive at should be robust, extensible, and simple to use.

I think the solution 2 is concise and robust.

Regarding the UI thread stuff: I haven't done anything such with Yarni, yet everything works perfectly. Under which circumstances would this be a problem? (In my app, all subscribers are defined in the view codebehind and update the view directly; I don't use bindings. Perhaps that's why things work for me?)

It happen when dispatch is called from a background thread. It does not appear often because the dispatch is often called directly after a user input or after awaiting a network call (so we are still on the UI thread if ConfigureAwait(false) has not been used). I had to dispatch from the background thread for performance optimisation. In theory, actions creator, middleware, reducer, selectors (= projection) could be executed on the background thread and only use the UI thread when the view is updated.

Regarding the "awaiting completion" stuff: What about middleware? Middleware may dispatch actions forward (ultimately reaching the store) and then do other stuff that the store can't possibly know about. See e.g. my ListenerMiddleware which sends the action down and then calls all listeners (subscribers), which will perform necessary async tasks and often dispatch actions of their own. So again, I'm not sure what exactly @dcolthorp intends to be awaited.

It's currently hard to dispatch asynchronously from inside a middleware with Redux.NET because the Dispatch signature. (object -> object or IAction -> IAction). The Dispatch caller need to cast the dispatch return to something awaitable. If I understand correctly your listeners implementation, you need to use an async void delegate to execute async calls? It can be solved if we modify the Dispatch delegate with object -> Task but I don't think it should be done in the core library. Maybe an AsyncStore enhancer?

@GuillaumeSalles
Copy link
Owner Author

@dcolthorp Redux-saga is extremely interesting but I think we can all agree that it should be built as an extension. I reopen the issue #47 to go deeper on this topic. It would be impressive to build Saga in C#.

@cmeeren
Copy link

cmeeren commented Mar 30, 2017

Redux .NET calls the observer on subscribe and the js version don't. I think this behavior is implemented outside of redux.js by view engine specific extension (e.g. react-redux for react, ng-redux for angular). We could do this outside of Redux.NET but it's a trade of between maintaining a "Connect" nuget (like react-redux) and let this behavior inside with the risk that it does not satisfy a use case.

We could implement this as an enhancer, or make it optional through the store constructor or some CreateStore method or whatever we end up with.

@cmeeren
Copy link

cmeeren commented Mar 31, 2017

Currently reading up on Rx and liking what I learn. I came across the following on this page:

The Create method is also preferred over creating custom types that implement the IObservable interface. There really is no need to implement the observer/observable interfaces yourself. Rx tackles the intricacies that you may not think of such as thread safety of notifications and subscriptions.

Furthermore:

The usage of subjects should largely remain in the realms of samples and testing.

See the page for further details.

Currently, the Store implementation "fails" on both accounts: It implements IObservable and uses subjects. Should we implement the observable stuff differently?

@cmeeren
Copy link

cmeeren commented Apr 2, 2017

I mentioned somewhere I'd post my (trivial) implementation of ProjectingStore. Here it is:

namespace Redux
{
    using System;
    using System.Reactive.Linq;
    using JetBrains.Annotations;

    /// <summary>
    ///     A wrapper for <see cref="C:Store{TState}" /> whose state
    ///     is only accessible through projections.
    /// </summary>
    public class ProjectingStore<TState>
    {
        private readonly Store<TState> store;

        /// <inheritdoc cref="Store{TState}" />
        public ProjectingStore(
            Reducer<TState> reducer,
            TState initialState,
            params Middleware<TState>[] middlewares)
        {
            this.store = new Store<TState>(reducer, initialState, middlewares);
        }

        /// <inheritdoc cref="M:Store{TState}.Dispatch" />
        public IAction Dispatch(IAction action)
        {
            return this.store.Dispatch(action);
        }

        /// <summary>
        ///     Returns an observable of a projection of the state. Uses DistinctUntilChanged
        ///     to only push elements when the projection result changes.
        /// </summary>
        public IObservable<TProjection> Observe<TProjection>(
            Func<TState, TProjection> projection)
        {
            return this.store.Select(projection).DistinctUntilChanged();
        }

        /// <summary>Returns a (snapshot) projection of the current state.</summary>
        [CanBeNull]
        public TProjection Project<TProjection>(
            Func<TState, TProjection> projection)
        {
            return projection(this.store.GetState());
        }
    }
}

@cmeeren
Copy link

cmeeren commented May 3, 2017

@GuillaumeSalles There has been little activity in this repo for a while now. I'm specifically thinking of the outstanding pull requests (#61, #62, #64). Everything okay? Do you need anything more?

@tseemann-dk
Copy link

@GuillaumeSalles Is the Redux.Net project still active?

@JeroMiya
Copy link

I also rolled my own for a work project. I also used an Rx Extensions focused design, with some built-in functionality inspired by the redux-observable project in JS land. It didn't have support for middleware though, which would have been better than my built-in redux-observable design, but it was just a small library to get going. I also wrote some helper methods to make binding UI to Redux state a little easier.

I too noticed how difficult it can be to mix MVVM and Redux styles in a single project. If possible, putting all state in Redux and keeping view bindings to state as simple as possible is the only sustainable way to handle it for MVVM heavy frameworks like UWP, WPF, and XF. However, the model works a lot better with Blazor I've noticed, which is more like React and lends itself better to one-way bindings to observable state.

I write all the application Redux code (state, actions, and reducers) in F# and reference the F# library from the view project, which is C#. Not ideal but C# is too verbose to implement Redux properly. Perhaps C# 9 will help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants