Tests and fixes

Author: SteveSandersonMS

src/Components/Components/perf/RenderTreeDiffBuilderBenchmark.cs

@@ -79,7 +79,7 @@ public RenderTreeDiffBuilderBenchmark()
     public void ComputeDiff_SingleFormField()
     {
         builder.ClearStateForCurrentBatch();
-        var diff = RenderTreeDiffBuilder.ComputeDiff(renderer, builder, 0, modified.GetFrames(), original.GetFrames(), original.GetNamedEvents());
+        var diff = RenderTreeDiffBuilder.ComputeDiff(renderer, builder, 0, modified.GetFrames(), original.GetFrames());
         GC.KeepAlive(diff);
     }
 

src/Components/Components/src/PublicAPI.Unshipped.txt

@@ -41,7 +41,6 @@ Microsoft.AspNetCore.Components.RenderTree.NamedEvent.NamedEvent() -> void
 Microsoft.AspNetCore.Components.RenderTree.NamedEvent.NamedEvent(int componentId, int frameIndex, string! eventType, string! assignedName) -> void
 Microsoft.AspNetCore.Components.RenderTree.RenderBatch.AddedNamedEvents.get -> Microsoft.AspNetCore.Components.RenderTree.ArrayRange<Microsoft.AspNetCore.Components.RenderTree.NamedEvent>?
 Microsoft.AspNetCore.Components.RenderTree.RenderBatch.RemovedNamedEvents.get -> Microsoft.AspNetCore.Components.RenderTree.ArrayRange<Microsoft.AspNetCore.Components.RenderTree.NamedEvent>?
-Microsoft.AspNetCore.Components.RenderTree.Renderer.GetCurrentRenderTreeFrames(int componentId, bool optional) -> Microsoft.AspNetCore.Components.RenderTree.ArrayRange<Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame>?
 Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame.ComponentFrameFlags.get -> Microsoft.AspNetCore.Components.RenderTree.ComponentFrameFlags
 Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrameType.ComponentRenderMode = 9 -> Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrameType
 Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrameType.NamedEvent = 10 -> Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrameType

src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs

@@ -5,7 +5,6 @@
 
 using System.Diagnostics;
 using Microsoft.AspNetCore.Components.HotReload;
-using Microsoft.AspNetCore.Components.Reflection;
 using Microsoft.AspNetCore.Components.Rendering;
 
 namespace Microsoft.AspNetCore.Components.RenderTree;

src/Components/Components/src/RenderTree/RenderTreeFrame.cs

@@ -414,6 +414,12 @@ internal static RenderTreeFrame ElementReferenceCapture(int sequence, Action<Ele
     internal static RenderTreeFrame ComponentReferenceCapture(int sequence, Action<object> componentReferenceCaptureAction, int parentFrameIndex)
         => new RenderTreeFrame(sequence, componentReferenceCaptureAction: componentReferenceCaptureAction, parentFrameIndex: parentFrameIndex);
 
+    internal static RenderTreeFrame NamedEvent(int sequence, string eventType, string assignedName)
+        => new RenderTreeFrame { SequenceField = sequence, FrameTypeField = RenderTreeFrameType.NamedEvent, NamedEventTypeField = eventType, NamedEventAssignedNameField = assignedName };
+
+    internal static RenderTreeFrame ComponentRenderModeFrame(int sequence, IComponentRenderMode renderMode)
+        => new RenderTreeFrame { SequenceField = sequence, FrameTypeField = RenderTreeFrameType.ComponentRenderMode, ComponentRenderModeField = renderMode };
+
     internal RenderTreeFrame WithElementSubtreeLength(int elementSubtreeLength)
         => new RenderTreeFrame(SequenceField, elementSubtreeLength: elementSubtreeLength, ElementNameField, ElementKeyField);
 

src/Components/Components/src/Rendering/RenderTreeBuilder.cs

@@ -28,6 +28,7 @@ public sealed class RenderTreeBuilder : IDisposable
     private bool _hasSeenAddMultipleAttributes;
     private Dictionary<string, int>? _seenAttributeNames;
     private IComponentRenderMode? _pendingComponentCallSiteRenderMode; // TODO: Remove when Razor compiler supports call-site @rendermode
+    private (int Sequence, string AssignedName)? _pendingNamedSubmitEvent; // TODO: Remove when Razor compiler supports @onsubmit:name
 
     /// <summary>
     /// The reserved parameter name used for supplying child content.
@@ -63,6 +64,12 @@ public void OpenElement(int sequence, string elementName)
     /// </summary>
     public void CloseElement()
     {
+        if (_pendingNamedSubmitEvent is { } pendingNamedSubmitEvent)
+        {
+            AddNamedEvent(pendingNamedSubmitEvent.Sequence, "onsubmit", pendingNamedSubmitEvent.AssignedName);
+            _pendingNamedSubmitEvent = default;
+        }
+
         var indexOfEntryBeingClosed = _openElementIndices.Pop();
 
         // We might be closing an element with only attributes, run the duplicate cleanup pass
@@ -226,7 +233,7 @@ public void AddAttribute(int sequence, string name, string? value)
             // That should compile directly as a call to AddNamedValue.
             if (string.Equals(name, "@onsubmit:name", StringComparison.Ordinal) && _lastNonAttributeFrameType == RenderTreeFrameType.Element)
             {
-                AddNamedEvent(sequence, "onsubmit", value!);
+                _pendingNamedSubmitEvent = (sequence, value!);
             }
             else
             {
@@ -704,6 +711,9 @@ public void AddComponentRenderMode(int sequence, IComponentRenderMode renderMode
     /// <param name="assignedName">The application-assigned name.</param>
     public void AddNamedEvent(int sequence, string eventType, string assignedName)
     {
+        ArgumentNullException.ThrowIfNull(eventType);
+        ArgumentNullException.ThrowIfNull(assignedName);
+
         // Note that we could trivially extend this to a generic concept of "named values" that exist within the rendertree
         // and are tracked when added, removed, or updated. Currently we don't need that generality, but if we ever do, we
         // can replace RenderTreeFrameType.NamedEvent with RenderTreeFrameType.NamedValue and use it to implement named events.

src/Components/Components/test/RenderTreeDiffBuilderTest.cs

@@ -816,7 +816,7 @@ public void RecognizesComponentTypeChangesAtSameSequenceNumber()
         using var batchBuilder = new RenderBatchBuilder();
 
         // Act
-        var diff = RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, oldTree.GetFrames(), newTree.GetFrames(), newTree.GetNamedEvents());
+        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.Equal(new[] { 0 }, batchBuilder.ComponentDisposalQueue);
@@ -1627,7 +1627,7 @@ public void RetainsChildComponentsForExistingFrames()
 
         using var batchBuilder = new RenderBatchBuilder();
         using var renderTreeBuilder = new RenderTreeBuilder();
-        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTreeBuilder.GetFrames(), oldTree.GetFrames(), oldTree.GetNamedEvents());
+        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTreeBuilder.GetFrames(), oldTree.GetFrames());
         var originalFakeComponentInstance = oldTree.GetFrames().Array[2].Component;
         var originalFakeComponent2Instance = oldTree.GetFrames().Array[3].Component;
 
@@ -1713,7 +1713,7 @@ public void SetsUpdatedParametersOnChildComponents()
 
         using var batchBuilder = new RenderBatchBuilder();
         using var renderTree = new RenderTreeBuilder();
-        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTree.GetFrames(), oldTree.GetFrames(), oldTree.GetNamedEvents());
+        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTree.GetFrames(), oldTree.GetFrames());
         var originalComponentInstance = (FakeComponent)oldTree.GetFrames().Array[0].Component;
 
         // Act
@@ -1763,7 +1763,7 @@ public void SkipsUpdatingParametersOnChildComponentsIfAllAreDefinitelyImmutableA
 
         using var batchBuilder = new RenderBatchBuilder();
         using var renderTreeBuilder = new RenderTreeBuilder();
-        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTreeBuilder.GetFrames(), oldTree.GetFrames(), oldTree.GetNamedEvents());
+        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTreeBuilder.GetFrames(), oldTree.GetFrames());
         var originalComponentInstance = (CaptureSetParametersComponent)oldTree.GetFrames().Array[0].Component;
         Assert.Equal(1, originalComponentInstance.SetParametersCallCount);
 
@@ -1793,7 +1793,7 @@ public void AlwaysRegardsRenderFragmentAsPossiblyChanged()
 
         using var batchBuilder = new RenderBatchBuilder();
         using var renderTreeBuilder = new RenderTreeBuilder();
-        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTreeBuilder.GetFrames(), oldTree.GetFrames(), oldTree.GetNamedEvents());
+        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTreeBuilder.GetFrames(), oldTree.GetFrames());
         var componentInstance = (CaptureSetParametersComponent)oldTree.GetFrames().Array[0].Component;
         Assert.Equal(1, componentInstance.SetParametersCallCount);
 
@@ -1819,13 +1819,13 @@ public void QueuesRemovedChildComponentsForDisposal()
 
         using var batchBuilder = new RenderBatchBuilder();
         using var renderTree = new RenderTreeBuilder();
-        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTree.GetFrames(), oldTree.GetFrames(), oldTree.GetNamedEvents());
+        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, renderTree.GetFrames(), oldTree.GetFrames());
 
         // Act/Assert
         // Note that we track NonDisposableComponent was disposed even though it's not IDisposable,
         // because it's up to the upstream renderer to decide what "disposing" a component means
         Assert.Empty(batchBuilder.ComponentDisposalQueue);
-        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, oldTree.GetFrames(), newTree.GetFrames(), newTree.GetNamedEvents());
+        RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, oldTree.GetFrames(), newTree.GetFrames());
         Assert.Equal(new[] { 0, 1 }, batchBuilder.ComponentDisposalQueue);
     }
 
@@ -2211,24 +2211,136 @@ public void CanChangeFrameTypeWithMatchingKey()
             entry => AssertEdit(entry, RenderTreeEditType.RemoveFrame, 1));
     }
 
+    [Fact]
+    public void RecognizesNamedEventBeingAdded()
+    {
+        oldTree.OpenElement(0, "existing");
+        oldTree.AddAttribute(1, "attr1", "unrelated val1");
+        oldTree.CloseElement();
+
+        newTree.OpenElement(0, "existing");
+        newTree.AddAttribute(1, "attr1", "unrelated val1");
+        newTree.AddNamedEvent(2, "someevent1", "added to existing element");
+        newTree.CloseElement();
+        newTree.OpenElement(3, "new element");
+        newTree.AddNamedEvent(4, "someevent2", "added with new element");
+        newTree.CloseElement();
+
+        // Act
+        var (result, referenceFrames, batch) = GetSingleUpdatedComponentWithBatch(componentId: 123);
+
+        // Assert
+        Assert.Collection(result.Edits,
+            entry =>
+            {
+                AssertEdit(entry, RenderTreeEditType.PrependFrame, 1);
+                Assert.Equal(0, entry.ReferenceFrameIndex);
+                Assert.Equal("new element", referenceFrames[entry.ReferenceFrameIndex].ElementName);
+            });
+        Assert.Collection(batch.AddedNamedEvents.Value.AsEnumerable(),
+            entry => AssertNamedEvent(entry, 123, 2, "someevent1", "added to existing element"),
+            entry => AssertNamedEvent(entry, 123, 4, "someevent2", "added with new element"));
+        Assert.False(batch.RemovedNamedEvents.HasValue);
+    }
+
+    [Fact]
+    public void RecognizesNamedEventBeingRemoved()
+    {
+        oldTree.OpenElement(0, "retaining");
+        oldTree.AddAttribute(1, "attr1", "unrelated val1");
+        oldTree.AddNamedEvent(2, "someevent1", "removing from retained element");
+        oldTree.CloseElement();
+        oldTree.OpenElement(3, "removing");
+        oldTree.AddNamedEvent(4, "someevent2", "removed because element was removed");
+        oldTree.CloseElement();
+
+        newTree.OpenElement(0, "retaining");
+        newTree.AddAttribute(1, "attr1", "unrelated val1");
+        newTree.CloseElement();
+
+        // Act
+        var (result, referenceFrames, batch) = GetSingleUpdatedComponentWithBatch(componentId: 123);
+
+        // Assert
+        Assert.Collection(result.Edits,
+            entry => AssertEdit(entry, RenderTreeEditType.RemoveFrame, 1));
+        Assert.False(batch.AddedNamedEvents.HasValue);
+        Assert.Collection(batch.RemovedNamedEvents.Value.AsEnumerable(),
+            entry => AssertNamedEvent(entry, 123, 2, "someevent1", "removing from retained element"),
+            entry => AssertNamedEvent(entry, 123, 4, "someevent2", "removed because element was removed"));
+    }
+
+    [Fact]
+    public void RecognizesNamedEventBeingMoved()
+    {
+        oldTree.OpenElement(0, "elem");
+        oldTree.AddNamedEvent(2, "eventname", "assigned name");
+        oldTree.CloseElement();
+
+        newTree.OpenElement(0, "elem");
+        newTree.AddAttribute(1, "attr1", "unrelated val1");
+        newTree.AddNamedEvent(2, "eventname", "assigned name");
+        newTree.CloseElement();
+
+        // Act
+        var (result, referenceFrames, batch) = GetSingleUpdatedComponentWithBatch(componentId: 123);
+
+        // Assert
+        Assert.Collection(result.Edits,
+            entry =>
+            {
+                AssertEdit(entry, RenderTreeEditType.SetAttribute, 0);
+                Assert.Equal(0, entry.ReferenceFrameIndex);
+                Assert.Equal("attr1", referenceFrames[entry.ReferenceFrameIndex].AttributeName);
+            });
+        Assert.Collection(batch.RemovedNamedEvents.Value.AsEnumerable(),
+            entry => AssertNamedEvent(entry, 123, 1, "eventname", "assigned name"));
+        Assert.Collection(batch.AddedNamedEvents.Value.AsEnumerable(),
+            entry => AssertNamedEvent(entry, 123, 2, "eventname", "assigned name"));
+    }
+
+    [Fact]
+    public void RecognizesNamedEventChangingAssignedName()
+    {
+        oldTree.OpenElement(0, "elem");
+        oldTree.AddNamedEvent(1, "eventname1", "original name");
+        oldTree.AddNamedEvent(2, "eventname2", "will be left unchanged");
+        oldTree.CloseElement();
+
+        newTree.OpenElement(0, "elem");
+        newTree.AddNamedEvent(1, "eventname1", "changed name");
+        newTree.AddNamedEvent(2, "eventname2", "will be left unchanged");
+        newTree.CloseElement();
+
+        // Act
+        var (result, referenceFrames, batch) = GetSingleUpdatedComponentWithBatch(componentId: 123);
+
+        // Assert
+        Assert.Empty(result.Edits);
+        Assert.Collection(batch.RemovedNamedEvents.Value.AsEnumerable(),
+            entry => AssertNamedEvent(entry, 123, 1, "eventname1", "original name"));
+        Assert.Collection(batch.AddedNamedEvents.Value.AsEnumerable(),
+            entry => AssertNamedEvent(entry, 123, 1, "eventname1", "changed name"));
+    }
+
     private (RenderTreeDiff, RenderTreeFrame[]) GetSingleUpdatedComponent(bool initializeFromFrames = false)
     {
         var result = GetSingleUpdatedComponentWithBatch(initializeFromFrames);
         return (result.Item1, result.Item2);
     }
 
-    private (RenderTreeDiff, RenderTreeFrame[], RenderBatch) GetSingleUpdatedComponentWithBatch(bool initializeFromFrames = false)
+    private (RenderTreeDiff, RenderTreeFrame[], RenderBatch) GetSingleUpdatedComponentWithBatch(bool initializeFromFrames = false, int componentId = 0)
     {
-        var batch = GetRenderedBatch(initializeFromFrames);
+        var batch = GetRenderedBatch(initializeFromFrames, componentId);
         var diffsInBatch = batch.UpdatedComponents;
         Assert.Equal(1, diffsInBatch.Count);
         return (diffsInBatch.Array[0], batch.ReferenceFrames.AsEnumerable().ToArray(), batch);
     }
 
-    private RenderBatch GetRenderedBatch(bool initializeFromFrames = false)
-        => GetRenderedBatch(oldTree, newTree, initializeFromFrames);
+    private RenderBatch GetRenderedBatch(bool initializeFromFrames = false, int componentId = 0)
+        => GetRenderedBatch(oldTree, newTree, initializeFromFrames, componentId);
 
-    private RenderBatch GetRenderedBatch(RenderTreeBuilder from, RenderTreeBuilder to, bool initializeFromFrames)
+    private RenderBatch GetRenderedBatch(RenderTreeBuilder from, RenderTreeBuilder to, bool initializeFromFrames, int componentId = 0)
     {
         if (initializeFromFrames)
         {
@@ -2238,14 +2350,14 @@ private RenderBatch GetRenderedBatch(RenderTreeBuilder from, RenderTreeBuilder t
             var emptyFrames = renderTreeBuilder.GetFrames();
             var oldFrames = from.GetFrames();
 
-            RenderTreeDiffBuilder.ComputeDiff(renderer, initializeBatchBuilder, 0, emptyFrames, oldFrames, from.GetNamedEvents());
+            RenderTreeDiffBuilder.ComputeDiff(renderer, initializeBatchBuilder, 0, emptyFrames, oldFrames);
         }
 
         batchBuilder?.Dispose();
         // This gets disposed as part of the test type's Dispose
         batchBuilder = new RenderBatchBuilder();
 
-        var diff = RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, 0, from.GetFrames(), to.GetFrames(), to.GetNamedEvents());
+        var diff = RenderTreeDiffBuilder.ComputeDiff(renderer, batchBuilder, componentId, from.GetFrames(), to.GetFrames());
         batchBuilder.UpdatedComponentDiffs.Append(diff);
         return batchBuilder.ToBatch();
     }
@@ -2373,4 +2485,17 @@ private static void AssertPermutationListEntry(
         Assert.Equal(fromSiblingIndex, edit.SiblingIndex);
         Assert.Equal(toSiblingIndex, edit.MoveToSiblingIndex);
     }
+
+    private static void AssertNamedEvent(
+        NamedEvent namedEvent,
+        int componentId,
+        int frameIndex,
+        string eventType,
+        string assignedName)
+    {
+        Assert.Equal(componentId, namedEvent.ComponentId);
+        Assert.Equal(frameIndex, namedEvent.FrameIndex);
+        Assert.Equal(eventType, namedEvent.EventType);
+        Assert.Equal(assignedName, namedEvent.AssignedName);
+    }
 }