From ec1a2751188b9372a7e15eb7f93d22256ef1bb27 Mon Sep 17 00:00:00 2001 From: Egil Hansen Date: Fri, 10 May 2024 15:27:17 +0000 Subject: [PATCH] refactor - wip - only generate markup for root components --- src/bunit/Rendering/BunitRenderer.cs | 69 ++++------ src/bunit/Rendering/IRenderedComponent.cs | 12 +- src/bunit/Rendering/Internal/Htmlizer.cs | 27 +++- src/bunit/Rendering/RenderedComponent.cs | 130 ++++++++++-------- tests/bunit.tests/BunitContextTest.cs | 4 +- ...RendererBunits.cs => BunitRendererTest.cs} | 27 +--- .../Rendering/RenderedComponentTest.cs | 6 +- 7 files changed, 146 insertions(+), 129 deletions(-) rename tests/bunit.tests/Rendering/{BunitRendererBunits.cs => BunitRendererTest.cs} (96%) diff --git a/src/bunit/Rendering/BunitRenderer.cs b/src/bunit/Rendering/BunitRenderer.cs index 81ed0f0a4..99c1661ba 100644 --- a/src/bunit/Rendering/BunitRenderer.cs +++ b/src/bunit/Rendering/BunitRenderer.cs @@ -14,7 +14,7 @@ public sealed class BunitRenderer : Renderer { private readonly BunitServiceProvider services; private readonly List disposalTasks = []; - private static readonly ConcurrentDictionary componentActivatorCache = new(); + private static readonly ConcurrentDictionary ComponentActivatorCache = new(); [UnsafeAccessor(UnsafeAccessorKind.Field, Name = "_isBatchInProgress")] private static extern ref bool GetIsBatchInProgressField(Renderer renderer); @@ -224,7 +224,7 @@ protected override ComponentState CreateComponentState(int componentId, ICompone object CreateComponentInstance() { - var constructorInfo = componentActivatorCache.GetOrAdd(renderedComponentType, type + var constructorInfo = ComponentActivatorCache.GetOrAdd(renderedComponentType, type => type.GetConstructor( [ typeof(BunitRenderer), @@ -349,65 +349,50 @@ protected override Task UpdateDisplayAsync(in RenderBatch renderBatch) for (var i = 0; i < renderBatch.UpdatedComponents.Count; i++) { var diff = renderBatch.UpdatedComponents.Array[i]; - var componentState = GetComponentState(diff.ComponentId); - var renderedComponent = (IRenderedComponent)componentState; + var componentState = GetRenderedComponent(diff.ComponentId); + componentState.RenderCount++; - if (returnedRenderedComponentIds.Contains(diff.ComponentId)) - { - renderedComponent.UpdateState(hasRendered: true, isMarkupGenerationRequired: diff.Edits.Count > 0); - } - else + componentState.IsDirty = true; + + if (componentState.Root is not null) { - renderedComponent.UpdateState(hasRendered: true, false); + componentState.Root.IsDirty = true; } - - UpdateParents(diff.Edits.Count > 0, componentState, in renderBatch); } - return Task.CompletedTask; - - void UpdateParents(bool hasChanges, ComponentState componentState, in RenderBatch renderBatch) + foreach (var item in rootComponents) { - var parent = componentState.ParentComponentState; - if (parent is null) + var root = GetRenderedComponent(item); + if (root.IsDirty) { - return; - } - - if (!IsParentComponentAlreadyUpdated(parent.ComponentId, in renderBatch)) - { - if (returnedRenderedComponentIds.Contains(parent.ComponentId)) - { - ((IRenderedComponent)parent).UpdateState(hasRendered: true, isMarkupGenerationRequired: hasChanges); - } - else - { - ((IRenderedComponent)parent).UpdateState(hasRendered: true, false); - } - - UpdateParents(hasChanges, parent, in renderBatch); + root.UpdateMarkup(); } } - static bool IsParentComponentAlreadyUpdated(int componentId, in RenderBatch renderBatch) + foreach (var renderedComponentId in returnedRenderedComponentIds) { - for (var i = 0; i < renderBatch.UpdatedComponents.Count; i++) + var renderedComponent = GetRenderedComponent(renderedComponentId); + if (renderedComponent.IsDirty) { - var diff = renderBatch.UpdatedComponents.Array[i]; - if (diff.ComponentId == componentId) - { - return diff.Edits.Count > 0; - } + renderedComponent.UpdateMarkup(); } - - return false; } + + return Task.CompletedTask; } /// internal new ArrayRange GetCurrentRenderTreeFrames(int componentId) => base.GetCurrentRenderTreeFrames(componentId); + /// + internal IRenderedComponent GetRenderedComponent(int componentId) + => (IRenderedComponent)GetComponentState(componentId); + + /// + internal IRenderedComponent GetRenderedComponent(IComponent component) + => (IRenderedComponent)GetComponentState(component); + /// protected override void Dispose(bool disposing) { @@ -487,7 +472,7 @@ private List> FindComponents(IRendere FindComponentsInRenderTree(parentComponent.ComponentId); foreach (var rc in result) { - ((IRenderedComponent)rc).UpdateState(hasRendered: false, isMarkupGenerationRequired: true); + ((IRenderedComponent)rc).UpdateMarkup(); } } diff --git a/src/bunit/Rendering/IRenderedComponent.cs b/src/bunit/Rendering/IRenderedComponent.cs index b47604940..6f4d7cc8b 100644 --- a/src/bunit/Rendering/IRenderedComponent.cs +++ b/src/bunit/Rendering/IRenderedComponent.cs @@ -11,9 +11,17 @@ internal interface IRenderedComponent : IDisposable int ComponentId { get; } /// - /// Called by the owning when it finishes a render. + /// Gets the total number times the fragment has been through its render life-cycle. /// - void UpdateState(bool hasRendered, bool isMarkupGenerationRequired); + int RenderCount { get; set; } + + void UpdateMarkup(); + + void SetMarkupIndices(int start, int end); + + bool IsDirty { get; set; } + + IRenderedComponent? Root { get; } } /// diff --git a/src/bunit/Rendering/Internal/Htmlizer.cs b/src/bunit/Rendering/Internal/Htmlizer.cs index db786f266..9d93ebca8 100644 --- a/src/bunit/Rendering/Internal/Htmlizer.cs +++ b/src/bunit/Rendering/Internal/Htmlizer.cs @@ -55,21 +55,35 @@ public static string ToBlazorAttribute(string attributeName) public static string GetHtml(int componentId, BunitRenderer renderer) { var context = new HtmlRenderingContext(renderer); + var componentState = renderer.GetRenderedComponent(componentId); var frames = context.GetRenderTreeFrames(componentId); var newPosition = RenderFrames(context, frames, 0, frames.Count); + + componentState.SetMarkupIndices(0, context.Result.Length); + Debug.Assert( newPosition == frames.Count, $"frames.Length = {frames.Count}. newPosition = {newPosition}" ); + return context.Result.ToString(); } + private static RenderTreeFrame RenderComponent(HtmlRenderingContext context, in RenderTreeFrame frame) + { + var startIndex = context.Result.Length; + var frames = context.GetRenderTreeFrames(frame.ComponentId); + RenderFrames(context, frames, 0, frames.Count); + var endIndex = context.Result.Length; + context.GetRenderedComponent(frame.ComponentId).SetMarkupIndices(startIndex, endIndex); + return frame; + } + private static int RenderFrames( HtmlRenderingContext context, ArrayRange frames, int position, - int maxElements - ) + int maxElements) { var nextPosition = position; var endPosition = position + maxElements; @@ -130,12 +144,10 @@ int position private static int RenderChildComponent( HtmlRenderingContext context, ArrayRange frames, - int position - ) + int position) { var frame = frames.Array[position]; - var childFrames = context.GetRenderTreeFrames(frame.ComponentId); - RenderFrames(context, childFrames, 0, childFrames.Count); + frame = RenderComponent(context, in frame); return position + frame.ComponentSubtreeLength; } @@ -405,6 +417,9 @@ public HtmlRenderingContext(BunitRenderer renderer) public ArrayRange GetRenderTreeFrames(int componentId) => renderer.GetCurrentRenderTreeFrames(componentId); + public IRenderedComponent GetRenderedComponent(int componentId) + => renderer.GetRenderedComponent(componentId); + public StringBuilder Result { get; } = new(); public string? ClosestSelectValueAsString { get; set; } diff --git a/src/bunit/Rendering/RenderedComponent.cs b/src/bunit/Rendering/RenderedComponent.cs index afd868453..f0ee9b914 100644 --- a/src/bunit/Rendering/RenderedComponent.cs +++ b/src/bunit/Rendering/RenderedComponent.cs @@ -16,10 +16,14 @@ internal sealed class RenderedComponent : ComponentState, IRenderedC [SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "Owned by BunitServiceProvider, disposed by it.")] private readonly BunitHtmlParser htmlParser; - + private int renderCount; private string markup = string.Empty; + private int markupStartIndex; + private int markupEndIndex; private INodeList? latestRenderNodes; + public bool IsDirty { get; set; } + /// /// Gets the component under test. /// @@ -53,10 +57,22 @@ public string Markup } } + /// + /// Adds or removes an event handler that will be triggered after + /// each render of this . + /// + public event EventHandler? OnAfterRender; + + /// + /// An event that is raised after the markup of the + /// is updated. + /// + public event EventHandler? OnMarkupUpdated; + /// /// Gets the total number times the fragment has been through its render life-cycle. /// - public int RenderCount { get; private set; } + public int RenderCount => renderCount; /// /// Gets the AngleSharp based @@ -77,6 +93,10 @@ public INodeList Nodes /// public IServiceProvider Services { get; } + int IRenderedComponent.RenderCount { get => renderCount; set { renderCount = value; } } + + public IRenderedComponent? Root { get; } + public RenderedComponent( BunitRenderer renderer, int componentId, @@ -89,57 +109,76 @@ public RenderedComponent( this.renderer = renderer; this.instance = (TComponent)instance; htmlParser = Services.GetRequiredService(); + var parentRenderedComponent = parentComponentState as IRenderedComponent; + Root = parentRenderedComponent?.Root ?? parentRenderedComponent; } - /// - /// Adds or removes an event handler that will be triggered after each render of this . - /// - public event EventHandler? OnAfterRender; + /// + public void Dispose() + { + if (IsDisposed) + return; - /// - /// An event that is raised after the markup of the is updated. - /// - public event EventHandler? OnMarkupUpdated; + if (Root is not null) + Root.IsDirty = true; + + IsDisposed = true; + markup = string.Empty; + OnAfterRender = null; + OnMarkupUpdated = null; + } + + /// + public override ValueTask DisposeAsync() + { + Dispose(); + return base.DisposeAsync(); + } + + public void SetMarkupIndices(int start, int end) + { + markupStartIndex = start; + markupEndIndex = end; + IsDirty = true; + } /// /// Called by the owning when it finishes a render. /// - public void UpdateState(bool hasRendered, bool isMarkupGenerationRequired) + public void UpdateMarkup() { if (IsDisposed) return; - if (hasRendered) + if (Root is RenderedComponent root) { - RenderCount++; + var newMarkup = root.markup[markupStartIndex..markupEndIndex]; + if (markup != newMarkup) + { + Volatile.Write(ref markup, newMarkup); + latestRenderNodes = null; + OnMarkupUpdated?.Invoke(this, EventArgs.Empty); + } + else + { + // no change + } } - - if (isMarkupGenerationRequired) + else { - UpdateMarkup(); + var newMarkup = Htmlizer.GetHtml(ComponentId, renderer); + + // Volatile write is necessary to ensure the updated markup + // is available across CPU cores. Without it, the pointer to the + // markup string can be stored in a CPUs register and not + // get updated when another CPU changes the string. + Volatile.Write(ref markup, newMarkup); + latestRenderNodes = null; OnMarkupUpdated?.Invoke(this, EventArgs.Empty); } - // The order here is important, since consumers of the events - // expect that markup has indeed changed when OnAfterRender is invoked - // (assuming there are markup changes) - if (hasRendered) - OnAfterRender?.Invoke(this, EventArgs.Empty); - } - - /// - /// Updates the markup of the rendered fragment. - /// - private void UpdateMarkup() - { - latestRenderNodes = null; - var newMarkup = Htmlizer.GetHtml(ComponentId, renderer); - - // Volatile write is necessary to ensure the updated markup - // is available across CPU cores. Without it, the pointer to the - // markup string can be stored in a CPUs register and not - // get updated when another CPU changes the string. - Volatile.Write(ref markup, newMarkup); + IsDirty = false; + OnAfterRender?.Invoke(this, EventArgs.Empty); } /// @@ -151,22 +190,5 @@ private void EnsureComponentNotDisposed() if (IsDisposed) throw new ComponentDisposedException(ComponentId); } - - /// - public void Dispose() - { - if (IsDisposed) - return; - - IsDisposed = true; - markup = string.Empty; - OnAfterRender = null; - OnMarkupUpdated = null; - } - - public override ValueTask DisposeAsync() - { - Dispose(); - return base.DisposeAsync(); - } } + diff --git a/tests/bunit.tests/BunitContextTest.cs b/tests/bunit.tests/BunitContextTest.cs index 38669ea65..661064491 100644 --- a/tests/bunit.tests/BunitContextTest.cs +++ b/tests/bunit.tests/BunitContextTest.cs @@ -93,7 +93,7 @@ public void Test031() .ShouldBe("FOO"); } - [Fact(DisplayName = "Render(builder) renders TComponent inside RenderTreee")] + [Fact(DisplayName = "Render(builder) renders TComponent inside RenderTree")] public void Test032() { RenderTree.Add>(ps => ps.Add(p => p.Value, "FOO")); @@ -104,7 +104,7 @@ public void Test032() .ShouldBe("FOO"); } - [Fact(DisplayName = "Render(factories) renders TComponent inside RenderTreee")] + [Fact(DisplayName = "Render(factories) renders TComponent inside RenderTree")] public void Test033() { RenderTree.Add>(ps => ps.Add(p => p.Value, "FOO")); diff --git a/tests/bunit.tests/Rendering/BunitRendererBunits.cs b/tests/bunit.tests/Rendering/BunitRendererTest.cs similarity index 96% rename from tests/bunit.tests/Rendering/BunitRendererBunits.cs rename to tests/bunit.tests/Rendering/BunitRendererTest.cs index 17e6a5fb5..c000637e9 100644 --- a/tests/bunit.tests/Rendering/BunitRendererBunits.cs +++ b/tests/bunit.tests/Rendering/BunitRendererTest.cs @@ -3,11 +3,11 @@ namespace Bunit.Rendering; -public class BunitRendererBunits : BunitContext +public class BunitRendererTest : BunitContext { - public BunitRendererBunits(ITestOutputHelper outputHelper) + public BunitRendererTest(ITestOutputHelper outputHelper) { - DefaultWaitTimeout = TimeSpan.FromSeconds(30); + //DefaultWaitTimeout = TimeSpan.FromSeconds(30); Services.AddXunitLogger(outputHelper); } @@ -159,7 +159,7 @@ public void Test030() .AddChildContent(pps => pps .Add(p => p.Value, PARENT_VALUE) .AddChildContent(ppps => ppps - .Add(p=>p.Value, CHILD_VALUE)))); + .Add(p => p.Value, CHILD_VALUE)))); // act var childCuts = Renderer.FindComponents(cut) @@ -230,22 +230,6 @@ public async Task Test041() cut.Markup.ShouldBe("X"); } - [Fact(DisplayName = "Rendered component updates on re-renders from child components with changes in render tree")] - public async Task Test050() - { - // arrange - var cut = Renderer.Render(ps => ps - .AddChildContent()); - var child = Renderer.FindComponent(cut); - - // act - await child.Instance.TriggerWithValue("X"); - - // assert - cut.RenderCount.ShouldBe(2); - cut.Markup.ShouldBe("X"); - } - [Fact(DisplayName = "When component is disposed by renderer, getting Markup throws and IsDisposed returns true")] public async Task Test060() { @@ -507,6 +491,9 @@ internal sealed class HasParams : ComponentBase [Parameter] public string? Value { get; set; } [Parameter] public RenderFragment? ChildContent { get; set; } + public override Task SetParametersAsync(ParameterView parameters) + => base.SetParametersAsync(parameters); + protected override void BuildRenderTree(RenderTreeBuilder builder) { builder.AddMarkupContent(0, Value); diff --git a/tests/bunit.tests/Rendering/RenderedComponentTest.cs b/tests/bunit.tests/Rendering/RenderedComponentTest.cs index df463be29..ed5362c54 100644 --- a/tests/bunit.tests/Rendering/RenderedComponentTest.cs +++ b/tests/bunit.tests/Rendering/RenderedComponentTest.cs @@ -57,7 +57,7 @@ public void Test020() Should.Throw(() => target.Instance); } - + [Fact(DisplayName = "Find throws an exception if no element matches the css selector")] public void Test021() { @@ -197,13 +197,13 @@ public void Test010() first.Render(); - wrapper.RenderCount.ShouldBe(2); + wrapper.RenderCount.ShouldBe(1); first.RenderCount.ShouldBe(2); second.RenderCount.ShouldBe(1); second.Render(); - wrapper.RenderCount.ShouldBe(3); + wrapper.RenderCount.ShouldBe(1); first.RenderCount.ShouldBe(2); second.RenderCount.ShouldBe(2); }