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

Implement IKeyedServiceProvider interface on ServiceProviderEngineScope #89509

Merged
merged 2 commits into from
Jul 28, 2023

Conversation

CarnaViire
Copy link
Member

My take on #89447

Mainly, just implemented the missing interface on ServiceProviderEngineScope.

I don't know whether I missed something obscure with regards to the keyed feature -- I'm not an expert in the area -- but both cases I mentioned in the issue seem to work fine with the fix.

Fixes #89447

@ghost
Copy link

ghost commented Jul 26, 2023

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

My take on #89447

Mainly, just implemented the missing interface on ServiceProviderEngineScope.

I don't know whether I missed something obscure with regards to the keyed feature -- I'm not an expert in the area -- but both cases I mentioned in the issue seem to work fine with the fix.

Fixes #89447

Author: CarnaViire
Assignees: -
Labels:

area-Extensions-DependencyInjection

Milestone: -

@CarnaViire
Copy link
Member Author

Test failures are unrelated (#83659)

@CarnaViire
Copy link
Member Author

@steveharter PTAL (or can I just merge this given @halter73's approval?)

@BrennanConroy
Copy link
Member

FYI, this is blocking ASP.NET Core trying to use keyed DI. e.g. in frameworks like SignalR, Minimal, MVC, Blazor.

The sooner this gets in the more time we will have to attempt to update the frameworks with support for the feature for 8.0 😃

@halter73
Copy link
Member

It's tempting to ask for a preview7 backport even though it is late.

@CarnaViire
Copy link
Member Author

CarnaViire commented Jul 28, 2023

Ok I'm merging it by CET EOD today, given it is blocking ASP.NET Core -- and dotnet/extensions (dotnet/extensions#4211, dotnet/extensions#4222), and NCL...
If anyone is unhappy please scream now :D

cc @steveharter @buyaa-n @karelz @ericstj

Copy link
Contributor

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@CarnaViire CarnaViire merged commit ec5c223 into dotnet:main Jul 28, 2023
98 of 103 checks passed
@CarnaViire CarnaViire deleted the di-implement-keyed-interface branch July 28, 2023 16:53
@ghost ghost locked as resolved and limited conversation to collaborators Aug 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Injected or scoped ServiceProvider does not support Keyed services
5 participants