Removing selectors in subscriptions / dependant subscriptions

[ltjax]

Hi there,

We have a CRUD-type application where we have an "item" UI control that only exists when there is a selected item. Now when we delete the last item in the list, the application crashes while erxecuting a selector we created with CreateSelector that does something like state => state.CurrentItem.Id because state.CurrentItem is null. The thing is, that this selector should not be executed, because we already Dispose()ed all subscriptions to it (we remove and Dispose() of the UI control in an "outer" subscription). Interstingly, the code in the inner Subscribe() is not executed, which is correct. I reckon this is a fairly common pattern when mapping any kind of list to UI controls. Here's a test case illustrating the problem:

record Thing
{
    public int Detail { get; init; } = 42;
}

record State
{
    public Thing Thing { get; init; } = new();

}

record DeleteAction
{
}

State HandleAction(State state, DeleteAction action)
{
    return state with { Thing = null };
}

[Fact]
public void SelectorIsNotExecutedWhenDisposed()
{
    var reducers = new List<On<State>>
    {
        On<DeleteAction, State>(HandleAction),
    };

    int executedCount = 0;

    var existsSelector = CreateSelector((State state) => state.Thing != null);

    var squareSelector = CreateSelector((State state) =>
    {
        executedCount++;
        //return state.Thing.Detail; <- this would crash
        return 42;
    });

    var store = new ReduxStore<State>(reducers);

    List<IDisposable> disposables = new();
    store.Select(existsSelector)
        .Subscribe(exists =>
    {
        if (exists)
        {
            disposables.Add(store
                .Select(squareSelector)
                .Subscribe(count => { /* Do something on details. */ }));
        }
        else
        {
            disposables.ForEach(x => x.Dispose());
        }
    });

    var executedBefore = executedCount;
    store.Dispatch(new DeleteAction());
    Assert.Equal(executedBefore, executedCount);
}

Sorry, I was not able to make the test case simpler. It seems this case is difficult because you need to remove a selector while already reacting to other selectors. But the selector really should not be executed when the subscribtion is not executed. For this to be well defined, subscriptions should be ordered/executed in the order their Subscribe calls are beginning. Obviously, in this case, the other Subscribe calls the inner Subscribe, so it is important to distinguish between the beginning and end of the call. And with this ordering, "outer" subscriptions would preceed inner subscriptions.

[Odonno]

Hi Marius!

Well, this is a curious use case you showed me there. Before we dig in, can you explain me why you need to do "inner" observable this way? I ask you that because using a Subscribe in a Subscribe is always a bad move.

To complete my answer, I recommend you to use the UntilDestroyed operator. See here : https://github.com/Odonno/ReduxSimple/blob/3ec922e54d9ea79748798d52e96e3e76fc6e83eb/ReduxSimple.Uwp/Operators.cs It is currently only made for UWP apps but we can implement a new one for your use case.

[ltjax]

I agree that having inner Subscribes seems fishy at first, but I think it is unavoidable. Also it works well without CreateSelector and just Select/Subscribe. Basically, we're creating controls depending on the data in the State, where each control does its own subscriptions and (disposes them again when Dispose()'d itself). Sometimes this will be just one, but sometimes we will have 0 to N controls all doing their own subscriptions when reacting to a change of the number. I don't think your "UntilDestroyed" helps here. It works similar to a subscription's Dispose(), reacting to a control unloading. But in our case, this unloading is triggered by a Subscription (and so is the loading)

[Odonno]

The UntilDestroyed operator is simply built on top of a TakeUntil. So, you can use directly a TakeUntil with an Observable as parameter which is directly linked to the dispose of your other object (meaning calling OnNext() with a Subject, if you have to).

[ltjax]

Yes, that is exactly how I understood it, but that is just syntactic sugar on what I am already doing. I'm pretty sure the test case will fail with TakeUntil as well.

[Odonno]

Well, I believe your problem is that your selector crashes because Thing is nullable. Would you not opt-in for non-nullable reference type features and return a ThingDetail? instead? Then you would prevent any exception related to that problem!

[ltjax]

