mrpmorris / Fluxor

Fluxor is a zero boilerplate Flux/Redux library for Microsoft .NET and Blazor.
MIT License
1.26k stars 145 forks source link

Effect class constructor called only once #265

Closed mpp closed 2 years ago

mpp commented 2 years ago

Hi! I have a question regarding the Effect classes. I see that you registered them as scoped here but I see that the constructor is called only once instead than at every dispatch of the trigger action.

For example I have two HttpClient that come from two different Nuget that I register as scoped services. One of the two has a Dispose method that is called after every "use".

Due to this difference I found my self needing to handle them in a different way. This for the client that do not "dispose itself":

public class FooStateEffects
{
    private readonly IState<FooStateEffects> _fooState;
    private readonly IBarHttpClient _client;

    public GlobalizationStateEffects(IBarHttpClient client, IState<FooStateEffects> fooState)
    {
        _client = client;
        _fooState= fooState;
    }

    [EffectMethod]
    public async Task HandleFooAction(FooAction action, IDispatcher dispatcher)
    {
        // Use IBarHttpClient
    }
}

This for the client that do "dispose itself":

public class TadStateEffects
{
    private readonly IState<TadStateEffects> _tadState;
    private readonly IServiceProvider _serviceProvider;

    public TadStateEffects(IServiceProvider serviceProvider, IState<TadStateEffects> tadState)
    {
        IServiceProvider = serviceProvider;
        _tadState= fooState;
    }

    [EffectMethod]
    public async Task HandleFooAction(FooAction action, IDispatcher dispatcher)
    {
        using var scope = _serviceProvider.CreateScope();
        var client = scope.ServiceProvider.GetService<IBazHttpClient>();
        // Use IBazHttpClient
    }
}

Is this the expected behavior? Am I missing/mixing something on how scoped services works?

uhfath commented 2 years ago

Perhaps it would be best to add some sort of an Attribute with a ServiceLifetime parameter so anyone could control how effects are registered. I personally would have expected effects to be registered as Transient.

mpp commented 2 years ago

I personally would have expected effects to be registered as Transient.

I agree with you as I'd expect a new Effect<> instance for every dispatch of the trigger action

mrpmorris commented 2 years ago

Effect types are discovered at app start up and registered as scoped, the same as a store.

When a store is created via dependency injection it will create and instance of each effect type and add them to a list within the store.

HttpClients should be reused, not disposed. If you do that, your app will quickly exhaust all available connections.

peterbozso commented 2 years ago

Connected discussion: #261

mpp commented 2 years ago

Connected discussion: #261

Hi @peterbozso sorry for not noticing your issue, it is exactly the same as what I posted. We might close this issue and continue the discussion in the "original one"

@mrpmorris As @peterbozso I see actions conceptually the same as HTTP Requests and Effects as Controllers. I also acknowledge the problem you mentioned by disposing the HttpClient. I'll reach the nuget library owner and ask for a fix.