Skip to content

Disconnect circuit on beforeunload event and fix display #24592

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 3 commits into from
Aug 13, 2020

Conversation

captainsafia
Copy link
Member

@captainsafia captainsafia commented Aug 5, 2020

Description

Blazor's front-end logic implements some code to disconnect a circuit when a user navigates away from a page. Previously, this code was triggered on the unload DOM event. However, not all browsers trigger this event at the right time. As a result, we changed the cleanup logic to trigger earlier to ensure that circuits are properly disposed.

This also includes some UI fixes to account for the fact that the disconnect happens before the webpage is unloaded.

Customer Impact

This issue was reported by a customer in #19050. It affects the experience for users of Blazor apps and ensures that unneeded circuits are properly disposed.

Without this change, a Blazor circuit might remain open on the server long after a user has left the page and consume unnecessary resources.

Regression?

No.

Risk

Low risk because the change is backwards-compatible.

Note: The macOS build is failing for an unrelated issue that is currently affecting all PR builds targeting 3.1. This issue is tracked at #24667.

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Aug 5, 2020
@captainsafia captainsafia added the Servicing-consider Shiproom approval is required for the issue label Aug 5, 2020
@ghost
Copy link

ghost commented Aug 5, 2020

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@captainsafia captainsafia removed the Servicing-consider Shiproom approval is required for the issue label Aug 6, 2020
@captainsafia captainsafia reopened this Aug 7, 2020
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.

Approving the Boot.Server.ts changes. Looks good!

Is it intentional to bundle the "delay appearance of disconnection UI" in with this PR for backporting? If that's deliberate then OK great.

@captainsafia
Copy link
Member Author

Is it intentional to bundle the "delay appearance of disconnection UI" in with this PR for backporting?

Yeah, I wanted to make sure that @SQL-MisterMagoo got credit for his work in the backport even though we ended up modifying it.

@captainsafia captainsafia added the Servicing-consider Shiproom approval is required for the issue label Aug 7, 2020
@ghost
Copy link

ghost commented Aug 7, 2020

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@mkArtakMSFT mkArtakMSFT added the ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. label Aug 7, 2020
@mkArtakMSFT mkArtakMSFT added this to the 3.1.x milestone Aug 7, 2020
@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Aug 11, 2020
@leecow leecow modified the milestones: 3.1.x, 3.1.8 Aug 11, 2020
@Pilchie
Copy link
Member

Pilchie commented Aug 11, 2020

This is approved. @dougbu is working on getting the branding changes done, and then this can be merged for 3.1.8.

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 ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants