Skip to content

[General] Ability to change service lifetime #2991

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

Merged
merged 4 commits into from
Dec 3, 2024

Conversation

svrooij
Copy link
Contributor

@svrooij svrooij commented Nov 29, 2024

Pull Request

📖 Description

There are situations where you might want to access the built-in services from a singleton service. This pull requests allows the developer to configure the lifetime if needed. Normal operation is not changed

🎫 Issues

👩‍💻 Reviewer Notes

Normal operation is not changed, only the ability to register the services as Singleton

📑 Test Plan

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new component
  • I have modified an existing component
  • I have validated the Unit Tests for an existing component

⏭ Next Steps

@dvoituron
Copy link
Collaborator

To be clear:

  • AddScoped: This option creates a new instance of the service for each Blazor circuit. A circuit represents a user connection, so each user gets its own instance of the service. This is useful for services that need to maintain a user-specific state.

  • AddSingleton: This option creates a single instance of the service that is shared between all users and sessions. This is appropriate for services that do not maintain user state, or for services that need to share data between users.

In general, including for Blazor WebAssembly, it is recommended to use AddScoped for services that interact with the user interface or manage user-specific data, as this ensures that each user has his or her own instance of the service. AddSingleton can be used for static services or services that share data between users.

I have 3 comments about this PR:

  1. Why do you want to change the default AddFluentUIComponents method, when you can successively call the services.AddSingleton<IToastService, ToastService>(); methods you want to inject the services of your choice?
    Example, the standard AddRazorComponents doesn't contain an argument like that.

    The same problem will occur if a developer want to inject a single service in Singleton and the others in Scoped. Why inject everything in a single mode?

  2. I'm afraid that offering this possibility by default in the library will lead to developers making mistakes: using a Singleton is not recommended for all services and variable storage in memory. This could cause undesirable side-effects that we can't yet fully detect.

  3. If we really want to add such a configuration, I think it would be better to use the LibraryConfiguration service configuration object. And why refuse th Transient?

@svrooij
Copy link
Contributor Author

svrooij commented Nov 29, 2024

I taught that when you have WebAssembly using scoped is useless, does not exists and only stops you from using the services in Singleton services. And it seems this is in the documentation as well. You always have the option to do your own service registering, but that will lead to people forgetting new services.

Giving the option to use the default registration while still allowing to change the lifetime seems like a nice way for developers.

Off-course this should NEVER be used when using this server side, because there you have a scope per request which makes far more sense.

@dvoituron
Copy link
Collaborator

I taught that when you have WebAssembly using scoped is useless, does not exists and only stops you from using the services in Singleton services

Yes and no :-)
Like explained in the doc you referenced:

Scoped-registered services behave like Singleton services.

So, the services injected using AddScope will behave like Singleton services. But it is possible to inject a Scoped service into a WASM application (which is what we're using in the demo site). And that's no problem. The only difference is that you need to retrieve it as a “Scoped” service (and not Singleton) if you need it later.

@svrooij
Copy link
Contributor Author

svrooij commented Nov 30, 2024

If they behave the same, then it helps to make them singleton in the first place.

Because if you want to retrieve any service from a singleton service it has to be either a singleton as well or a transient service.

If it is scoped, you're forced to create a "fake" scope from your singleton service, and then retrieve the service which acts like a singleton anyway (on webassembly).

@dvoituron
Copy link
Collaborator

Now I understand. You want to retrieve a FluentUI service from a Singleton service.

It's really a special case. And I don't think we need to manage it from the library. You can inject these services manually to use the Singleton without calling AdsFluentUIComponents.

Or via a ServiceLifetime property of the ServiceConfiguration object.

@svrooij
Copy link
Contributor Author

svrooij commented Nov 30, 2024

You want to retrieve a FluentUI service from a Singleton service.

No I don't want that, did not have a use case for this yet. But I understand the question from #2691 and though I would fix some of the long standing issues.

@dvoituron
Copy link
Collaborator

By adapting your PR to put this Flag in the LibraryConfiguration object, I think it would be better adapted.

@dvoituron dvoituron enabled auto-merge (squash) December 2, 2024 17:42
auto-merge was automatically disabled December 3, 2024 12:21

Head branch was pushed to by a user without write access

@vnbaaij vnbaaij merged commit 1552ee3 into microsoft:dev Dec 3, 2024
4 checks passed
@vnbaaij vnbaaij added this to the v4.11 milestone Dec 19, 2024
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

Successfully merging this pull request may close these issues.

3 participants