From 4acb0c5e1ec9f7a2e639165907922c5152557f6a Mon Sep 17 00:00:00 2001 From: Mackinnon Buck Date: Thu, 8 Aug 2024 17:14:29 -0700 Subject: [PATCH 1/4] Allow cascading value subscribers to change during notification --- .../Components/src/CascadingValueSource.cs | 3 +- .../Components/test/CascadingParameterTest.cs | 92 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/Components/Components/src/CascadingValueSource.cs b/src/Components/Components/src/CascadingValueSource.cs index dbf9a4662265..8646aca3173a 100644 --- a/src/Components/Components/src/CascadingValueSource.cs +++ b/src/Components/Components/src/CascadingValueSource.cs @@ -96,7 +96,8 @@ public Task NotifyChangedAsync() { tasks.Add(dispatcher.InvokeAsync(() => { - foreach (var subscriber in subscribers) + Span subscribersCopy = [.. subscribers]; + foreach (var subscriber in subscribersCopy) { subscriber.NotifyCascadingValueChanged(ParameterViewLifetime.Unbound); } diff --git a/src/Components/Components/test/CascadingParameterTest.cs b/src/Components/Components/test/CascadingParameterTest.cs index a99baeb96833..ddbc10ec6398 100644 --- a/src/Components/Components/test/CascadingParameterTest.cs +++ b/src/Components/Components/test/CascadingParameterTest.cs @@ -634,6 +634,61 @@ public async Task CanTriggerUpdatesOnCascadingValuesFromServiceProvider() await cascadingValueSource.NotifyChangedAsync(new MyParamType("Nobody is listening, but this shouldn't be an error")); } + [Fact] + public async Task CanAddSubscriberDuringChangeNotification() + { + // Arrange + var services = new ServiceCollection(); + var paramValue = new MyParamType("Initial value"); + var cascadingValueSource = new CascadingValueSource(paramValue, isFixed: false); + services.AddCascadingValue(_ => cascadingValueSource); + var renderer = new TestRenderer(services.BuildServiceProvider()); + var component = new ConditionallyRenderSubscriberComponent() + { + RenderWhenEqualTo = "Final value", + }; + + // Act/Assert 1: Initial render + var componentId = await renderer.Dispatcher.InvokeAsync(() => renderer.AssignRootComponentId(component)); + renderer.RenderRootComponent(componentId); + var firstBatch = renderer.Batches.Single(); + var diff = firstBatch.DiffsByComponentId[componentId].Single(); + Assert.Collection(diff.Edits, + edit => + { + Assert.Equal(RenderTreeEditType.PrependFrame, edit.Type); + AssertFrame.Text( + firstBatch.ReferenceFrames[edit.ReferenceFrameIndex], + "CascadingParameter=Initial value"); + }); + Assert.Equal(1, component.NumRenders); + + // Act: Second render + paramValue.ChangeValue("Final value"); + await cascadingValueSource.NotifyChangedAsync(); + var secondBatch = renderer.Batches[1]; + var diff2 = secondBatch.DiffsByComponentId[componentId].Single(); + + // Assert: Subscriber can get added during change notification and receive the cascading value + AssertFrame.Text( + secondBatch.ReferenceFrames[diff2.Edits[0].ReferenceFrameIndex], + "CascadingParameter=Final value"); + Assert.Equal(2, component.NumRenders); + + // Assert: Subscriber can get added during change notification and receive the cascading value + var nestedComponent = FindComponent(secondBatch, out var nestedComponentId); + var nestedComponentDiff = secondBatch.DiffsByComponentId[nestedComponentId].Single(); + Assert.Collection(nestedComponentDiff.Edits, + edit => + { + Assert.Equal(RenderTreeEditType.PrependFrame, edit.Type); + AssertFrame.Text( + secondBatch.ReferenceFrames[edit.ReferenceFrameIndex], + "CascadingParameter=Final value"); + }); + Assert.Equal(1, nestedComponent.NumRenders); + } + [Fact] public async Task AfterSupplyingValueThroughNotifyChanged_InitialValueFactoryIsNotUsed() { @@ -861,6 +916,43 @@ public void AttemptIllegalAccessToLastParameterView() } } + class ConditionallyRenderSubscriberComponent : AutoRenderComponent + { + public int NumRenders { get; private set; } + + public SimpleSubscriberComponent NestedSubscriber { get; private set; } + + [Parameter] public string RenderWhenEqualTo { get; set; } + + [CascadingParameter] MyParamType CascadingParameter { get; set; } + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + NumRenders++; + builder.AddContent(0, $"CascadingParameter={CascadingParameter}"); + + if (string.Equals(RenderWhenEqualTo, CascadingParameter.ToString(), StringComparison.OrdinalIgnoreCase)) + { + builder.OpenComponent(1); + builder.AddComponentReferenceCapture(2, component => NestedSubscriber = component as SimpleSubscriberComponent); + builder.CloseComponent(); + } + } + } + + class SimpleSubscriberComponent : AutoRenderComponent + { + public int NumRenders { get; private set; } + + [CascadingParameter] MyParamType CascadingParameter { get; set; } + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + NumRenders++; + builder.AddContent(0, $"CascadingParameter={CascadingParameter}"); + } + } + class SingleDeliveryParameterConsumerComponent : AutoRenderComponent { public int NumSetParametersCalls { get; private set; } From 1ae82bb85233187542cf79815c468f2f321727d7 Mon Sep 17 00:00:00 2001 From: Mackinnon Buck Date: Fri, 9 Aug 2024 09:05:44 -0700 Subject: [PATCH 2/4] PR feedback --- src/Components/Components/src/CascadingValueSource.cs | 5 +++-- src/Components/Components/test/CascadingParameterTest.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Components/Components/src/CascadingValueSource.cs b/src/Components/Components/src/CascadingValueSource.cs index 8646aca3173a..2d3f20cfe9ef 100644 --- a/src/Components/Components/src/CascadingValueSource.cs +++ b/src/Components/Components/src/CascadingValueSource.cs @@ -96,8 +96,9 @@ public Task NotifyChangedAsync() { tasks.Add(dispatcher.InvokeAsync(() => { - Span subscribersCopy = [.. subscribers]; - foreach (var subscriber in subscribersCopy) + // We iterate over a copy of the list because new subscribers might get + // added or removed during change notification + foreach (var subscriber in subscribers.ToArray()) { subscriber.NotifyCascadingValueChanged(ParameterViewLifetime.Unbound); } diff --git a/src/Components/Components/test/CascadingParameterTest.cs b/src/Components/Components/test/CascadingParameterTest.cs index ddbc10ec6398..3ca3efb58772 100644 --- a/src/Components/Components/test/CascadingParameterTest.cs +++ b/src/Components/Components/test/CascadingParameterTest.cs @@ -648,7 +648,7 @@ public async Task CanAddSubscriberDuringChangeNotification() RenderWhenEqualTo = "Final value", }; - // Act/Assert 1: Initial render + // Act/Assert: Initial render var componentId = await renderer.Dispatcher.InvokeAsync(() => renderer.AssignRootComponentId(component)); renderer.RenderRootComponent(componentId); var firstBatch = renderer.Batches.Single(); From 655b8712699eb43f44c6d4cdb331c20dd9f885f9 Mon Sep 17 00:00:00 2001 From: Mackinnon Buck Date: Fri, 9 Aug 2024 09:46:11 -0700 Subject: [PATCH 3/4] Use `[InlineArray]` --- .../Components/src/CascadingValueSource.cs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Components/Components/src/CascadingValueSource.cs b/src/Components/Components/src/CascadingValueSource.cs index 2d3f20cfe9ef..eecccf8efe04 100644 --- a/src/Components/Components/src/CascadingValueSource.cs +++ b/src/Components/Components/src/CascadingValueSource.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Concurrent; +using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Components.Rendering; namespace Microsoft.AspNetCore.Components; @@ -96,9 +97,16 @@ public Task NotifyChangedAsync() { tasks.Add(dispatcher.InvokeAsync(() => { + var subscribersBuffer = new ComponentStateBuffer(); + var subscribersCount = subscribers.Count; + var subscribersCopy = subscribersCount <= ComponentStateBuffer.Capacity + ? subscribersBuffer[..subscribersCount] + : new ComponentState[subscribersCount]; + subscribers.CopyTo(subscribersCopy); + // We iterate over a copy of the list because new subscribers might get // added or removed during change notification - foreach (var subscriber in subscribers.ToArray()) + foreach (var subscriber in subscribersCopy) { subscriber.NotifyCascadingValueChanged(ParameterViewLifetime.Unbound); } @@ -176,4 +184,13 @@ void ICascadingValueSupplier.Unsubscribe(ComponentState subscriber, in Cascading } } } + + [InlineArray(Capacity)] + private struct ComponentStateBuffer + { + public const int Capacity = 64; +#pragma warning disable IDE0051 // Remove unused private members + private ComponentState _values; +#pragma warning restore IDE0051 // Remove unused private members + } } From 0620dd098793f7c6585a39bb9bcba2002bceb4c1 Mon Sep 17 00:00:00 2001 From: Mackinnon Buck Date: Fri, 9 Aug 2024 10:08:42 -0700 Subject: [PATCH 4/4] Suppress warning + add tests --- .../Components/src/CascadingValueSource.cs | 4 ++- .../Components/test/CascadingParameterTest.cs | 34 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/Components/Components/src/CascadingValueSource.cs b/src/Components/Components/src/CascadingValueSource.cs index eecccf8efe04..3645b17bed39 100644 --- a/src/Components/Components/src/CascadingValueSource.cs +++ b/src/Components/Components/src/CascadingValueSource.cs @@ -186,11 +186,13 @@ void ICascadingValueSupplier.Unsubscribe(ComponentState subscriber, in Cascading } [InlineArray(Capacity)] - private struct ComponentStateBuffer + internal struct ComponentStateBuffer { public const int Capacity = 64; #pragma warning disable IDE0051 // Remove unused private members +#pragma warning disable IDE0044 // Add readonly modifier private ComponentState _values; +#pragma warning restore IDE0044 // Add readonly modifier #pragma warning restore IDE0051 // Remove unused private members } } diff --git a/src/Components/Components/test/CascadingParameterTest.cs b/src/Components/Components/test/CascadingParameterTest.cs index 3ca3efb58772..411ed14a5e07 100644 --- a/src/Components/Components/test/CascadingParameterTest.cs +++ b/src/Components/Components/test/CascadingParameterTest.cs @@ -827,6 +827,40 @@ public void CanUseTryAddPatternForCascadingValuesInServiceCollection_CascadingVa Assert.Equal(2, services.Count()); } + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(CascadingValueSource.ComponentStateBuffer.Capacity - 1)] + [InlineData(CascadingValueSource.ComponentStateBuffer.Capacity)] + [InlineData(CascadingValueSource.ComponentStateBuffer.Capacity + 1)] + [InlineData(CascadingValueSource.ComponentStateBuffer.Capacity * 2)] + public async Task CanHaveManySubscribers(int numSubscribers) + { + // Arrange + var services = new ServiceCollection(); + var paramValue = new MyParamType("Initial value"); + var cascadingValueSource = new CascadingValueSource(paramValue, isFixed: false); + services.AddCascadingValue(_ => cascadingValueSource); + var renderer = new TestRenderer(services.BuildServiceProvider()); + var components = Enumerable.Range(0, numSubscribers).Select(_ => new SimpleSubscriberComponent()).ToArray(); + + // Act/Assert: Initial render + foreach (var component in components) + { + await renderer.Dispatcher.InvokeAsync(() => renderer.AssignRootComponentId(component)); + component.TriggerRender(); + Assert.Equal(1, component.NumRenders); + } + + // Act/Assert: All components re-render when the cascading value changes + paramValue.ChangeValue("Final value"); + await cascadingValueSource.NotifyChangedAsync(); + foreach (var component in components) + { + Assert.Equal(2, component.NumRenders); + } + } + private class SingleDeliveryValue(string text) { public string Text => text;