From 1f4341a2485085b5b5e4d326bb2b6bafa9c5ae94 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 9 Aug 2019 15:44:22 -0700 Subject: [PATCH] Add 'firstTime' parameter to OnAfterRender Fixes: #11610 I took the approach here of building this into `ComponentBase` instead of `IHandleAfterRender` - *because* my reasoning is that `firstTime` is an opinionated construct. There's nothing fundamental about `firstTime` that requires tracking by the rendering, it's simply an opinion that it's going to be useful for component authors, and reinforces a common technique. Feedback on this is welcome. --- .../Pages/Index.razor | 2 +- .../Pages/Json.razor | 2 +- .../Pages/RenderList.razor | 2 +- ...oft.AspNetCore.Components.netcoreapp3.0.cs | 4 +- ...ft.AspNetCore.Components.netstandard2.0.cs | 4 +- .../Components/src/ComponentBase.cs | 32 ++++- .../Components/test/ComponentBaseTest.cs | 128 ++++++++++++++++++ .../Components/test/RendererTest.cs | 2 +- .../AfterRenderInteropComponent.razor | 2 +- .../InteropOnInitializationComponent.razor | 12 +- .../PrerenderedToInteractiveTransition.razor | 7 +- .../HtmlHelperComponentExtensionsTests.cs | 2 +- 12 files changed, 170 insertions(+), 29 deletions(-) diff --git a/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/Index.razor b/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/Index.razor index 7509137e37..e740dc121c 100644 --- a/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/Index.razor +++ b/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/Index.razor @@ -4,7 +4,7 @@ Hello, world! @code { - protected override void OnAfterRender() + protected override void OnAfterRender(bool firstRender) { BenchmarkEvent.Send(JSRuntime, "Rendered index.cshtml"); } diff --git a/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/Json.razor b/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/Json.razor index e8dded3721..84722da2f1 100644 --- a/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/Json.razor +++ b/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/Json.razor @@ -37,7 +37,7 @@ largeOrgChartJson = JsonSerializer.Serialize(largeOrgChart); } - protected override void OnAfterRender() + protected override void OnAfterRender(bool firstRender) { BenchmarkEvent.Send(JSRuntime, "Finished JSON processing"); } diff --git a/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/RenderList.razor b/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/RenderList.razor index 062d3d69fd..95fcb40963 100644 --- a/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/RenderList.razor +++ b/src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/Pages/RenderList.razor @@ -46,7 +46,7 @@ Number of items: show = true; } - protected override void OnAfterRender() + protected override void OnAfterRender(bool firstRender) { BenchmarkEvent.Send(JSRuntime, "Finished rendering list"); } diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp3.0.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp3.0.cs index 39e576ab5f..468bea98e9 100644 --- a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp3.0.cs +++ b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp3.0.cs @@ -84,8 +84,8 @@ namespace Microsoft.AspNetCore.Components void Microsoft.AspNetCore.Components.IComponent.Attach(Microsoft.AspNetCore.Components.RenderHandle renderHandle) { } System.Threading.Tasks.Task Microsoft.AspNetCore.Components.IHandleAfterRender.OnAfterRenderAsync() { throw null; } System.Threading.Tasks.Task Microsoft.AspNetCore.Components.IHandleEvent.HandleEventAsync(Microsoft.AspNetCore.Components.EventCallbackWorkItem callback, object arg) { throw null; } - protected virtual void OnAfterRender() { } - protected virtual System.Threading.Tasks.Task OnAfterRenderAsync() { throw null; } + protected virtual void OnAfterRender(bool firstRender) { } + protected virtual System.Threading.Tasks.Task OnAfterRenderAsync(bool firstRender) { throw null; } protected virtual void OnInitialized() { } protected virtual System.Threading.Tasks.Task OnInitializedAsync() { throw null; } protected virtual void OnParametersSet() { } diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs index 39e576ab5f..468bea98e9 100644 --- a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs +++ b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs @@ -84,8 +84,8 @@ namespace Microsoft.AspNetCore.Components void Microsoft.AspNetCore.Components.IComponent.Attach(Microsoft.AspNetCore.Components.RenderHandle renderHandle) { } System.Threading.Tasks.Task Microsoft.AspNetCore.Components.IHandleAfterRender.OnAfterRenderAsync() { throw null; } System.Threading.Tasks.Task Microsoft.AspNetCore.Components.IHandleEvent.HandleEventAsync(Microsoft.AspNetCore.Components.EventCallbackWorkItem callback, object arg) { throw null; } - protected virtual void OnAfterRender() { } - protected virtual System.Threading.Tasks.Task OnAfterRenderAsync() { throw null; } + protected virtual void OnAfterRender(bool firstRender) { } + protected virtual System.Threading.Tasks.Task OnAfterRenderAsync(bool firstRender) { throw null; } protected virtual void OnInitialized() { } protected virtual System.Threading.Tasks.Task OnInitializedAsync() { throw null; } protected virtual void OnParametersSet() { } diff --git a/src/Components/Components/src/ComponentBase.cs b/src/Components/Components/src/ComponentBase.cs index 018c016975..51c95f386b 100644 --- a/src/Components/Components/src/ComponentBase.cs +++ b/src/Components/Components/src/ComponentBase.cs @@ -30,6 +30,7 @@ namespace Microsoft.AspNetCore.Components private bool _initialized; private bool _hasNeverRendered = true; private bool _hasPendingQueuedRender; + private bool _hasCalledOnAfterRender; /// /// Constructs an instance of . @@ -129,7 +130,17 @@ namespace Microsoft.AspNetCore.Components /// /// Method invoked after each time the component has been rendered. /// - protected virtual void OnAfterRender() + /// + /// Set to true if this is the first time has been invoked + /// on this component instance; otherwise false. + /// + /// + /// The and lifecycle methods + /// are useful for performing interop, or interacting with values recieved from @ref. + /// Use the parameter to ensure that initialization work is only performed + /// once. + /// + protected virtual void OnAfterRender(bool firstRender) { } @@ -138,8 +149,18 @@ namespace Microsoft.AspNetCore.Components /// not automatically re-render after the completion of any returned , because /// that would cause an infinite render loop. /// + /// + /// Set to true if this is the first time has been invoked + /// on this component instance; otherwise false. + /// /// A representing any asynchronous operation. - protected virtual Task OnAfterRenderAsync() + /// + /// The and lifecycle methods + /// are useful for performing interop, or interacting with values recieved from @ref. + /// Use the parameter to ensure that initialization work is only performed + /// once. + /// + protected virtual Task OnAfterRenderAsync(bool firstRender) => Task.CompletedTask; /// @@ -298,9 +319,12 @@ namespace Microsoft.AspNetCore.Components Task IHandleAfterRender.OnAfterRenderAsync() { - OnAfterRender(); + var firstRender = !_hasCalledOnAfterRender; + _hasCalledOnAfterRender |= true; - return OnAfterRenderAsync(); + OnAfterRender(firstRender); + + return OnAfterRenderAsync(firstRender); // Note that we don't call StateHasChanged to trigger a render after // handling this, because that would be an infinite loop. The only diff --git a/src/Components/Components/test/ComponentBaseTest.cs b/src/Components/Components/test/ComponentBaseTest.cs index 1f9ab22826..bd09cc49c5 100644 --- a/src/Components/Components/test/ComponentBaseTest.cs +++ b/src/Components/Components/test/ComponentBaseTest.cs @@ -3,6 +3,8 @@ using System; using System.Diagnostics; +using System.Reflection.Metadata.Ecma335; +using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Rendering; @@ -261,6 +263,98 @@ namespace Microsoft.AspNetCore.Components.Test Assert.Equal(2, renderer.Batches.Count); } + [Fact] + public async Task RunsOnAfterRender_AfterRenderingCompletes() + { + // Arrange + var renderer = new TestRenderer(); + var component = new TestComponent() { Counter = 1 }; + + var onAfterRenderCompleted = false; + component.OnAfterRenderLogic = (c, firstRender) => + { + Assert.True(firstRender); + Assert.Single(renderer.Batches); + onAfterRenderCompleted = true; + }; + + // Act + var componentId = renderer.AssignRootComponentId(component); + var renderTask = renderer.RenderRootComponentAsync(componentId); + + // Assert + await renderTask; + Assert.True(onAfterRenderCompleted); + + // Component should not be rendered again. OnAfterRender doesn't do that. + Assert.Single(renderer.Batches); + + // Act: Render again! + onAfterRenderCompleted = false; + component.OnAfterRenderLogic = (c, firstRender) => + { + Assert.False(firstRender); + Assert.Equal(2, renderer.Batches.Count); + onAfterRenderCompleted = true; + }; + + renderTask = renderer.RenderRootComponentAsync(componentId); + + // Assert + Assert.True(onAfterRenderCompleted); + Assert.Equal(2, renderer.Batches.Count); + await renderTask; + } + + [Fact] + public async Task RunsOnAfterRenderAsync_AfterRenderingCompletes() + { + // Arrange + var renderer = new TestRenderer(); + var component = new TestComponent() { Counter = 1 }; + + var onAfterRenderCompleted = false; + var tcs = new TaskCompletionSource(); + component.OnAfterRenderAsyncLogic = async (c, firstRender) => + { + Assert.True(firstRender); + Assert.Single(renderer.Batches); + onAfterRenderCompleted = true; + await tcs.Task; + }; + + // Act + var componentId = renderer.AssignRootComponentId(component); + var renderTask = renderer.RenderRootComponentAsync(componentId); + + // Assert + tcs.SetResult(null); + await renderTask; + Assert.True(onAfterRenderCompleted); + + // Component should not be rendered again. OnAfterRenderAsync doesn't do that. + Assert.Single(renderer.Batches); + + // Act: Render again! + onAfterRenderCompleted = false; + tcs = new TaskCompletionSource(); + component.OnAfterRenderAsyncLogic = async (c, firstRender) => + { + Assert.False(firstRender); + Assert.Equal(2, renderer.Batches.Count); + onAfterRenderCompleted = true; + await tcs.Task; + }; + + renderTask = renderer.RenderRootComponentAsync(componentId); + + // Assert + tcs.SetResult(null); + await renderTask; + Assert.True(onAfterRenderCompleted); + Assert.Equal(2, renderer.Batches.Count); + } + [Fact] public async Task DoesNotRenderAfterOnInitAsyncTaskIsCancelledUsingCancellationToken() { @@ -386,6 +480,10 @@ namespace Microsoft.AspNetCore.Components.Test public bool RunsBaseOnParametersSetAsync { get; set; } = true; + public bool RunsBaseOnAfterRender { get; set; } = true; + + public bool RunsBaseOnAfterRenderAsync { get; set; } = true; + public Action OnInitLogic { get; set; } public Func OnInitAsyncLogic { get; set; } @@ -394,6 +492,10 @@ namespace Microsoft.AspNetCore.Components.Test public Func OnParametersSetAsyncLogic { get; set; } + public Action OnAfterRenderLogic { get; set; } + + public Func OnAfterRenderAsyncLogic { get; set; } + public int Counter { get; set; } protected override void BuildRenderTree(RenderTreeBuilder builder) @@ -448,6 +550,32 @@ namespace Microsoft.AspNetCore.Components.Test await OnParametersSetAsyncLogic(this); } } + + protected override void OnAfterRender(bool firstRender) + { + if (RunsBaseOnAfterRender) + { + base.OnAfterRender(firstRender); + } + + if (OnAfterRenderLogic != null) + { + OnAfterRenderLogic(this, firstRender); + } + } + + protected override async Task OnAfterRenderAsync(bool firstRender) + { + if (RunsBaseOnAfterRenderAsync) + { + await base.OnAfterRenderAsync(firstRender); + } + + if (OnAfterRenderAsyncLogic != null) + { + await OnAfterRenderAsyncLogic(this, firstRender); + } + } } } } diff --git a/src/Components/Components/test/RendererTest.cs b/src/Components/Components/test/RendererTest.cs index 677b9fb952..fbd91decf7 100644 --- a/src/Components/Components/test/RendererTest.cs +++ b/src/Components/Components/test/RendererTest.cs @@ -4244,7 +4244,7 @@ namespace Microsoft.AspNetCore.Components.Test renderFactory(this)(builder); } - protected override async Task OnAfterRenderAsync() + protected override async Task OnAfterRenderAsync(bool firstRender) { if (TryGetEntry(EventType.OnAfterRenderAsyncSync, out var entrySync)) { diff --git a/src/Components/test/testassets/BasicTestApp/AfterRenderInteropComponent.razor b/src/Components/test/testassets/BasicTestApp/AfterRenderInteropComponent.razor index 57f99f485a..41bcfe275d 100644 --- a/src/Components/test/testassets/BasicTestApp/AfterRenderInteropComponent.razor +++ b/src/Components/test/testassets/BasicTestApp/AfterRenderInteropComponent.razor @@ -6,7 +6,7 @@ @code { ElementReference myInput; - protected override void OnAfterRender() + protected override void OnAfterRender(bool firstRender) { JSRuntime.InvokeAsync("setElementValue", myInput, "Value set after render"); } diff --git a/src/Components/test/testassets/BasicTestApp/InteropOnInitializationComponent.razor b/src/Components/test/testassets/BasicTestApp/InteropOnInitializationComponent.razor index ae3f3491e3..b6c0b4e512 100644 --- a/src/Components/test/testassets/BasicTestApp/InteropOnInitializationComponent.razor +++ b/src/Components/test/testassets/BasicTestApp/InteropOnInitializationComponent.razor @@ -26,17 +26,9 @@ string infoFromJs; ElementReference myElem; - protected override async Task OnAfterRenderAsync() + protected override async Task OnAfterRenderAsync(bool firstRender) { - // TEMPORARY: Currently we need this guard to avoid making the interop - // call during prerendering. Soon this will be unnecessary because we - // will change OnAfterRenderAsync not to run during the prerendering phase. - if (!ComponentContext.IsConnected) - { - return; - } - - if (infoFromJs == null) + if (firstRender) { // We can only use the ElementRef in OnAfterRenderAsync (and not any // earlier lifecycle method), because there is no JS element until diff --git a/src/Components/test/testassets/BasicTestApp/PrerenderedToInteractiveTransition.razor b/src/Components/test/testassets/BasicTestApp/PrerenderedToInteractiveTransition.razor index 5fd043e055..59020435e3 100644 --- a/src/Components/test/testassets/BasicTestApp/PrerenderedToInteractiveTransition.razor +++ b/src/Components/test/testassets/BasicTestApp/PrerenderedToInteractiveTransition.razor @@ -23,13 +23,10 @@ @code { int count; - bool firstRender = false; - protected override Task OnAfterRenderAsync() + protected override Task OnAfterRenderAsync(bool firstRender) { - if (!firstRender) + if (firstRender) { - firstRender = true; - // We need to queue another render when we connect, otherwise the // browser won't see anything. StateHasChanged(); diff --git a/src/Mvc/Mvc.ViewFeatures/test/HtmlHelperComponentExtensionsTests.cs b/src/Mvc/Mvc.ViewFeatures/test/HtmlHelperComponentExtensionsTests.cs index 421fc49d37..3947f3fd5a 100644 --- a/src/Mvc/Mvc.ViewFeatures/test/HtmlHelperComponentExtensionsTests.cs +++ b/src/Mvc/Mvc.ViewFeatures/test/HtmlHelperComponentExtensionsTests.cs @@ -333,7 +333,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Test { [Parameter] public OnAfterRenderState State { get; set; } - protected override void OnAfterRender() + protected override void OnAfterRender(bool firstRender) { State.OnAfterRenderRan = true; }