-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Adds extension method to create service scope that implements IAsyncDisposable. #51840
Conversation
…isposable. - Introduces a new type AsyncServiceScope that implements IServiceScope and IAsyncDisposable. The type just wraps an existing IServiceScope instance, which it tries to cast it to an IAsyncDisposable when DisposeAsync is called. - Adds netstandard2.1 target to avoid bringing in System.Threading.Tasks.Extensions and Microsoft.Bcl.AsyncInterfaces if not needed. - Fixes dotnet#43970
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Tagging subscribers to this area: @eerhardt, @maryamariyan Issue Details
Can't remember (nor find info) whether we agreed on putting this in // cc @davidfowl
|
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/AsyncServiceScope.cs
Show resolved
Hide resolved
This looks good but needs tests |
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/AsyncServiceScope.cs
Outdated
Show resolved
Hide resolved
...dencyInjection.Abstractions/ref/Microsoft.Extensions.DependencyInjection.Abstractions.csproj
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/AsyncServiceScope.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: David Fowler <davidfowl@gmail.com>
Co-authored-by: David Fowler <davidfowl@gmail.com>
Tests added to |
...ependencyInjection.Abstractions/ref/Microsoft.Extensions.DependencyInjection.Abstractions.cs
Outdated
Show resolved
Hide resolved
...ies/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the contribution!
Excellent work @bjorkstromm Im looking forward to more changes like this |
Can't remember (nor find info) whether we agreed on putting this in
Microsoft.Extensions.DependencyInjection
orMicrosoft.Extensions.DependencyInjection.Abstractions
. I added it toMicrosoft.Extensions.DependencyInjection.Abstractions
, which however only targetednet461
andnetstandard2.0
. This means that we have to pull inSystem.Threading.Tasks.Extensions
(forValueTask
) andMicrosoft.Bcl.AsyncInterfaces
(forIAsyncDisposable
). Because of this, I think it would be wise to add anetstandard2.1
target so that these dependencies can be ignored onnet5
and newer,// cc @davidfowl