mrpmorris / Fluxor

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

Scoped services become singletons if injected into effects #261

Closed peterbozso closed 2 years ago

peterbozso commented 2 years ago

Hi!

While working with Fluxor in a Blazor WebAssembly app, I ran into the problem described in the title of this issue. I tried to use typed clients in my effects and I was surprised to find out that I am getting the same client everytime I handle an action. I consider this to be a serious problem, since this way I am not getting the benefits of using IHttpClientFactory, like solving the DNS change problem.

My intuition as a .NET developer is that scoped services in Fluxor would work similar to ASP.NET Core: instead of getting new instances on each request in my controllers, I would get new instances on each action in my effects. This is not what happens now: I always get the same effect instance, and inside that, the same instances of my scoped services.

As I understand, the source of the problem is that the initialization of the Store is happening like this:

  1. ReflectionScanner.Scan is called in the AddFluxor extension method on app startup: https://github.com/mrpmorris/Fluxor/blob/65a96b579af82387ddddacb50d7c76a3b6b3b377/Source/Fluxor/DependencyInjection/ServiceCollectionExtensions.cs#L39
  2. ReflectionScanner scans the assembly for class information, among those, finds the effect classes. Then calls StoreRegistration.Register: https://github.com/mrpmorris/Fluxor/blob/65a96b579af82387ddddacb50d7c76a3b6b3b377/Source/Fluxor/DependencyInjection/ReflectionScanner.cs#L67
  3. In Register, all the Effect classes are registered as scoped services: https://github.com/mrpmorris/Fluxor/blob/65a96b579af82387ddddacb50d7c76a3b6b3b377/Source/Fluxor/DependencyInjection/ServiceRegistration/StoreRegistration.cs#L29 https://github.com/mrpmorris/Fluxor/blob/65a96b579af82387ddddacb50d7c76a3b6b3b377/Source/Fluxor/DependencyInjection/ServiceRegistration/EffectClassRegistration.cs#L12
  4. Later, in the same Register method, a scoped instance of the Store class is set up to instantiate and store a reference to all discovered and registered effect classes: https://github.com/mrpmorris/Fluxor/blob/65a96b579af82387ddddacb50d7c76a3b6b3b377/Source/Fluxor/DependencyInjection/ServiceRegistration/StoreRegistration.cs#L55

So far so good. Store is scoped and it holds references to effects what are scoped as well. But this is when our the troubles begin. In Blazor, to use Fluxor, we must initialize our store by putting this component on top of our App component: <Fluxor.Blazor.Web.StoreInitializer/>. The problem is that StoreInitializer is essentially a singleton: it's instantiated only once for the lifetime of the app and (normally) it's never replaced. The bigger problem is that a Store object is injected into it: https://github.com/mrpmorris/Fluxor/blob/65a96b579af82387ddddacb50d7c76a3b6b3b377/Source/Fluxor.Blazor.Web/StoreInitializer.cs#L20 This essentially leads to the Store becoming a singleton too, which turns all the effects it holds a reference to, into singletons, which then turns all the scoped services the effects are depending on, into singletons.

I see two solutions to this problem:

  1. Work it around by injecting IServiceScopeFactory into my effects, creating scopes myself and resolving my services from there, instead of injecting anything into my effects.
  2. Fix the above explained scoping problem in Fluxor by resolving effects within a new scope from the service provider on each dispatched action.

I created a simple example project the demonstrate the problem: https://github.com/peterbozso/FluxorScopedServiceExample Here you can see both the 'naive' implementation, and the workaround with IServiceScopeFactory.

I personally prefer the 2nd solution, but unfortunately I can't offer help with it's implementation right now. @mrpmorris if you decide against that, what do you think about at least mentioning the workaround with IServiceScopeFactory in the documentation?

uhfath commented 2 years ago

Could this be the source of your issue? https://docs.microsoft.com/en-us/aspnet/core/blazor/fundamentals/dependency-injection?view=aspnetcore-6.0#service-lifetime

Blazor WebAssembly apps don't currently have a concept of DI scopes. Scoped-registered services behave like Singleton services.

