Skip to content

Remove await for WASM platform.callEntryPoint #31978

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

Conversation

TanayParikh
Copy link
Contributor

@TanayParikh TanayParikh commented Apr 20, 2021

Description

Regression introduced via: #31769

  • On Blazor Server, Blazor.start returns a promise that resolves when the .NET code starts up and the app becomes interactive
  • On WebAssembly:
    • Prior to Remove Entrypoint Invoker #31769, Blazor.start behaves the same as server (the promise resolves when the app becomes interactive)
    • Since Remove Entrypoint Invoker #31769, Blazor.start returns a promise that doesn’t resolve until the .NET app exits, which typically means never, and hence prevents its usage for triggering post-.NET startup logic

This PR is meant to revert that specific bit of logic, so that we no longer await the platform.callEntryPoint function and thus Blazor.start returns a promise that resolves when the .NET code starts up and the app becomes interactive. This now matches the behavior from 6.0-preview3.

Customer Impact

Without this PR we inadvertently introduce a breaking change where the Blazor.start promise doesn't resolve until the .NET app exits. This prevents its usage for triggering post-.NET startup logic.

Regression?

  • Yes
  • No

[If yes, specify the version the behavior has regressed from]:

Regression from 6.0-preview3
Regressed by: #31769

Risk

  • High
  • Medium
  • Low

[Justify the selection above]
Returning the await logic to what it was prior to #31769 which was introduced in Preview 4.

Verification

Packaging changes reviewed?

  • Yes
  • No
  • N/A

Addresses #31971

@TanayParikh TanayParikh requested a review from a team as a code owner April 20, 2021 15:55
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Apr 20, 2021
@mkArtakMSFT mkArtakMSFT added the Servicing-consider Shiproom approval is required for the issue label Apr 20, 2021
@ghost
Copy link

ghost commented Apr 20, 2021

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 this to the 6.0-preview4 milestone Apr 20, 2021
@TanayParikh TanayParikh removed the Servicing-consider Shiproom approval is required for the issue label Apr 20, 2021
Copy link
Contributor

@pranavkm pranavkm left a comment

Choose a reason for hiding this comment

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

Changes look fine. As part of merging this in to main, could we add a test for this? I was under the impression that we had end-to-end coverage for all of the startup behavior.

@SteveSandersonMS SteveSandersonMS self-requested a review April 20, 2021 17:01
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.

Nice one!

+1 on the E2E test if possible, though if it ends up being complicated for some reason we could proceed with out it for now.

@TanayParikh
Copy link
Contributor Author

TanayParikh commented Apr 20, 2021

Did the manual validation by running the GlobalizationWasmApp test app. Will add the E2E tests when merging into main. Thanks for the reviews.

@TanayParikh TanayParikh added the Servicing-consider Shiproom approval is required for the issue label Apr 20, 2021
@ghost
Copy link

ghost commented Apr 20, 2021

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.

TanayParikh added a commit that referenced this pull request Apr 20, 2021
Also added an E2E test.

Addresses #31971

Insertion into preview 4: #31978
@mkArtakMSFT mkArtakMSFT added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Apr 21, 2021
@mkArtakMSFT mkArtakMSFT merged commit ba7142e into release/6.0-preview4 Apr 21, 2021
@mkArtakMSFT mkArtakMSFT deleted the taparik/removeAwaitOnWASMCallEntryPoint branch April 21, 2021 17:33
TanayParikh added a commit that referenced this pull request Apr 21, 2021
* Remove await for WASM `platform.callEntryPoint`

Also added an E2E test.

Addresses #31971

Insertion into preview 4: #31978

* Update src/Components/test/E2ETest/Tests/StandaloneAppTest.cs
3dots pushed a commit to 3dots/aspnetcore-Web.JS that referenced this pull request Feb 19, 2024
* Remove await for WASM `platform.callEntryPoint`

Also added an E2E test.

Addresses #31971

Insertion into preview 4: dotnet/aspnetcore#31978

* Update src/Components/test/E2ETest/Tests/StandaloneAppTest.cs
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 Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants