Skip to content

Fix SSR page rendering intermediate state instead of the end state of components #52823

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 10 commits into from
Dec 21, 2023

Conversation

surayya-MS
Copy link
Member

@surayya-MS surayya-MS commented Dec 14, 2023

Fix SSR page rendering intermediate state instead of the end state of components

The problem was that not all non streaming pending tasks are awaited.
By the time it reaches this line not all non streaming pending tasks are in the list. Later other tasks can be added by child components.
I used a similar technique to correctly await all the non streaming pending tasks that is already is used here.

Same problem when post requests awaits non streaming pending tasks. Not all non streaming pending tasks are in the list. Later other tasks can be added by child components.

Fixes #52131
Fixes #52871

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Dec 14, 2023
@SteveSandersonMS
Copy link
Member

@surayya-MS This approach looks perfect to me. It's reassuring that nothing weird is going on and we just need to extend our use of the same pattern. Thanks for figuring this out!

@surayya-MS surayya-MS marked this pull request as ready for review December 18, 2023 08:57
@surayya-MS surayya-MS requested a review from a team as a code owner December 18, 2023 08:57
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Looks great!

A small tweak to the tests, if you want to make something asynchronous, Task.Yield() is better than Task.Delay() it does the same thing and we aren't adding time to the test run (imagine this over 60 tests, it adds 1 minute to the test run, and so on).

As long as we change, the rest looks good.

surayya-MS and others added 2 commits December 20, 2023 17:36
…onents/Components/ChildComponentThatDelaysLoading.razor

Co-authored-by: Javier Calvarro Nelson <[email protected]>
…onents/Components/ParentComponentThatDelaysLoading.razor

Co-authored-by: Javier Calvarro Nelson <[email protected]>
@surayya-MS surayya-MS enabled auto-merge (squash) December 20, 2023 16:43
@surayya-MS surayya-MS merged commit 363f910 into dotnet:main Dec 21, 2023
@ghost ghost added this to the 9.0-preview1 milestone Dec 21, 2023
@surayya-MS surayya-MS deleted the fix-ssr-oninitialized branch December 21, 2023 10:14
@surayya-MS surayya-MS modified the milestone: 9.0-preview1 Dec 21, 2023
surayya-MS added a commit to surayya-MS/aspnetcore that referenced this pull request Dec 21, 2023
… components (dotnet#52823)

* finish all non streaming pending tasks before rendering ssr page

* fix tests

* refactor fix for the tests

* call Dispatcher.AssertAccess() in AddPendingTask()

* add e2e test

* fix post request await all non streaming pending tasks; add e2e test

* move NonStreamingPendingTasks class to another file

* save WaitForNonStreamingPendingTasks into a variable

* Update src/Components/test/testassets/Components.TestServer/RazorComponents/Components/ChildComponentThatDelaysLoading.razor

Co-authored-by: Javier Calvarro Nelson <[email protected]>

* Update src/Components/test/testassets/Components.TestServer/RazorComponents/Components/ParentComponentThatDelaysLoading.razor

Co-authored-by: Javier Calvarro Nelson <[email protected]>

---------

Co-authored-by: Javier Calvarro Nelson <[email protected]>
wtgodbe pushed a commit that referenced this pull request Jan 4, 2024
… components (#52823) (#52943)

* finish all non streaming pending tasks before rendering ssr page

* fix tests

* refactor fix for the tests

* call Dispatcher.AssertAccess() in AddPendingTask()

* add e2e test

* fix post request await all non streaming pending tasks; add e2e test

* move NonStreamingPendingTasks class to another file

* save WaitForNonStreamingPendingTasks into a variable

* Update src/Components/test/testassets/Components.TestServer/RazorComponents/Components/ChildComponentThatDelaysLoading.razor



* Update src/Components/test/testassets/Components.TestServer/RazorComponents/Components/ParentComponentThatDelaysLoading.razor



---------

Co-authored-by: Javier Calvarro Nelson <[email protected]>
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
4 participants