No, that is not the problem. The selector is a way to select the Thing so I do not have to duplicate that logic for each detail in it. In the real world, there are lots (100+) of details for each Thing. And there is not just 0-1 Things (which a nullable reference can represent), there are 0-N. Hence accessing it is not merely accessing a reference, but looking it up in a list or a dictionary, akin to the lenses we discussed a while back - but this time on the "view" side of things. I need a way to deduplicate that logic and use it at the right level of abstraction. This is a conceptual problem, and it will happen with any sort of collection where the UI "follows" the structure of nested data. JS React can handle this situation naturally, but with persistent UI controls, you need to be able to adapt your UI control tree at the same time as setting up new connection (because naturally, newly created controls need to look at something).

[ltjax]

I've looked into this some more, and it seems Rx.NET Select() will also have the same behavior that ReduxSimple selectors have. That's not too surprising since yours are based on the former. From what I could scrape of the net, it's not defined whether Reactive.LINQ "elements" are executed when no observer is connected. So it seems your behavior is in line with what Rx.NET does. I can only get my test to pass if I do the selection in the Subscribe(). But I still need a way to fix my crash without duplicating the coarse-selection code into every fine-selection. Do you think it would be possible to make the Selector types implement IDisposable so I can at least manually deactivate them?

[ltjax]

Some more research, and it seems changing the Selector a little does the trick:

public IObservable<TOutput> Apply(IObservable<TInput> input)
{
    return input
        .Publish()
        .RefCount()
        .Select(Selector)
        .DistinctUntilChanged();
}

Not entirely sure how it'd make sense to integrate this into the library. Does it make sense to add this directly to the store's observable? If I understand correctly, the refcount would always just be 0 or 1 when integrating it in the selector.

[Odonno]

Oh yes. The ref count internally keep the number of subscribers curently using this Observable source. So it can be 0 to N. When the number of subscribers hits 0, the source does not emit new values.

The good part is that, in theory, you can create your own operator in your code and apply that new operator when you need it.

[ltjax]

Yea, I'm already doing that, and it fixes my crash! Since the Apply() creates a new RefCount() every time you call it, it is not reused when you compose selectors though, so you end up with lots of RefCount instances for more complicated selector "networks"

[ltjax]

So while the Share aka Publish/RefCount method prevents my crashes, it does not work because recursive Subscriptions do not

[ltjax]

Oops, sorry about the close&repoen. So it seems that the Publish/RefCount method only works in preventing the crashes, but I had other inconsistencies using them. I suspect it has to do with recursive Subscriptions not working well with them. Either way, I've implemented a replacement with a few nice properties:

1. Each selector is only called at max once for each store update, no matter how many subscriptions connect to it. So you can use expensive computations here!
2. Selectors that are not used (i.e. subscribed to) are not evaluated (this prevents my crash)
3. If no input changes, the selectors are not evaluated (except for the root that looks at the state, which is always evaluated)
4. Subscriptions can be added in subscriptions, and the new Observers will immediately get the current derived value ("Default" Rx.Net Subscriptions do this too, but I suspect this is were Publish/RefCount fails, because this needs some kind of "pull" mechanism)
5. It seemlessly integrates with other Observables - every ISelector is-an IObservable

I suspect I have not covered all the edge cases in my implementation, but it works pretty nicely already. You interested in that? I can put it in a gist or something. It's about 300 LOC

[Odonno]

Well, that may be a good idea, if anyone has the same problem as you in the future.

But integrating it in ReduxSimple, that's another topic. I still struggle to understand the use case so I can't imagine for other people. :)

[ltjax]

By "my use case" you mean the nested subscriptions? Maybe I'm doing something wrong, but I don't see how. How about a simple example:

public record Thingy
{
    public string Name { get; init; } = "Unnamed";
}

public record CrudState
{
    public ImmutableList<Thingy> Thingies { get; init; } = ImmutableList<Thingy>.Empty;
}

public record CreateAction { }
public record RenameAction { public int Id { get; init; } public string NewName { get; init; } }
public record DeleteAction { }

With these reducers:

var reducers = new List<On<CrudState>>
    {
        On<CreateAction, CrudState>( (state, action) => state with { Thingies = state.Thingies.Add(new Thingy()) }),
        On<DeleteAction, CrudState>( (state, action) => state.Thingies.Count > 0 ? state with { Thingies = state.Thingies.RemoveAt(state.Thingies.Count-1) } : state),
        On<RenameAction, CrudState>( (state, action) =>
        {
            return action.Id < state.Thingies.Count
                ? state with { Thingies = state.Thingies.SetItem(action.Id, state.Thingies[action.Id] with { Name = action.NewName })}
                : state;
        })
    };
