OwningComponentBase implements IAsyncDisposable #62583
Conversation
dotnet-policy-service
bot
added
the
community-contribution
ilonatommy
added
the
breaking-change
dotnet-policy-service
bot
added
the
needs-breaking-change-announcement
|
@dotnet-policy-service agree |
ilonatommy
removed
the
community-contribution
Reverted one change
Removed `OwningComponentBase.DisposeAsync()` from the PublicAPI as the implementation is private now
| /// <summary> | ||
| /// Releases the service scope used by the component. | ||
| /// </summary> |
There was a problem hiding this comment.
I sense the AI working here 😄
These methods are not public, so no need to document it. In .NET when a method is an interface implementation, it can use /// <inhertidoc /> to tell it to use the same text as the interface definition.
| if (disposing) | ||
| { | ||
| if (_scope.HasValue) | ||
| { | ||
| if (_scope.Value is IDisposable disposable) | ||
| { | ||
| disposable.Dispose(); | ||
| } | ||
| _scope = null; | ||
| } | ||
| } |
There was a problem hiding this comment.
You can probably combine the three conditions rather than nesting them:
if(disposing && _scope.HasValue && _scope.Value is IDisposable disposable)
Might be possible to also just do
if(disposing && _scope.Value is IDisposable disposable) (without the has value check). You can set _scope = null unconditionally inside the clause
| /// Asynchronously releases the service scope used by the component. | ||
| /// </summary> | ||
| /// <returns>A task that represents the asynchronous dispose operation.</returns> | ||
| protected virtual async ValueTask DisposeAsyncCore() |
There was a problem hiding this comment.
Note for API review. Since this is a "standard" implementation pattern that people might have followed within their own classes, it might be problematic if we use the same name (as the name will start conflicting) so we should consider "breaking away" from this and using a different name to "avoid the breackage".
Overall, I think this is potentially uncommon since OwningComponentBase is not common in the first place.
OwningComponentBase implements IAsyncDisposable
Description
This pull request enhances the
OwningComponentBaseclass by adding support for asynchronous disposal, improving resource management, and updating tests to validate the new functionality. The most important changes include implementing theIAsyncDisposableinterface, adding new methods for asynchronous disposal, updating the public API, and introducing new test cases to ensure proper behavior.Enhancements to
OwningComponentBase:src/Components/Components/src/OwningComponentBase.cs: Updated theOwningComponentBaseclass to implement theIAsyncDisposableinterface. AddedDisposeAsync,DisposeAsyncCore, and updated theDisposemethod to handle both synchronous and asynchronous disposal of the service scope.Public API updates:
src/Components/Components/src/PublicAPI.Unshipped.txt: AddedDisposeAsyncandDisposeAsyncCoremethods to the public API to reflect the new asynchronous disposal capabilities.Testing improvements:
src/Components/Components/test/OwningComponentBaseTest.cs: Added new test cases to verify the behavior ofDisposeAsync, ensure that accessing scoped services after disposal throws an exception.Fixes #25873