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

* Remove the hard stop - the whole batch has to finish its rendering.

* Change the test expectation - prerendering redirection behaves same as server or wasm redirection.

* Add unit tests.

* Feedback - improve trimming.

* Add stopping renderer tests.

* Do not throw on new way of navigation.

Author: ilonatommy

src/Components/Components/src/PublicAPI.Unshipped.txt

@@ -17,4 +17,3 @@ static Microsoft.AspNetCore.Components.Infrastructure.RegisterPersistentComponen
 static Microsoft.AspNetCore.Components.Infrastructure.ComponentsMetricsServiceCollectionExtensions.AddComponentsMetrics(Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
 static Microsoft.AspNetCore.Components.Infrastructure.ComponentsMetricsServiceCollectionExtensions.AddComponentsTracing(Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
 static Microsoft.Extensions.DependencyInjection.SupplyParameterFromPersistentComponentStateProviderServiceCollectionExtensions.AddSupplyValueFromPersistentComponentStateProvider(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
-virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.SignalRendererToFinishRendering() -> void

src/Components/Components/src/RenderTree/Renderer.cs

@@ -49,7 +49,6 @@ public abstract partial class Renderer : IDisposable, IAsyncDisposable
     private bool _rendererIsDisposed;
 
     private bool _hotReloadInitialized;
-    private bool _rendererIsStopped;
 
     /// <summary>
     /// Allows the caller to handle exceptions from the SynchronizationContext when one is available.
@@ -708,12 +707,6 @@ internal void AddToRenderQueue(int componentId, RenderFragment renderFragment)
     {
         Dispatcher.AssertAccess();
 
-        if (_rendererIsStopped)
-        {
-            // Once we're stopped, we'll disregard further attempts to queue anything
-            return;
-        }
-
         var componentState = GetOptionalComponentState(componentId);
         if (componentState == null)
         {
@@ -780,22 +773,14 @@ private ComponentState GetRequiredRootComponentState(int componentId)
         return componentState;
     }
 
-    /// <summary>
-    /// Stop adding render requests to the render queue.
-    /// </summary>
-    protected virtual void SignalRendererToFinishRendering()
-    {
-        _rendererIsStopped = true;
-    }
-
     /// <summary>
     /// Processes pending renders requests from components if there are any.
     /// </summary>
     protected virtual void ProcessPendingRender()
     {
-        if (_rendererIsDisposed || _rendererIsStopped)
+        if (_rendererIsDisposed)
         {
-            // Once we're disposed or stopped, we'll disregard further attempts to render anything
+            // Once we're disposed, we'll disregard further attempts to render anything
             return;
         }
 

src/Components/Components/test/NavigationManagerTest.cs

@@ -1,7 +1,10 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Buffers;
 using System.Diagnostics;
+using System.Net.Http;
+using System.Text;
 using Microsoft.AspNetCore.Components.Routing;
 using Microsoft.AspNetCore.InternalTesting;
 
@@ -868,6 +871,22 @@ async ValueTask HandleLocationChanging(LocationChangingContext context)
         }
     }
 
+    [Fact]
+    public void OnNotFoundSubscriptionIsTriggeredWhenNotFoundCalled()
+    {
+        // Arrange
+        var baseUri = "scheme://host/";
+        var testNavManager = new TestNavigationManager(baseUri);
+        bool notFoundTriggered = false;
+        testNavManager.OnNotFound += (sender, args) => notFoundTriggered = true;
+
+        // Simulate a component triggered NotFound
+        testNavManager.NotFound();
+
+        // Assert
+        Assert.True(notFoundTriggered, "The OnNotFound event was not triggered as expected.");
+    }
+ 
     private class TestNavigationManager : NavigationManager
     {
         public TestNavigationManager()

src/Components/Endpoints/src/DependencyInjection/HttpNavigationManager.cs

@@ -2,13 +2,15 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using Microsoft.AspNetCore.Components.Routing;
+using System.Diagnostics.CodeAnalysis;
 
 namespace Microsoft.AspNetCore.Components.Endpoints;
 
 internal sealed class HttpNavigationManager : NavigationManager, IHostEnvironmentNavigationManager
 {
     private const string _enableThrowNavigationException = "Microsoft.AspNetCore.Components.Endpoints.NavigationManager.EnableThrowNavigationException";
 
+    [FeatureSwitchDefinition(_enableThrowNavigationException)]
     private static bool _throwNavigationException =>
         AppContext.TryGetSwitch(_enableThrowNavigationException, out var switchValue) && switchValue;
 

src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.EventDispatch.cs

@@ -97,7 +97,7 @@ private async Task SetNotFoundResponseAsync(string baseUri)
         // When the application triggers a NotFound event, we continue rendering the current batch.
         // However, after completing this batch, we do not want to process any further UI updates,
         // as we are going to return a 404 status and discard the UI updates generated so far.
-        SignalRendererToFinishRenderingAfterCurrentBatch();
+        SignalRendererToFinishRendering();
     }
 
     private async Task OnNavigateTo(string uri)

src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs

@@ -183,19 +183,12 @@ protected override void AddPendingTask(ComponentState? componentState, Task task
         base.AddPendingTask(componentState, task);
     }
 
-    private void SignalRendererToFinishRenderingAfterCurrentBatch()
+    private void SignalRendererToFinishRendering()
     {
         // sets a deferred stop on the renderer, which will have an effect after the current batch is completed
         _rendererIsStopped = true;
     }
 
-    protected override void SignalRendererToFinishRendering()
-    {
-        SignalRendererToFinishRenderingAfterCurrentBatch();
-        // sets a hard stop on the renderer, which will have an effect immediately
-        base.SignalRendererToFinishRendering();
-    }
-
     protected override void ProcessPendingRender()
     {
         if (_rendererIsStopped)

src/Components/Endpoints/test/EndpointHtmlRendererTest.cs

@@ -46,6 +46,21 @@ public EndpointHtmlRendererTest()
         renderer = GetEndpointHtmlRenderer();
     }
 
+    [Fact]
+    public async Task DoesNotRenderChildAfterRendererStopped()
+    {
+        renderer.SignalRendererToFinishRendering();
+
+        var httpContext = GetHttpContext();
+        var writer = new StringWriter();
+
+        var result = await renderer.PrerenderComponentAsync(httpContext, typeof(SimpleComponent), null, ParameterView.Empty);
+        await renderer.Dispatcher.InvokeAsync(() => result.WriteTo(writer, HtmlEncoder.Default));
+        var content = writer.ToString();
+
+        Assert.DoesNotContain("Hello from SimpleComponent", content);
+    }
+
     [Fact]
     public async Task CanRender_ParameterlessComponent_ClientMode()
     {
@@ -1756,6 +1771,7 @@ private TestEndpointHtmlRenderer GetEndpointHtmlRenderer(IServiceProvider servic
 
     private class TestEndpointHtmlRenderer : EndpointHtmlRenderer
     {
+        private bool _rendererIsStopped = false;
         public TestEndpointHtmlRenderer(IServiceProvider serviceProvider, ILoggerFactory loggerFactory) : base(serviceProvider, loggerFactory)
         {
         }
@@ -1764,6 +1780,20 @@ internal int TestAssignRootComponentId(IComponent component)
         {
             return base.AssignRootComponentId(component);
         }
+        public void SignalRendererToFinishRendering()
+        {
+            // sets a deferred stop on the renderer, which will have an effect after the current batch is completed
+            _rendererIsStopped = true;
+        }
+
+        protected override void ProcessPendingRender()
+        {
+            if (_rendererIsStopped)
+            {
+                return;
+            }
+            base.ProcessPendingRender();
+        }
     }
 
     private HttpContext GetHttpContext(HttpContext context = null)

src/Components/Server/src/Circuits/RemoteNavigationManager.cs

@@ -18,6 +18,8 @@ internal sealed partial class RemoteNavigationManager : NavigationManager, IHost
     private IJSRuntime _jsRuntime;
     private bool? _navigationLockStateBeforeJsRuntimeAttached;
     private const string _enableThrowNavigationException = "Microsoft.AspNetCore.Components.Endpoints.NavigationManager.EnableThrowNavigationException";
+
+    [FeatureSwitchDefinition("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.EnableThrowNavigationException")]
     private static bool _throwNavigationException =>
         AppContext.TryGetSwitch(_enableThrowNavigationException, out var switchValue) && switchValue;
     private Func<string, Task>? _onNavigateTo;

src/Components/test/E2ETest/ServerExecutionTests/PrerenderingTest.cs

@@ -97,24 +97,6 @@ public void CanReadUrlHashOnlyOnceConnected()
             () => Browser.Exists(By.TagName("strong")).Text);
     }
 
-    [Theory]
-    [InlineData("base/relative", "prerendered/base/relative")]
-    [InlineData("/root/relative", "/root/relative")]
-    [InlineData("http://absolute/url", "http://absolute/url")]
-    public async Task CanRedirectDuringPrerendering(string destinationParam, string expectedRedirectionLocation)
-    {
-        var requestUri = new Uri(
-            _serverFixture.RootUri,
-            "prerendered/prerendered-redirection?destination=" + destinationParam);
-
-        var httpClient = new HttpClient(new HttpClientHandler { AllowAutoRedirect = false });
-        var response = await httpClient.GetAsync(requestUri);
-
-        var expectedUri = new Uri(_serverFixture.RootUri, expectedRedirectionLocation);
-        Assert.Equal(HttpStatusCode.Redirect, response.StatusCode);
-        Assert.Equal(expectedUri, response.Headers.Location);
-    }
-
     [Theory]
     [InlineData(null, null)]
     [InlineData(null, "Bert")]

src/Components/test/E2ETest/ServerRenderingTests/RenderingTest.cs

@@ -66,4 +66,53 @@ public void PostRequestRendersEndStateOfComponentsOnSSRPage()
 
         Browser.Equal("loaded child", () => Browser.Exists(By.Id("child")).Text);
     }
+
+    [Theory]
+    [InlineData(false, "ServerPrerendered", true)]
+    [InlineData(false, "ServerPrerendered", false)]
+    [InlineData(true, "ServerPrerendered", false)]
+    [InlineData(true, "ServerNonPrerendered", false)]
+    [InlineData(true, "WebAssemblyPrerendered", false)]
+    [InlineData(true, "WebAssemblyNonPrerendered", false)]
+    public async Task RenderBatchQueuedAfterRedirectionIsNotProcessed(bool redirect, string renderMode, bool throwSync)
+    {
+        string relativeUri = $"subdir/stopping-renderer?renderMode={renderMode}";
+        if (redirect)
+        {
+            relativeUri += $"&destination=redirect";
+        }
+
+        // async operation forces the next render batch
+        if (throwSync)
+        {
+            relativeUri += $"&delay=0";
+        }
+        else
+        {
+            relativeUri += $"&delay=1";
+        }
+
+        var requestUri = new Uri(_serverFixture.RootUri, relativeUri);
+        var httpClient = new HttpClient(new HttpClientHandler { AllowAutoRedirect = false });
+        var response = await httpClient.GetAsync(requestUri);
+
+        if (redirect)
+        {
+            var expectedUri = new Uri(_serverFixture.RootUri, "subdir/redirect");
+            Assert.Equal(HttpStatusCode.Redirect, response.StatusCode);
+            Assert.Equal(expectedUri, response.Headers.Location);
+        }
+        else
+        {
+            // the status code cannot be changed after it got set, so async throwing returns OK
+            if (throwSync)
+            {
+                Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
+            }
+            else
+            {
+                Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            }
+        }
+    }
 }

src/Components/test/testassets/Components.TestServer/RazorComponents/Components/InteractiveStreamingRenderingComponent.razor

@@ -59,7 +59,7 @@ else
         object key = DisableKeys ? null : counter.Id;
 
         <Counter
-            @rendermode="@GetRenderMode(counter.RenderModeId)"
+            @rendermode="@RenderModeHelper.GetRenderMode(counter.RenderModeId)"
             @key="@key"
             IdSuffix="@counter.Id.ToString()"
             IncrementAmount="counter.IncrementAmount"
@@ -201,30 +201,6 @@ else
             $"&{nameof(DisableKeys)}={disableKeys}";
     }
 
-    private static IComponentRenderMode GetRenderMode(RenderModeId renderMode)
-    {
-        return renderMode switch
-        {
-            RenderModeId.ServerPrerendered => RenderMode.InteractiveServer,
-            RenderModeId.ServerNonPrerendered => new InteractiveServerRenderMode(false),
-            RenderModeId.WebAssemblyPrerendered => RenderMode.InteractiveWebAssembly,
-            RenderModeId.WebAssemblyNonPrerendered => new InteractiveWebAssemblyRenderMode(false),
-            RenderModeId.AutoPrerendered => RenderMode.InteractiveAuto,
-            RenderModeId.AutoNonPrerendered => new InteractiveAutoRenderMode(false),
-            _ => throw new InvalidOperationException($"Unknown render mode: {renderMode}"),
-        };
-    }
-
-    private enum RenderModeId
-    {
-        ServerPrerendered = 0,
-        ServerNonPrerendered = 1,
-        WebAssemblyPrerendered = 2,
-        WebAssemblyNonPrerendered = 3,
-        AutoPrerendered = 4,
-        AutoNonPrerendered = 5,
-    }
-
     private record struct CounterInfo(int Id, int IncrementAmount, RenderModeId RenderModeId);
 
     private record ComponentState(ImmutableArray<CounterInfo> Counters, int NextCounterId)

src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/Rendering/AsyncComponent.razor

@@ -0,0 +1,28 @@
+@if(throwException)
+{
+    throw new InvalidOperationException("Child component UI exception: redirection should have stopped renderer.");
+}
+
+@code {
+    [Parameter]
+    public int Delay { get; set; }
+
+    private bool throwException { get; set; }
+
+    private string message = string.Empty;
+
+    protected override async Task OnInitializedAsync()
+    {
+        await Task.Yield();
+        _ = ScheduleRenderingExceptionAfterDelay();
+    }
+
+    private async Task ScheduleRenderingExceptionAfterDelay()
+    {
+        // This update should not happen if the renderer is stopped
+        await Task.Delay(Delay);
+        throwException = true;
+        StateHasChanged();
+    }
+}
+

src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/Rendering/StoppingRenderer.razor

@@ -0,0 +1,27 @@
+@page "/stopping-renderer"
+@inject NavigationManager NavigationManager
+
+<p>Parent content</p>
+<AsyncComponent @rendermode="@RenderModeHelper.GetRenderMode(CurrentRenderMode)" Delay="@Delay" />
+
+@code {
+    [Parameter, SupplyParameterFromQuery(Name = "destination")]
+    public string Destination { get; set; } = string.Empty;
+
+    [Parameter, SupplyParameterFromQuery(Name = "renderMode")]
+    public string? RenderModeStr { get; set; }
+
+    [Parameter, SupplyParameterFromQuery(Name = "delay")]
+    public int Delay { get; set; }
+
+    private RenderModeId CurrentRenderMode => RenderModeHelper.ParseRenderMode(RenderModeStr);
+
+    protected override Task OnInitializedAsync()
+    {
+        if (!string.IsNullOrEmpty(Destination))
+        {
+            NavigationManager.NavigateTo(Destination);
+        }
+        return Task.CompletedTask;
+    }
+}