mrpmorris commented 2 years ago

This is just the way Blazor works. Because a scope in Blazor Server is tied to the SignalR connection, you effectively have the following...

Why would you want multiple stores for a single user? The point of the Flux pattern is that you have a single store.

PS: Thanks for the amount of effort you went to to create the detail in this issue!

mrpmorris commented 2 years ago

I'm closing this as it lies within Blazor.

peterbozso commented 2 years ago

Sorry both for the late reply! My past couple of weeks were more busy than I like them to be... Even though I was thinking about this topic for a while now, I was "a bit" tired when creating this issue. Rereading it now, I realize that I didn't do a very good job of describing my problem/idea. :) Sorry for that as well!

First: thank you @uhfath for highlighting this. I am aware of the fact that scoped services are handled as singletons in Blazor WebAssembly, though as I mentioned above, my first comment doesn't do a great job of demonstrating that. :) Second: thank you @mrpmorris for adding the info about Blazor Server. Since I have only worked with Blazor WebAssembly so far, I was totally unaware of how scopes are handled in Blazor Server.

Now let me try to explain my problem/idea again.

After years of developing .NET applications (mainly ASP.NET Core Web API-s), I felt a very big similarity between the following concepts:

ASP.NET Core Web API Blazor with Fluxor
HTTP request Action
Controller Effect

It might be totally just me, but even though there's no mention of this in Fluxor's documentation, I intuitively expected effects to work just like controllers, but instead of handling incoming HTTP requests sent to the server, they would handle actions dispatched to Fluxor's store. When I registered my typed HttpClients (what are scoped) in a Blazor WebAssembly app using Fluxor, I expected Fluxor to work like this (omitting some steps like middlewares and reducers for brevity):

  1. At app startup, the store is created as a singleton. It gets a reference to IServiceScopeFactory injected as a constructor parameter and saves it in a private field.
  2. When I dispatch an action from a component, the store gets it and looks for the appropriate effects that could handle it.
  3. The store creates a scope using IServiceScopeFactory, then inside that scope, instantiates the effects that will handle the action. The scoped services that the effects depend on (in the above example, the typed HttpClients) are also resolved inside the current scope.
  4. The store passes the action to the instantiated effects. The effects handle the action, then they go out of scope alongside their scoped dependencies.

So instead of scoped services being singletons (like they are in Blazor), I expected them to be scoped to actions in Fluxor.

While I was working on my app, I realized that my above detailed assumption was incorrect. I started to look at the code and found the source of it. Now I understand (or at least strongly assume :)) that Fluxor is intentionally designed like this, to align better with Blazor's handling of scoped services. But since using Blazor with Fluxor gives your application such a different structure and way of working compared to "vanilla" Blazor applications, I still think that we should consider handling scoped services in Fluxor like the way I described above. I think it'd be an additional improvement Fluxor brings to Blazor applications. While it'd be different from how Blazor itself handles scoped services, it'd still stay true to the design of the underlying ASP.NET Core framework. Of course, if you'd inject scoped services not just into your effects, but into your components as well, that'd lead to a great deal of confusion. But I think that's not something one should do when using Fluxor anyway. What do you think?

Since English is not my native language, I'd also like explicitly state that personally I can totally understand if this is only intuitive to me and the community decides against changing Fluxor's current implementation. My intention here is not to argue or complain, but to suggest something that I think would be an improvement to this library.

peterbozso commented 2 years ago

Ping @mrpmorris

mrpmorris commented 2 years ago

Reducers and effects are manually added to the store and held on to. The dependency injection extensions just auto create them as a one off when a store is created through DI.

If you want your effects to create new instances of services each tone they are executed, you can inject an IServiceScopeFactory into it and create a new scope in the method.

peterbozso commented 2 years ago

Okay, thanks @mrpmorris! What do you think about adding this piece of information/advice to the documentation? If you agree, then I am more than happy to help with a PR on that! :)

mrpmorris commented 2 years ago

Sure, write something and do a PR and I'll see if I hate it or not :)