Skip to content

Follow-up: Improve stopping renderer, fix identity template #61633

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Apr 23, 2025

Follow up for #61306 and #60970.

Feedback

Templates

  • Blazor identity templates (-auth Individual) were expecting NavigateTo to throw. We should remove this expectation for the new way of navigation.
  • Blazor templates do not test behavior at runtime. That is why we did not catch this bug in the PR that changed the navigation flow. We have build and publish and check of produced elements on dotnet new command covered. I don't think we want to add runtime tests for templates, it would add a lot of testing and maintenance load, even if it was added to outerloop. Manual tests caught this problem. Fixes https://github.com/dotnet/AspNetCore-ManualTests/issues/3609.

Framework

  • Hard stop of renderer was reverted. Maintaining a hard stop would mean forcing similar behavior as we had when throwing the exception: stopping rendering when the batch is not finished. It would happen only in prerendering, so it would maintain the behavioral difference between render modes. It included removal of SignalRendererToFinishRendering public API proposal.

Testing

  • RenderBatchQueuedAfterRedirectionIsNotProcessed makes sure that no renders connected with async child component are done when the parent triggered redirection in prerendering. Because more than one render mode is tested, I found it useful to reuse the code from InteractiveStreamingRenderingComponent.razor for getting available render modes. The common bits got moved to RenderModeHelper.cs.
  • CanRedirectDuringPrerendering was removed - its expected behavior changed and the tested scenario overlaps with these covered by RenderBatchQueuedAfterRedirectionIsNotProcessed.

@ilonatommy ilonatommy added this to the .NET 10 Planning milestone Apr 28, 2025
@ilonatommy ilonatommy added the feature-prerendering Issues related to prerendering blazor components label Apr 28, 2025
@ilonatommy ilonatommy changed the title Improve stopping renderer Follow-up: Improve stopping renderer, fix identity template Apr 28, 2025
@ilonatommy ilonatommy marked this pull request as ready for review April 29, 2025 13:49
@ilonatommy ilonatommy requested a review from a team as a code owner April 29, 2025 13:49
@ilonatommy ilonatommy requested a review from javiercn April 29, 2025 13:49
@lewing
Copy link
Member

lewing commented Apr 30, 2025

/backport to release/10.0-preview4

Copy link
Contributor

Started backporting to release/10.0-preview4: https://github.com/dotnet/aspnetcore/actions/runs/14759993828

Copy link
Contributor

@lewing backporting to "release/10.0-preview4" failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Remove the hard stop - the whole batch has to finish its rendering.
Using index info to reconstruct a base tree...
M	src/Components/Components/src/PublicAPI.Unshipped.txt
M	src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.EventDispatch.cs
M	src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs
CONFLICT (content): Merge conflict in src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs
Auto-merging src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.EventDispatch.cs
CONFLICT (content): Merge conflict in src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.EventDispatch.cs
Auto-merging src/Components/Components/src/PublicAPI.Unshipped.txt
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Remove the hard stop - the whole batch has to finish its rendering.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@lewing lewing requested a review from MackinnonBuck April 30, 2025 16:59
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 feature-blazor-navigation feature-prerendering Issues related to prerendering blazor components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants