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

[Feature Request] Register services as singletons #174

Closed
ShadowDancer opened this issue Mar 3, 2022 · 6 comments · Fixed by #175
Closed

[Feature Request] Register services as singletons #174

ShadowDancer opened this issue Mar 3, 2022 · 6 comments · Fixed by #175
Labels
Feature Request Request to add a new feature

Comments

@ShadowDancer
Copy link

Is your feature request related to a problem? Please describe.
Hi, I see quite big flaw in the library design, that is - it is registered as scoped service, where in reality this is singleton (talking to window.localStorage).
Using singletons and nested lifetime scopes is really common in GUIs (at least more common than in web servers), and I think frontend library should support such scenarios.

Describe the solution you'd like
Register services as singleton, or maybe expose it so user can compose them.

@ShadowDancer ShadowDancer added Feature Request Request to add a new feature Triage Issue needs to be triaged labels Mar 3, 2022
@chrissainty
Copy link
Member

Hi @ShadowDancer, I'm guessing you're either new to Blazor or not familiar with how its service lifetimes differ from those in the rest of .NET. Let me explain.

Blazor WebAssembly doesn't have the concept of a scoped service lifetime, scoped services behave as singletons. Blazor Server does have the concept of the usual service lifetimes, however, singleton services can be very dangerous as all clients of the app will share the same instance.

Bringing this to your comment. If libraries such as this one registered their services as singleton, then Blazor Server applications would end up sharing the local storage of whoever was the first user to access the application with all subsequent users. I'm sure you can see how this is very very bad.

In reality this wouldn't happen as Blazor registers the IJSRuntime service with different scopes depending on whether its a Blazor WebAssembly or Blazor Server app, Singleton and Scoped respectively. So if we registered our services as Singleton, they would work in Blazor WebAssembly, but would throw an exception in Blazor Server as you can't inject a service with a shorter lifetime into one with a longer lifetime (IJSRuntime (Scoped) into LocalStorageService (Singleton)).

Hence, we register our services as scoped they then behave constantly across both hosting models.

I hope that's explained why we register our services the way we do. If you feel there is still a good enough reason to change the way we register services I'd be happy to look at it.

@chrissainty chrissainty removed the Triage Issue needs to be triaged label Mar 4, 2022
@Fabster1993
Copy link

@chrissainty I absolutely agree with your explanation. But nevertheless, it can be very annoying for Blazor WebAssembly developers to be forced to use scoped services in every service you use ILocalStorageService. I have a use case, where it is very difficult to develop a workaround. Let me explain:

  • I made a fitness training app based on Blazor Wasm where i have to store current training data in local storage (to reduce traffic while an ongoing training). But a big challenge for me is the refresh button of the browser. Because it is not yet possible to block the single thread in Blazor Wasm with Task.Wait(), it is nearly impossible to restore the state out of local storage before first rendering of such components. With a singleton service, i would have the choice to force the service not to be lazy instatiated and could therefore do the restoring before app startup.

Maybe i'm wrong and this is absolutely no argument. But i think it would already make sense to differ service registration for Blazor Server and Blazor Wasm to not force the developers to use Scoped services all the time.

Have a great day!

@chrissainty
Copy link
Member

Thanks for your comment @Fabster1993. Normally in this situation I would have something like an isLoading or isInitialising variable that would show a spinner or UI skeleton until the data had loaded. It's a pretty common in SPAs.

However, I don't want to make this seem like a I'm right, you're wrong, you must do it my way thing. So, let's add the ability to register as singleton. I think exposing it as part of the LocalStorageOptions might be the best option. 99% of developers are only going to need the standard registration and I want there to be a little effort to doing this so people understand what they're doing. It could look something like this:

builder.Services.AddBlazoredLocalStorage(config =>  config.RegisterServicesAsSingleton = true);

@Fabster1993
Copy link

@chrissainty Thank you for your answer. Absolutely agree as, I use to apply the same pattern for data fetching from backend. I really have to redesign this component i guess.
Nevertheless i would really appreciate the change like proposed, since impact should be minimalistic.

@ShadowDancer
Copy link
Author

ShadowDancer commented Mar 10, 2022

Blazor WebAssembly doesn't have the concept of a scoped service lifetime, scoped services behave as singletons. Blazor Server does have the concept of the usual service lifetimes, however, singleton services can be very dangerous as all clients of the app will share the same instance.

That's not correct, you may create your own lifetime scopes, and that's really common in GUI apps, where you register context per tab. Lots of other libraries depends on things being singletons.

Bringing this to your comment. If libraries such as this one registered their services as singleton, then Blazor Server applications would end up sharing the local storage of whoever was the first user to access the application with all subsequent users. I'm sure you can see how this is very very bad.

I do not use server so I had no idea about this :(

In reality this wouldn't happen as Blazor registers the IJSRuntime service with different scopes depending on whether its a Blazor WebAssembly or Blazor Server app, Singleton and Scoped respectively. So if we registered our services as Singleton, they would work in Blazor WebAssembly, but would throw an exception in Blazor Server as you can't inject a service with a shorter lifetime into one with a longer lifetime (IJSRuntime (Scoped) into LocalStorageService (Singleton)).

I think libraries should follow this behavior, they do it for a reason.

I think exposing it as part of the LocalStorageOptions might be the best option. 99% of developers are only going to need the standard registration and I want there to be a little effort to doing this so people understand what they're doing.

Maybe there should be two separate methods (building on top of what you said about IJSRuntime), to guide people in right direction? Like this:

Services.AddWebassemblyBlazoredLocalStorage();
Services.AddServerBlazoredLocalStorage();

This way both groups of people would get correct defaults, because as you pointed out these environments are totally different, and setting in configuration is something which is easy to miss, and someone could make his/her app around scoped services where it'd break in long run.

@chrissainty
Copy link
Member

chrissainty commented Mar 10, 2022

That's not correct, you may create your own lifetime scopes, and that's really common in GUI apps, where you register context per tab. Lots of other libraries depends on things being singletons.

I think you've misunderstood my meaning. By default in ASP.NET Core, scoped services are scoped to a request and singletons are scoped to the lifetime of an application. In Blazor WebAssembly there is no request to scope scoped service too, hence they behave the same as a singleton. Also generally speaking, I don't believe client-side web applications do create their own scopes. This sounds like a desktop development practice and this isn't desktop development.

Anyway, as you may have seen, we're going to be adding the ability to registered services as scoped based on the example provided by @Fabster1993.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Request to add a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants