diff --git a/samples/HostedInAspNet.Client/Program.cs b/samples/HostedInAspNet.Client/Program.cs index c16d768296..69897ac083 100644 --- a/samples/HostedInAspNet.Client/Program.cs +++ b/samples/HostedInAspNet.Client/Program.cs @@ -24,6 +24,10 @@ namespace HostedInAspNet.Client { } + public void SetParameters(ParameterCollection parameters) + { + } + public void BuildRenderTree(RenderTreeBuilder builder) { builder.OpenElement(0, "h1"); diff --git a/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs b/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs index 44a9c200b4..150a74e51e 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs @@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Blazor.Components /// Optional base class for Blazor components. Alternatively, Blazor components may /// implement directly. /// - public abstract class BlazorComponent : IComponent, IHandlePropertiesChanged + public abstract class BlazorComponent : IComponent { private RenderHandle _renderHandle; @@ -28,6 +28,16 @@ namespace Microsoft.AspNetCore.Blazor.Components _renderHandle = renderHandle; } + void IComponent.SetParameters(ParameterCollection parameters) + { + parameters.AssignToProperties(this); + + // TODO: If we know conclusively that the parameters have not changed since last + // time (because they are all primitives and equal to the existing property values) + // then don't re-render. Can put an "out bool" parameter on AssignToProperties. + _renderHandle.Render(); + } + /// public virtual void BuildRenderTree(RenderTreeBuilder builder) { @@ -50,11 +60,6 @@ namespace Microsoft.AspNetCore.Blazor.Components public virtual Task ExecuteAsync() => throw new NotImplementedException($"Blazor components do not implement {nameof(ExecuteAsync)}."); - /// - public virtual void OnPropertiesChanged() - { - } - /// /// Handles click events by invoking . /// diff --git a/src/Microsoft.AspNetCore.Blazor/Components/IComponent.cs b/src/Microsoft.AspNetCore.Blazor/Components/IComponent.cs index 494d2e1ac8..0abc9100a4 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/IComponent.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/IComponent.cs @@ -16,6 +16,12 @@ namespace Microsoft.AspNetCore.Blazor.Components /// A that allows the component to be rendered. void Init(RenderHandle renderHandle); + /// + /// Sets parameters supplied by the component's parent in the render tree. + /// + /// The parameters. + void SetParameters(ParameterCollection parameters); + /// /// Builds a representing the current state of the component. /// diff --git a/src/Microsoft.AspNetCore.Blazor/Components/IHandlePropertiesChanged.cs b/src/Microsoft.AspNetCore.Blazor/Components/IHandlePropertiesChanged.cs deleted file mode 100644 index 299b6c94ac..0000000000 --- a/src/Microsoft.AspNetCore.Blazor/Components/IHandlePropertiesChanged.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNetCore.Blazor.Components -{ - /// - /// Interface implemented by components that receive notifications when their - /// properties are changed by their parent component. - /// - public interface IHandlePropertiesChanged - { - /// - /// Notifies the component that its properties have changed. - /// - void OnPropertiesChanged(); - } -} diff --git a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs new file mode 100644 index 0000000000..4fb9d69a2a --- /dev/null +++ b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs @@ -0,0 +1,71 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Reflection; + +namespace Microsoft.AspNetCore.Blazor.Components +{ + /// + /// Extension methods for the type. + /// + public static class ParameterCollectionExtensions + { + /// + /// Iterates through the , assigning each parameter + /// to a property of the same name on . + /// + /// The . + /// An object that has a public writable property matching each parameter's name and type. + public static void AssignToProperties( + this ParameterCollection parameterCollection, + object target) + { + if (target == null) + { + throw new ArgumentNullException(nameof(target)); + } + + foreach (var parameter in parameterCollection) + { + AssignToProperty(target, parameter); + } + } + + private static void AssignToProperty(object target, Parameter parameter) + { + // TODO: Don't just use naive reflection like this. Possible ways to make it faster: + // (a) Create and cache a property-assigning open delegate for each (target type, + // property name) pair, e.g., using propertyInfo.GetSetMethod().CreateDelegate(...) + // That's much faster than caching the PropertyInfo, at least on JIT-enabled platforms. + // (b) Or possibly just code-gen an IComponent.SetParameters implementation for each + // Razor component. However that might not work well with code-behind inheritance, + // because the code-behind wouldn't be able to override it. + + var propertyInfo = GetPropertyInfo(target.GetType(), parameter.Name); + try + { + propertyInfo.SetValue(target, parameter.Value); + } + catch (Exception ex) + { + throw new InvalidOperationException( + $"Unable to set property '{parameter.Name}' on object of " + + $"type '{target.GetType().FullName}'. The error was: {ex.Message}", ex); + } + } + + private static PropertyInfo GetPropertyInfo(Type targetType, string propertyName) + { + var property = targetType.GetProperty(propertyName); + if (property == null) + { + throw new InvalidOperationException( + $"Object of type '{targetType.FullName}' does not have a property " + + $"matching the name '{propertyName}'."); + } + + return property; + } + } +} diff --git a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffBuilder.cs b/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffBuilder.cs index 1ca2b1ff5f..72685e8523 100644 --- a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffBuilder.cs +++ b/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffBuilder.cs @@ -144,133 +144,23 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree int oldComponentIndex, int newComponentIndex) { - // The algorithm here is the same as in AppendDiffEntriesForRange, except that - // here we don't optimise for loops - we assume that both sequences are forward-only. - // That's because this is true for all currently supported scenarios, and it means - // fewer steps here. - var oldTree = diffContext.OldTree; var newTree = diffContext.NewTree; ref var oldComponentFrame = ref oldTree[oldComponentIndex]; ref var newComponentFrame = ref newTree[newComponentIndex]; var componentId = oldComponentFrame.ComponentId; var componentInstance = oldComponentFrame.Component; - var hasSetAnyProperty = false; // Preserve the actual componentInstance newComponentFrame = newComponentFrame.WithComponentInstance(componentId, componentInstance); - // Now locate any added/changed/removed properties - var oldStartIndex = oldComponentIndex + 1; - var newStartIndex = newComponentIndex + 1; - var oldEndIndexExcl = oldComponentIndex + oldComponentFrame.ComponentSubtreeLength; - var newEndIndexExcl = newComponentIndex + newComponentFrame.ComponentSubtreeLength; - var hasMoreOld = oldEndIndexExcl > oldStartIndex; - var hasMoreNew = newEndIndexExcl > newStartIndex; - while (hasMoreOld || hasMoreNew) - { - var oldSeq = hasMoreOld ? oldTree[oldStartIndex].Sequence : int.MaxValue; - var newSeq = hasMoreNew ? newTree[newStartIndex].Sequence : int.MaxValue; - - if (oldSeq == newSeq) - { - ref var oldFrame = ref oldTree[oldStartIndex]; - ref var newFrame = ref newTree[newStartIndex]; - var oldName = oldFrame.AttributeName; - var newName = newFrame.AttributeName; - var newPropertyValue = newFrame.AttributeValue; - if (string.Equals(oldName, newName, StringComparison.Ordinal)) - { - // Using Equals to account for string comparisons, nulls, etc. - var oldPropertyValue = oldFrame.AttributeValue; - if (!Equals(oldPropertyValue, newPropertyValue)) - { - SetChildComponentProperty(componentInstance, newName, newPropertyValue); - hasSetAnyProperty = true; - } - } - else - { - // Since this code path is never reachable for Razor components (because you - // can't have two different attribute names from the same source sequence), we - // could consider removing the 'name equality' check entirely for perf - SetChildComponentProperty(componentInstance, newName, newPropertyValue); - RemoveChildComponentProperty(componentInstance, oldName); - hasSetAnyProperty = true; - } - - oldStartIndex++; - newStartIndex++; - hasMoreOld = oldEndIndexExcl > oldStartIndex; - hasMoreNew = newEndIndexExcl > newStartIndex; - } - else - { - // Both sequences are proceeding through the same loop block, so do a simple - // preordered merge join (picking from whichever side brings us closer to being - // back in sync) - var treatAsInsert = newSeq < oldSeq; - - if (treatAsInsert) - { - ref var newFrame = ref newTree[newStartIndex]; - SetChildComponentProperty(componentInstance, newFrame.AttributeName, newFrame.AttributeValue); - hasSetAnyProperty = true; - newStartIndex++; - hasMoreNew = newEndIndexExcl > newStartIndex; - } - else - { - ref var oldFrame = ref oldTree[oldStartIndex]; - RemoveChildComponentProperty(componentInstance, oldFrame.AttributeName); - hasSetAnyProperty = true; - oldStartIndex++; - hasMoreOld = oldEndIndexExcl > oldStartIndex; - } - } - } - - if (hasSetAnyProperty) - { - diffContext.BatchBuilder.ComponentRenderQueue.Enqueue(newComponentFrame.ComponentId); - } - } - - private static void RemoveChildComponentProperty(IComponent component, string componentPropertyName) - { - var propertyInfo = GetChildComponentPropertyInfo(component.GetType(), componentPropertyName); - var defaultValue = propertyInfo.PropertyType.IsValueType - ? Activator.CreateInstance(propertyInfo.PropertyType) - : null; - SetChildComponentProperty(component, componentPropertyName, defaultValue); - } - - private static void SetChildComponentProperty(IComponent component, string componentPropertyName, object newPropertyValue) - { - var propertyInfo = GetChildComponentPropertyInfo(component.GetType(), componentPropertyName); - try - { - propertyInfo.SetValue(component, newPropertyValue); - } - catch (Exception ex) - { - throw new InvalidOperationException( - $"Unable to set property '{componentPropertyName}' on component of " + - $"type '{component.GetType().FullName}'. The error was: {ex.Message}", ex); - } - } - - private static PropertyInfo GetChildComponentPropertyInfo(Type componentType, string componentPropertyName) - { - var property = componentType.GetProperty(componentPropertyName); - if (property == null) - { - throw new InvalidOperationException( - $"Component of type '{componentType.FullName}' does not have a property " + - $"matching the name '{componentPropertyName}'."); - } - - return property; + // Supply latest parameters. They might not have changed, but it's up to the + // recipient to decide what "changed" means. Currently we only supply the new + // set of parameters and assume the recipient has enough info to do whatever + // comparisons it wants with the old values. Later we could choose to pass the + // old parameter values if we wanted. + var newParameters = new ParameterCollection(newTree, newComponentIndex); + componentInstance.SetParameters(newParameters); } private static int NextSiblingIndex(RenderTreeFrame frame, int frameIndex) @@ -541,18 +431,9 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree diffContext.Renderer.InstantiateChildComponentOnFrame(ref frame); var childComponentInstance = frame.Component; - // All descendants of a component are its properties - var componentDescendantsEndIndexExcl = frameIndex + frame.ComponentSubtreeLength; - for (var attributeFrameIndex = frameIndex + 1; attributeFrameIndex < componentDescendantsEndIndexExcl; attributeFrameIndex++) - { - ref var attributeFrame = ref frames[attributeFrameIndex]; - SetChildComponentProperty( - childComponentInstance, - attributeFrame.AttributeName, - attributeFrame.AttributeValue); - } - - diffContext.BatchBuilder.ComponentRenderQueue.Enqueue(frame.ComponentId); + // Set initial parameters + var initialParameters = new ParameterCollection(frames, frameIndex); + childComponentInstance.SetParameters(initialParameters); } private static void InitializeNewAttributeFrame(ref DiffContext diffContext, ref RenderTreeFrame newFrame) diff --git a/src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs b/src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs index d6922ecca6..f0e7641125 100644 --- a/src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs +++ b/src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs @@ -37,11 +37,6 @@ namespace Microsoft.AspNetCore.Blazor.Rendering public void RenderIntoBatch(RenderBatchBuilder batchBuilder) { - if (_component is IHandlePropertiesChanged notifyableComponent) - { - notifyableComponent.OnPropertiesChanged(); - } - // Swap the old and new tree builders (_renderTreeBuilderCurrent, _renderTreeBuilderPrevious) = (_renderTreeBuilderPrevious, _renderTreeBuilderCurrent); diff --git a/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs b/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs index b0f8022e3e..b279f2bf86 100644 --- a/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs @@ -482,6 +482,10 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test { } + public void SetParameters(ParameterCollection parameters) + { + } + public void BuildRenderTree(RenderTreeBuilder builder) { builder.AddText(0, $"Hello from {nameof(TestComponent)}"); diff --git a/test/Microsoft.AspNetCore.Blazor.Test/AutoRenderComponent.cs b/test/Microsoft.AspNetCore.Blazor.Test/AutoRenderComponent.cs new file mode 100644 index 0000000000..148d8c35d7 --- /dev/null +++ b/test/Microsoft.AspNetCore.Blazor.Test/AutoRenderComponent.cs @@ -0,0 +1,26 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Blazor.Components; +using Microsoft.AspNetCore.Blazor.RenderTree; + +namespace Microsoft.AspNetCore.Blazor.Test +{ + public abstract class AutoRenderComponent : IComponent + { + private RenderHandle _renderHandle; + + public abstract void BuildRenderTree(RenderTreeBuilder builder); + + public void Init(RenderHandle renderHandle) + { + _renderHandle = renderHandle; + } + + public void SetParameters(ParameterCollection parameters) + { + parameters.AssignToProperties(this); + _renderHandle.Render(); + } + } +} diff --git a/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionTest.cs index 0bd69627bc..4d058e4053 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionTest.cs @@ -107,6 +107,9 @@ namespace Microsoft.AspNetCore.Blazor.Test public void Init(RenderHandle renderHandle) => throw new NotImplementedException(); + public void SetParameters(ParameterCollection parameters) + => throw new NotImplementedException(); + public void BuildRenderTree(RenderTreeBuilder builder) => throw new NotImplementedException(); } diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs index a24d6e90e4..28db0e82a6 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs @@ -293,6 +293,9 @@ namespace Microsoft.AspNetCore.Blazor.Test { public void Init(RenderHandle renderHandle) { } + public void SetParameters(ParameterCollection parameters) + => throw new NotImplementedException(); + public void BuildRenderTree(RenderTreeBuilder builder) => throw new NotImplementedException(); } diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs index 5faf3c4ae0..d171c441ac 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs @@ -34,7 +34,7 @@ namespace Microsoft.AspNetCore.Blazor.Test appendAction(newTree); // Act - var (result, referenceFrames) = GetSingleUpdatedComponent(); + var (result, referenceFrames) = GetSingleUpdatedComponent(initializeFromFrames: true); // Assert Assert.Empty(result.Edits); @@ -51,7 +51,11 @@ namespace Microsoft.AspNetCore.Blazor.Test builder.AddAttribute(1, "My attribute", "My value"); builder.CloseElement(); }, - builder => builder.OpenComponent(0) + builder => + { + builder.OpenComponent(0); + builder.CloseComponent(); + } }.Select(x => new object[] { x }); [Fact] @@ -319,7 +323,7 @@ namespace Microsoft.AspNetCore.Blazor.Test // Arrange oldTree.OpenComponent(123); oldTree.CloseComponent(); - GetRenderedBatch(new RenderTreeBuilder(renderer), oldTree); // Assign initial IDs + GetRenderedBatch(new RenderTreeBuilder(renderer), oldTree, false); // Assign initial IDs newTree.OpenComponent(123); newTree.CloseComponent(); var batchBuilder = new RenderBatchBuilder(); @@ -327,11 +331,8 @@ namespace Microsoft.AspNetCore.Blazor.Test // Act var diff = RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, oldTree.GetFrames(), newTree.GetFrames()); - // Assert: We're going to dispose the old component, and render the new one + // Assert: We're going to dispose the old component and render the new one Assert.Equal(new[] { 0 }, batchBuilder.ComponentDisposalQueue); - Assert.Equal(new[] { 1 }, batchBuilder.ComponentRenderQueue); - - // Assert: Got correct info in diff Assert.Collection(diff.Edits, entry => AssertEdit(entry, RenderTreeEditType.RemoveFrame, 0), entry => @@ -616,7 +617,7 @@ namespace Microsoft.AspNetCore.Blazor.Test } [Fact] - public void SetsKnownPropertiesOnChildComponents() + public void SetsParametersOnChildComponents() { // Arrange var testObject = new object(); @@ -640,41 +641,6 @@ namespace Microsoft.AspNetCore.Blazor.Test Assert.Same(testObject, componentInstance.ObjectProperty); } - [Fact] - public void ThrowsIfAssigningUnknownPropertiesToChildComponents() - { - // Arrange - var testObject = new object(); - newTree.OpenComponent(0); - newTree.AddAttribute(1, "SomeUnknownProperty", 123); - newTree.CloseComponent(); - - // Act/Assert - var ex = Assert.Throws(() => - { - RenderTreeDiffBuilder.ComputeDiff(renderer, new RenderBatchBuilder(), 0, oldTree.GetFrames(), newTree.GetFrames()); - }); - Assert.Equal($"Component of type '{typeof(FakeComponent).FullName}' does not have a property matching the name 'SomeUnknownProperty'.", ex.Message); - } - - [Fact] - public void ThrowsIfAssigningReadOnlyPropertiesToChildComponents() - { - // Arrange - var testObject = new object(); - newTree.OpenComponent(0); - newTree.AddAttribute(1, nameof(FakeComponent.ReadonlyProperty), 123); - newTree.CloseComponent(); - - // Act/Assert - var ex = Assert.Throws(() => - { - RenderTreeDiffBuilder.ComputeDiff(renderer, new RenderBatchBuilder(), 0, oldTree.GetFrames(), newTree.GetFrames()); - }); - Assert.StartsWith($"Unable to set property '{nameof(FakeComponent.ReadonlyProperty)}' on " + - $"component of type '{typeof(FakeComponent).FullName}'.", ex.Message); - } - [Fact] public void RetainsChildComponentsForExistingFrames() { @@ -712,7 +678,7 @@ namespace Microsoft.AspNetCore.Blazor.Test } [Fact] - public void UpdatesChangedPropertiesOnRetainedChildComponents() + public void SetsUpdatedParametersOnChildComponents() { // Arrange var objectWillNotChange = new object(); @@ -727,7 +693,6 @@ namespace Microsoft.AspNetCore.Blazor.Test RenderTreeDiffBuilder.ComputeDiff(renderer, new RenderBatchBuilder(), 0, new RenderTreeBuilder(renderer).GetFrames(), oldTree.GetFrames()); var originalComponentInstance = (FakeComponent)oldTree.GetFrames().Array[0].Component; - originalComponentInstance.ObjectProperty = null; // So we can see it doesn't get reassigned // Act var renderBatch = GetRenderedBatch(); @@ -737,7 +702,7 @@ namespace Microsoft.AspNetCore.Blazor.Test Assert.Equal(1, renderBatch.UpdatedComponents.Count); // Because the diff builder only queues child component renders; it doesn't actually perfom them itself Assert.Same(originalComponentInstance, newComponentInstance); Assert.Equal("String did change", newComponentInstance.StringProperty); - Assert.Null(newComponentInstance.ObjectProperty); // To observe that the property wasn't even written, we nulled it out on the original + Assert.Same(objectWillNotChange, newComponentInstance.ObjectProperty); } [Fact] @@ -764,19 +729,26 @@ namespace Microsoft.AspNetCore.Blazor.Test Assert.Equal(new[] { 0, 1 }, batchBuilder.ComponentDisposalQueue); } - private (RenderTreeDiff, RenderTreeFrame[]) GetSingleUpdatedComponent() + private (RenderTreeDiff, RenderTreeFrame[]) GetSingleUpdatedComponent(bool initializeFromFrames = false) { - var batch = GetRenderedBatch(); + var batch = GetRenderedBatch(initializeFromFrames); var diffsInBatch = batch.UpdatedComponents; Assert.Equal(1, diffsInBatch.Count); return (diffsInBatch.Array[0], batch.ReferenceFrames.ToArray()); } - private RenderBatch GetRenderedBatch() - => GetRenderedBatch(oldTree, newTree); + private RenderBatch GetRenderedBatch(bool initializeFromFrames = false) + => GetRenderedBatch(oldTree, newTree, initializeFromFrames); - private RenderBatch GetRenderedBatch(RenderTreeBuilder from, RenderTreeBuilder to) + private RenderBatch GetRenderedBatch(RenderTreeBuilder from, RenderTreeBuilder to, bool initializeFromFrames) { + if (initializeFromFrames) + { + var emptyFrames = new RenderTreeBuilder(renderer).GetFrames(); + var oldFrames = from.GetFrames(); + RenderTreeDiffBuilder.ComputeDiff(renderer, new RenderBatchBuilder(), 0, emptyFrames, oldFrames); + } + var batchBuilder = new RenderBatchBuilder(); var diff = RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, from.GetFrames(), to.GetFrames()); batchBuilder.UpdatedComponentDiffs.Append(diff); @@ -799,12 +771,22 @@ namespace Microsoft.AspNetCore.Blazor.Test private string PrivateProperty { get; set; } public void Init(RenderHandle renderHandle) { } + public void SetParameters(ParameterCollection parameters) + { + parameters.AssignToProperties(this); + } public void BuildRenderTree(RenderTreeBuilder builder) { } } private class FakeComponent2 : IComponent { - public void Init(RenderHandle renderHandle) { } + public void Init(RenderHandle renderHandle) + { + } + + public void SetParameters(ParameterCollection parameters) + { + } public void BuildRenderTree(RenderTreeBuilder builder) { @@ -819,6 +801,8 @@ namespace Microsoft.AspNetCore.Blazor.Test public void Init(RenderHandle renderHandle) { } + public void SetParameters(ParameterCollection parameters) { } + public void BuildRenderTree(RenderTreeBuilder builder) { } } @@ -826,10 +810,11 @@ namespace Microsoft.AspNetCore.Blazor.Test { public void Init(RenderHandle renderHandle) { } + public void SetParameters(ParameterCollection parameters) { } + public void BuildRenderTree(RenderTreeBuilder builder) { } } - private static void AssertEdit( RenderTreeEdit edit, RenderTreeEditType type, diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs index 7f8300bd49..4ae670a37d 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs @@ -12,7 +12,7 @@ using Xunit; namespace Microsoft.AspNetCore.Blazor.Test { - public class RendererTest + public partial class RendererTest { [Fact] public void CanRenderTopLevelComponents() @@ -310,7 +310,7 @@ namespace Microsoft.AspNetCore.Blazor.Test Assert.Equal(0, edit.ReferenceFrameIndex); }); AssertFrame.Text(batch.ReferenceFrames[0], "Modified message"); - Assert.False(batch.DiffsByComponentId.ContainsKey(nestedComponentFrame.ComponentId)); + Assert.Empty(batch.DiffsByComponentId[nestedComponentFrame.ComponentId].Single().Edits); } [Fact] @@ -388,66 +388,6 @@ namespace Microsoft.AspNetCore.Blazor.Test AssertFrame.Text(renderer.Batches[1].ReferenceFrames[0], "second"); } - [Fact] - public void NotifiesIHandlePropertiesChangedBeforeFirstRender() - { - // Arrange - var renderer = new TestRenderer(); - var component = new HandlePropertiesChangedComponent(); - var rootComponentId = renderer.AssignComponentId(component); - - // Act - renderer.RenderNewBatch(rootComponentId); - - // Assert - AssertFrame.Text(renderer.Batches.Single().ReferenceFrames[0], "Notifications: 1", 0); - } - - [Fact] - public void NotifiesIHandlePropertiesChangedWhenChanged() - { - // Arrange - var renderer = new TestRenderer(); - var component = new ConditionalParentComponent - { - IncludeChild = true, - ChildParameters = new Dictionary - { - { nameof(HandlePropertiesChangedComponent.IntProperty), 123 } - } - }; - var rootComponentId = renderer.AssignComponentId(component); - - // Act/Assert 0: Initial render - renderer.RenderNewBatch(rootComponentId); - var batch1 = renderer.Batches.Single(); - var childComponentFrame = batch1 - .ReferenceFrames.Where(frame => frame.FrameType == RenderTreeFrameType.Component).Single(); - var childComponentId = childComponentFrame.ComponentId; - var diffForChildComponent0 = batch1.DiffsByComponentId[childComponentId].Single(); - var childComponentInstance = (HandlePropertiesChangedComponent)childComponentFrame.Component; - Assert.Equal(1, childComponentInstance.NotificationsCount); - Assert.Collection(diffForChildComponent0.Edits, - edit => - { - Assert.Equal(RenderTreeEditType.PrependFrame, edit.Type); - AssertFrame.Text(batch1.ReferenceFrames[edit.ReferenceFrameIndex], "Notifications: 1", 0); - }); - - // Act/Assert 1: If properties didn't change, we don't notify - renderer.RenderNewBatch(rootComponentId); - Assert.Equal(1, childComponentInstance.NotificationsCount); - - // Act/Assert 2: If properties did change, we do notify - component.ChildParameters[nameof(HandlePropertiesChangedComponent.IntProperty)] = 456; - renderer.RenderNewBatch(rootComponentId); - Assert.Equal(2, childComponentInstance.NotificationsCount); - var batch3 = renderer.Batches.Skip(2).Single(); - var diffForChildComponent2 = batch3.DiffsByComponentId[childComponentId].Single(); - Assert.Equal(2, childComponentInstance.NotificationsCount); - AssertFrame.Text(batch3.ReferenceFrames[0], "Notifications: 2", 0); - } - [Fact] public void RenderBatchIncludesListOfDisposedComponents() { @@ -708,7 +648,7 @@ namespace Microsoft.AspNetCore.Blazor.Test // Assert var batch = renderer.Batches.Single(); - Assert.Equal(3, batch.DiffsInOrder.Count); + Assert.Equal(4, batch.DiffsInOrder.Count); // First is the parent component's initial render var diff1 = batch.DiffsInOrder[0]; @@ -743,6 +683,11 @@ namespace Microsoft.AspNetCore.Blazor.Test Assert.Equal(RenderTreeEditType.UpdateText, diff3edit.Type); AssertFrame.Text(batch.ReferenceFrames[diff3edit.ReferenceFrameIndex], "Parent render count: 2"); + + // Fourth is child's rerender due to parent rendering + var diff4 = batch.DiffsInOrder[3]; + Assert.NotEqual(parentComponentId, diff4.ComponentId); + Assert.Empty(diff4.Edits); } private class NoOpRenderer : Renderer @@ -750,9 +695,6 @@ namespace Microsoft.AspNetCore.Blazor.Test public new int AssignComponentId(IComponent component) => base.AssignComponentId(component); - public new void RenderNewBatch(int componentId) - => base.RenderNewBatch(componentId); - protected override void UpdateDisplay(RenderBatch renderBatch) { } @@ -832,6 +774,9 @@ namespace Microsoft.AspNetCore.Blazor.Test _renderHandle = renderHandle; } + public void SetParameters(ParameterCollection parameters) + => _renderHandle.Render(); + public void BuildRenderTree(RenderTreeBuilder builder) => _renderAction(builder); @@ -839,15 +784,11 @@ namespace Microsoft.AspNetCore.Blazor.Test => _renderHandle.Render(); } - private class MessageComponent : IComponent + private class MessageComponent : AutoRenderComponent { public string Message { get; set; } - public void Init(RenderHandle renderHandle) - { - } - - public void BuildRenderTree(RenderTreeBuilder builder) + public override void BuildRenderTree(RenderTreeBuilder builder) { builder.AddText(0, Message); } @@ -859,25 +800,24 @@ namespace Microsoft.AspNetCore.Blazor.Test public string StringProperty { get; set; } public object ObjectProperty { get; set; } + public void BuildRenderTree(RenderTreeBuilder builder) + { + } + public void Init(RenderHandle renderHandle) { } - public void BuildRenderTree(RenderTreeBuilder builder) - { - } + public void SetParameters(ParameterCollection parameters) + => parameters.AssignToProperties(this); } - private class EventComponent : IComponent + private class EventComponent : AutoRenderComponent, IComponent { public UIEventHandler Handler { get; set; } public bool SkipElement { get; set; } - public void Init(RenderHandle renderHandle) - { - } - - public void BuildRenderTree(RenderTreeBuilder builder) + public override void BuildRenderTree(RenderTreeBuilder builder) { builder.OpenElement(0, "grandparent"); if (!SkipElement) @@ -895,16 +835,12 @@ namespace Microsoft.AspNetCore.Blazor.Test } } - private class ConditionalParentComponent : IComponent where T : IComponent + private class ConditionalParentComponent : AutoRenderComponent where T : IComponent { public bool IncludeChild { get; set; } public IDictionary ChildParameters { get; set; } - public void Init(RenderHandle renderHandle) - { - } - - public void BuildRenderTree(RenderTreeBuilder builder) + public override void BuildRenderTree(RenderTreeBuilder builder) { builder.AddText(0, "Parent here"); @@ -923,36 +859,13 @@ namespace Microsoft.AspNetCore.Blazor.Test } } } - - private class HandlePropertiesChangedComponent : IComponent, IHandlePropertiesChanged - { - public int NotificationsCount { get; private set; } - - public int IntProperty { get; set; } - - public void Init(RenderHandle renderHandle) - { - } - - public void BuildRenderTree(RenderTreeBuilder builder) - { - builder.AddText(0, $"Notifications: {NotificationsCount}"); - } - - public void OnPropertiesChanged() - { - NotificationsCount++; - } - } - private class ReRendersParentComponent : IComponent + private class ReRendersParentComponent : AutoRenderComponent { public TestComponent Parent { get; set; } private bool _isFirstTime = true; - public void Init(RenderHandle renderHandle) { } - - public void BuildRenderTree(RenderTreeBuilder builder) + public override void BuildRenderTree(RenderTreeBuilder builder) { if (_isFirstTime) // Don't want an infinite loop { diff --git a/test/testapps/BasicTestApp/PropertiesChangedHandlerChild.cshtml b/test/testapps/BasicTestApp/PropertiesChangedHandlerChild.cshtml index af7cf9f5b5..7a41c90368 100644 --- a/test/testapps/BasicTestApp/PropertiesChangedHandlerChild.cshtml +++ b/test/testapps/BasicTestApp/PropertiesChangedHandlerChild.cshtml @@ -3,10 +3,9 @@ @functions { public int SuppliedValue { get; set; } - private int ComputedValue { get; set; } - public override void OnPropertiesChanged() - { - ComputedValue = SuppliedValue * 2; - } + // TODO: Instead of computing this in the getter, override OnParametersChanged or whatever + // it ends up being called when that's implemented on BlazorComponent + private int ComputedValue + => SuppliedValue * 2; }