From 18df30568cfdc18a3498356b5ed85afb39ecc61e Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Mon, 15 Oct 2018 13:00:17 +0100 Subject: [PATCH] Support "Fixed" mode for --- .../Components/CascadingValue.cs | 33 ++++++ .../Components/ICascadingValueComponent.cs | 2 + .../Rendering/ComponentState.cs | 31 +++-- .../CascadingParameterTest.cs | 108 ++++++++++++++++++ .../ParameterCollectionTest.cs | 2 + 5 files changed, 165 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.AspNetCore.Blazor/Components/CascadingValue.cs b/src/Microsoft.AspNetCore.Blazor/Components/CascadingValue.cs index 5c0ac51950..d04a07646e 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/CascadingValue.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/CascadingValue.cs @@ -15,6 +15,7 @@ namespace Microsoft.AspNetCore.Blazor.Components { private RenderHandle _renderHandle; private HashSet _subscribers; // Lazily instantiated + private bool _hasSetParametersPreviously; /// /// The content to which the value should be provided. @@ -35,8 +36,18 @@ namespace Microsoft.AspNetCore.Blazor.Components /// [Parameter] private string Name { get; set; } + /// + /// If true, indicates that will not change. This is a + /// performance optimization that allows the framework to skip setting up + /// change notifications. Set this flag only if you will not change + /// during the component's lifetime. + /// + [Parameter] private bool Fixed { get; set; } + object ICascadingValueComponent.CurrentValue => Value; + bool ICascadingValueComponent.CurrentValueIsFixed => Fixed; + /// public void Init(RenderHandle renderHandle) { @@ -52,9 +63,11 @@ namespace Microsoft.AspNetCore.Blazor.Components var hasSuppliedValue = false; var previousValue = Value; + var previousFixed = Fixed; Value = default; ChildContent = null; Name = null; + Fixed = false; foreach (var parameter in parameters) { @@ -75,12 +88,23 @@ namespace Microsoft.AspNetCore.Blazor.Components throw new ArgumentException($"The parameter '{nameof(Name)}' for component '{nameof(CascadingValue)}' does not allow null or empty values."); } } + else if (parameter.Name.Equals(nameof(Fixed), StringComparison.OrdinalIgnoreCase)) + { + Fixed = (bool)parameter.Value; + } else { throw new ArgumentException($"The component '{nameof(CascadingValue)}' does not accept a parameter with the name '{parameter.Name}'."); } } + if (_hasSetParametersPreviously && Fixed != previousFixed) + { + throw new InvalidOperationException($"The value of {nameof(Fixed)} cannot be changed dynamically."); + } + + _hasSetParametersPreviously = true; + // It's OK for the value to be null, but some "Value" param must be suppled // because it serves no useful purpose to have a otherwise. if (!hasSuppliedValue) @@ -120,6 +144,15 @@ namespace Microsoft.AspNetCore.Blazor.Components void ICascadingValueComponent.Subscribe(ComponentState subscriber) { +#if DEBUG + if (Fixed) + { + // Should not be possible. User code cannot trigger this. + // Checking only to catch possible future framework bugs. + throw new InvalidOperationException($"Cannot subscribe to a {typeof(CascadingValue<>).Name} when {nameof(Fixed)} is true."); + } +#endif + if (_subscribers == null) { _subscribers = new HashSet(); diff --git a/src/Microsoft.AspNetCore.Blazor/Components/ICascadingValueComponent.cs b/src/Microsoft.AspNetCore.Blazor/Components/ICascadingValueComponent.cs index 2cf5349dd6..d64940e74b 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/ICascadingValueComponent.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/ICascadingValueComponent.cs @@ -15,6 +15,8 @@ namespace Microsoft.AspNetCore.Blazor.Components object CurrentValue { get; } + bool CurrentValueIsFixed { get; } + void Subscribe(ComponentState subscriber); void Unsubscribe(ComponentState subscriber); diff --git a/src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs b/src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs index 10f069f71c..3b0a4eaeba 100644 --- a/src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs +++ b/src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs @@ -20,6 +20,7 @@ namespace Microsoft.AspNetCore.Blazor.Rendering private readonly IComponent _component; private readonly Renderer _renderer; private readonly IReadOnlyList _cascadingParameters; + private readonly bool _hasAnyCascadingParameterSubscriptions; private RenderTreeBuilder _renderTreeBuilderCurrent; private RenderTreeBuilder _renderTreeBuilderPrevious; private ArrayBuilder _latestDirectParametersSnapshot; // Lazily instantiated @@ -48,7 +49,7 @@ namespace Microsoft.AspNetCore.Blazor.Rendering if (_cascadingParameters != null) { - AddCascadingParameterSubscriptions(); + _hasAnyCascadingParameterSubscriptions = AddCascadingParameterSubscriptions(); } } @@ -88,7 +89,7 @@ namespace Microsoft.AspNetCore.Blazor.Rendering RenderTreeDiffBuilder.DisposeFrames(batchBuilder, _renderTreeBuilderCurrent.GetFrames()); - if (_cascadingParameters != null) + if (_hasAnyCascadingParameterSubscriptions) { RemoveCascadingParameterSubscriptions(); } @@ -119,12 +120,7 @@ namespace Microsoft.AspNetCore.Blazor.Rendering // If we bypass this, the component won't receive the cascading parameters nor // will it update its snapshot of direct parameters. - // TODO: Consider adding a "static" mode for tree params in which we don't - // subscribe for updates, and hence don't have to do any of the parameter - // snapshotting. This would be useful for things like FormContext that aren't - // going to change. - - if (_cascadingParameters != null) + if (_hasAnyCascadingParameterSubscriptions) { // We may need to replay these direct parameters later (in NotifyCascadingValueChanged), // but we can't guarantee that the original underlying data won't have mutated in the @@ -133,8 +129,12 @@ namespace Microsoft.AspNetCore.Blazor.Rendering { _latestDirectParametersSnapshot = new ArrayBuilder(); } - parameters.CaptureSnapshot(_latestDirectParametersSnapshot); + parameters.CaptureSnapshot(_latestDirectParametersSnapshot); + } + + if (_cascadingParameters != null) + { parameters = parameters.WithCascadingParameters(_cascadingParameters); } @@ -150,13 +150,22 @@ namespace Microsoft.AspNetCore.Blazor.Rendering Component.SetParameters(allParams); } - private void AddCascadingParameterSubscriptions() + private bool AddCascadingParameterSubscriptions() { + var hasSubscription = false; var numCascadingParameters = _cascadingParameters.Count; + for (var i = 0; i < numCascadingParameters; i++) { - _cascadingParameters[i].ValueSupplier.Subscribe(this); + var valueSupplier = _cascadingParameters[i].ValueSupplier; + if (!valueSupplier.CurrentValueIsFixed) + { + valueSupplier.Subscribe(this); + hasSubscription = true; + } } + + return hasSubscription; } private void RemoveCascadingParameterSubscriptions() diff --git a/test/Microsoft.AspNetCore.Blazor.Test/CascadingParameterTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/CascadingParameterTest.cs index d4b2a6f40f..bf149d7177 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/CascadingParameterTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/CascadingParameterTest.cs @@ -4,6 +4,7 @@ using Microsoft.AspNetCore.Blazor.Components; using Microsoft.AspNetCore.Blazor.RenderTree; using Microsoft.AspNetCore.Blazor.Test.Helpers; +using System; using System.Linq; using Xunit; @@ -236,6 +237,113 @@ namespace Microsoft.AspNetCore.Blazor.Test Assert.Equal(2, nestedComponent.NumSetParametersCalls); } + [Fact] + public void DoesNotNotifyDescendantsOfUpdatedCascadingParameterValuesWhenFixed() + { + // Arrange + var providedValue = "Initial value"; + var shouldIncludeChild = true; + var renderer = new TestRenderer(); + var component = new TestComponent(builder => + { + builder.OpenComponent>(0); + builder.AddAttribute(1, "Value", providedValue); + builder.AddAttribute(2, "Fixed", true); + builder.AddAttribute(3, RenderTreeBuilder.ChildContent, new RenderFragment(childBuilder => + { + if (shouldIncludeChild) + { + childBuilder.OpenComponent>(0); + childBuilder.AddAttribute(1, "RegularParameter", "Goodbye"); + childBuilder.CloseComponent(); + } + })); + builder.CloseComponent(); + }); + + // Act 1: Initial render; capture nested component ID + var componentId = renderer.AssignRootComponentId(component); + component.TriggerRender(); + var firstBatch = renderer.Batches.Single(); + var nestedComponent = FindComponent>(firstBatch, out var nestedComponentId); + Assert.Equal(1, nestedComponent.NumRenders); + + // Assert: Initial value is supplied to descendant + var nestedComponentDiff = firstBatch.DiffsByComponentId[nestedComponentId].Single(); + Assert.Collection(nestedComponentDiff.Edits, edit => + { + Assert.Equal(RenderTreeEditType.PrependFrame, edit.Type); + AssertFrame.Text( + firstBatch.ReferenceFrames[edit.ReferenceFrameIndex], + "CascadingParameter=Initial value; RegularParameter=Goodbye"); + }); + + // Act 2: Re-render CascadingValue with new value + providedValue = "Updated value"; + component.TriggerRender(); + + // Assert: We did not re-render the descendant + Assert.Equal(2, renderer.Batches.Count); + var secondBatch = renderer.Batches[1]; + Assert.Equal(2, secondBatch.DiffsByComponentId.Count); // Root + CascadingValue, but not nested one + Assert.Equal(1, nestedComponent.NumSetParametersCalls); + Assert.Equal(1, nestedComponent.NumRenders); + + // Act 3: Dispose + shouldIncludeChild = false; + component.TriggerRender(); + + // Assert: Absence of an exception here implies we didn't cause a problem by + // trying to remove a non-existent subscription + } + + [Fact] + public void CascadingValueThrowsIfFixedFlagChangesToTrue() + { + // Arrange + var renderer = new TestRenderer(); + var isFixed = false; + var component = new TestComponent(builder => + { + builder.OpenComponent>(0); + builder.AddAttribute(1, "Fixed", isFixed); + builder.AddAttribute(2, "Value", new object()); + builder.CloseComponent(); + }); + renderer.AssignRootComponentId(component); + component.TriggerRender(); + + // Act/Assert + isFixed = true; + var ex = Assert.Throws(() => component.TriggerRender()); + Assert.Equal("The value of Fixed cannot be changed dynamically.", ex.Message); + } + + [Fact] + public void CascadingValueThrowsIfFixedFlagChangesToFalse() + { + // Arrange + var renderer = new TestRenderer(); + var isFixed = true; + var component = new TestComponent(builder => + { + builder.OpenComponent>(0); + if (isFixed) // Showing also that "unset" is treated as "false" + { + builder.AddAttribute(1, "Fixed", true); + } + builder.AddAttribute(2, "Value", new object()); + builder.CloseComponent(); + }); + renderer.AssignRootComponentId(component); + component.TriggerRender(); + + // Act/Assert + isFixed = false; + var ex = Assert.Throws(() => component.TriggerRender()); + Assert.Equal("The value of Fixed cannot be changed dynamically.", ex.Message); + } + private static T FindComponent(CapturedBatch batch, out int componentId) { var componentFrame = batch.ReferenceFrames.Single( diff --git a/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionTest.cs index f2def78acc..2da9dcf3dd 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionTest.cs @@ -327,6 +327,8 @@ namespace Microsoft.AspNetCore.Blazor.Test public object CurrentValue { get; } + public bool CurrentValueIsFixed => false; + public bool CanSupplyValue(Type valueType, string valueName) => throw new NotImplementedException();