var store = new ReduxStore<CrudState>(reducers);

I observe it like this:

public class DisplayControl
{
    private readonly ReduxStore<CrudState> store;
    private readonly List<IDisposable> connections = new();

    public DisplayControl(ReduxStore<CrudState> store)
    {
        this.store = store;
    }

    public void Start()
    {
        store.Select(state => state.Thingies).Subscribe(list =>
        {
            if (list.Count > connections.Count)
            {
                for (int i = connections.Count; i < list.Count; ++i)
                {
                    Console.WriteLine("Create thingy {0}", i);
                    var id = i;

                    var connection = store
                        .Select(state => state.Thingies[id].Name)
                        .Subscribe(name => Console.WriteLine("Thingy {0} has name {1}", id, name));

                    connections.Add(connection);
                }
            }
            else if (connections.Count > list.Count)
            {
                foreach (var disposable in connections.Skip(list.Count))
                    disposable.Dispose();
                connections.RemoveRange(list.Count, connections.Count - list.Count);
                Console.WriteLine("Removed {0} thingies", connections.Count - list.Count);
            }
        });
    }
}

It will crash if I do something like:

store.Dispatch(new CreateAction { });
store.Dispatch(new CreateAction { });
store.Dispatch(new RenameAction { Id = 0, NewName = "NewName" });
store.Dispatch(new DeleteAction { });

Or even just Create and Delete. However, it will not crash, if I change my "detail" connection to:

                    var connection = store
                        .Select(state => state.Thingies)
                        .Subscribe(thingies => Console.WriteLine("Thingy {0} has name {1}", id, thingies[id]));

But I will get too many updates that way, and it'll duplicate the selection logic into the subscription, which is bad if you have more properties than just name.

[Odonno]

Oh, well. I think I start to understand what you try to achieve. So may I suggest this code?

store
    .Select(state => state.Thingies)
    .SelectMany(list => list)
    .GroupBy(thing => thing.Id)
    .DistinctUntilChanged()
    .Subscribe(thing => Console.WriteLine("Thingy {0} has name {1}", thing.Id, thing.Name));

What do you think?

[ltjax]

Well that at least pointed me in the direction of how Rx.NET handles observables of sequences, so thanks for that. I still don't see how to get the "Create" and "Removed" outputs in there. In the real application, I'm creating and removing UI controls there, and they are updating themselves via the nested subscription. I'm guessing I'd need some kind of GroupBy with side-effects there?

[Odonno]

Well, you can filter actions and listen to ObserveAction() function. So that you know when something is created or removed. If that helps.

[ltjax]

That would be very un-Redux'y. The view should be derived completely from the State, never from the action stream.

[Odonno]

Well, not really. I do that very often. It really depends on your use case. Sometimes it is way simpler to execute a function based on an action than to observe a state value which has no meaning (only to execute a function).

[ltjax]

That's not how js redux sees it (e.g. first sentence here: https://redux.js.org/faq/design-decisions#why-doesnt-redux-pass-the-state-and-action-to-subscribers) or how I see it. But we can agree to disagree.

[ltjax]

Sorry it took so long, been a busy year. I wrote a blog article how we're using ReduxSimple and posted the 'SelectorGraph' code that can do selection caching and guarantees proper order for nested selections here: https://github.com/softwareschneiderei/WpfReduxSample/blob/main/WpfReduxSample/Reduxed/SelectorGraph.cs

[Odonno]

Wow, you took that thing very seriously. I am really proud you cited this lib ReduxSimple and going beyond with the Odonno's Ctinematic Universe :) (talking about Converto even if it is really outdated now).

So still, I don't see that feature needed in ReduxSimple. Like you guessed, the goal is to make it damn simple, as much as possible.

However, I see that you made a serious effort on WPF redux-like architecture and I would like you to share this expertise with the community. I wonder if you'd be interested in writing some libraries in the project? ex: ReduxSimple.Wpf (a collection of UI helpers), ReduxSimple.Wpf.DevTools, ReduxSimple.Wpf.RouterStore, etc..