Skip to content

Conversation

@pranavkm
Copy link
Contributor

No description provided.

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Jul 19, 2021
/// </summary>
/// <param name="disposing"><see langword="true"/> if this method is being invoked by <see cref="IDisposable.Dispose"/>, otherwise <see langword="false"/>.</param>
protected virtual void Dispose(bool disposing)
protected virtual void Dispose(bool disposing) => DisposeAsync().AsTask().GetAwaiter().GetResult();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javiercn you have the most context here - do you remember why Dispose doesn't implement by calling DisposeAsync and blocking?

Copy link
Member

Choose a reason for hiding this comment

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

Likely because that was virtual and would break consumers

Copy link
Member

Choose a reason for hiding this comment

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

Blocking is impossible and throws on WebAssembly. I know you don't normally dispose a renderer on WebAssembly but nothing stops you.

@pranavkm pranavkm marked this pull request as ready for review July 20, 2021 16:34
@pranavkm pranavkm requested a review from a team as a code owner July 20, 2021 16:34
Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Seems this doesn't change the sync-vs-async disposal thing any more, so if it's just moving the HotReloadManager.OnDeltaApplied -= ... instruction, looks very straightforwards :)

@pranavkm
Copy link
Contributor Author

Yeah, the virtual bit is an important detail we wouldn't to needlessly change.

@pranavkm pranavkm enabled auto-merge (squash) July 20, 2021 16:50
@pranavkm pranavkm merged commit 775899a into main Jul 20, 2021
@pranavkm pranavkm deleted the prkrishn/harmonize branch July 20, 2021 17:55
@ghost ghost added this to the 6.0-rc1 milestone Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants