Skip to content

Commit

Permalink
Add AddMultipleAttributes improve RTB
Browse files Browse the repository at this point in the history
- Adds AddMultipleAttributes
- Fix RTB to de-dupe attributes
- Fix RTB behaviour with boxed EventCallback (#8336)
- Add lots of tests for new RTB behaviour and EventCallback
  • Loading branch information
Ryan Nowak committed May 19, 2019
1 parent 956b0d3 commit 7dd8caf
Show file tree
Hide file tree
Showing 5 changed files with 1,292 additions and 100 deletions.
22 changes: 19 additions & 3 deletions src/Components/Components/src/EventCallback.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Components
/// <summary>
/// A bound event handler delegate.
/// </summary>
public readonly struct EventCallback
public readonly struct EventCallback : IEventCallback
{
/// <summary>
/// Gets a reference to the <see cref="EventCallbackFactory"/>.
Expand Down Expand Up @@ -60,12 +60,17 @@ public Task InvokeAsync(object arg)

return Receiver.HandleEventAsync(new EventCallbackWorkItem(Delegate), arg);
}

object IEventCallback.UnpackForRenderTree()
{
return RequiresExplicitReceiver ? (object)this : Delegate;
}
}

/// <summary>
/// A bound event handler delegate.
/// </summary>
public readonly struct EventCallback<T>
public readonly struct EventCallback<T> : IEventCallback
{
internal readonly MulticastDelegate Delegate;
internal readonly IHandleEvent Receiver;
Expand All @@ -86,7 +91,7 @@ public EventCallback(IHandleEvent receiver, MulticastDelegate @delegate)
/// </summary>
public bool HasDelegate => Delegate != null;

// This is a hint to the runtime that Reciever is a different object than what
// This is a hint to the runtime that Receiver is a different object than what
// Delegate.Target points to. This allows us to avoid boxing the command object
// when building the render tree. See logic where this is used.
internal bool RequiresExplicitReceiver => Receiver != null && !object.ReferenceEquals(Receiver, Delegate?.Target);
Expand All @@ -111,5 +116,16 @@ internal EventCallback AsUntyped()
{
return new EventCallback(Receiver ?? Delegate?.Target as IHandleEvent, Delegate);
}

object IEventCallback.UnpackForRenderTree()
{
return RequiresExplicitReceiver ? (object)AsUntyped() : Delegate;
}
}

// Used to understand boxed generic EventCallbacks
internal interface IEventCallback
{
object UnpackForRenderTree();
}
}
13 changes: 12 additions & 1 deletion src/Components/Components/src/RenderTree/ArrayBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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;
Expand Down Expand Up @@ -97,6 +97,17 @@ internal int Append(T[] source, int startIndex, int length)
public void Overwrite(int index, in T value)
=> _items[index] = value;

/// <summary>
/// Removes the item at the specified index.
/// </summary>
/// <param name="index">The index of the item to remove.</param>
public void RemoveAt(int index)
{
Array.Copy(_items, index + 1, _items, index, _itemsInUse - 1 - index);
Array.Clear(_items, _itemsInUse - 1, 1); // Clear last item
_itemsInUse--;
}

/// <summary>
/// Removes the last item.
/// </summary>
Expand Down
91 changes: 87 additions & 4 deletions src/Components/Components/src/RenderTree/RenderTreeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Components;
using Microsoft.AspNetCore.Components.Rendering;
Expand Down Expand Up @@ -157,6 +158,10 @@ public void AddAttribute(int sequence, string name, bool value)
// or absence of an attribute, and false => "False" which isn't falsy in js.
Append(RenderTreeFrame.Attribute(sequence, name, BoxedTrue));
}
else
{
ClearAttributesWithName(name);
}
}

/// <summary>
Expand All @@ -178,6 +183,10 @@ public void AddAttribute(int sequence, string name, string value)
{
Append(RenderTreeFrame.Attribute(sequence, name, value));
}
else
{
ClearAttributesWithName(name);
}
}

/// <summary>
Expand Down Expand Up @@ -275,6 +284,10 @@ public void AddAttribute(int sequence, string name, MulticastDelegate value)
{
Append(RenderTreeFrame.Attribute(sequence, name, value));
}
else
{
ClearAttributesWithName(name);
}
}

/// <summary>
Expand Down Expand Up @@ -372,16 +385,24 @@ public void AddAttribute(int sequence, string name, object value)
{
if (value == null)
{
// Do nothing, treat 'null' attribute values for elements as a conditional attribute.
// Treat 'null' attribute values for elements as a conditional attribute.
ClearAttributesWithName(name);
}
else if (value is bool boolValue)
{
if (boolValue)
{
Append(RenderTreeFrame.Attribute(sequence, name, BoxedTrue));
}

// Don't add anything for false bool value.
else
{
// Don't add anything for false bool value.
ClearAttributesWithName(name);
}
}
else if (value is IEventCallback callbackValue)
{
Append(RenderTreeFrame.Attribute(sequence, name, callbackValue.UnpackForRenderTree()));
}
else if (value is MulticastDelegate)
{
Expand All @@ -395,6 +416,7 @@ public void AddAttribute(int sequence, string name, object value)
}
else if (_lastNonAttributeFrameType == RenderTreeFrameType.Component)
{
// If this is a component, we always want to preserve the original type.
Append(RenderTreeFrame.Attribute(sequence, name, value));
}
else
Expand Down Expand Up @@ -425,6 +447,38 @@ public void AddAttribute(int sequence, in RenderTreeFrame frame)
Append(frame.WithAttributeSequence(sequence));
}

/// <summary>
/// Adds frames representing multiple attributes with the same sequence number.
/// </summary>
/// <typeparam name="T">The attribute value type.</typeparam>
/// <param name="sequence">An integer that represents the position of the instruction in the source code.</param>
/// <param name="attributes">A collection of key-value pairs representing attributes.</param>
public void AddMultipleAttributes<T>(int sequence, IEnumerable<KeyValuePair<string, T>> attributes)
{
// NOTE: The IEnumerable<KeyValuePair<string, T>> is the simplest way to support a variety of
// different types like IReadOnlyDictionary<>, Dictionary<>, and IDictionary<>.
//
// None of those types are contravariant, and since we want to support attributes having a value
// of type object, the simplest thing to do is drop down to IEnumerable<KeyValuePair<>> which
// is contravariant. This also gives us things like List<KeyValuePair<>> and KeyValuePair<>[]
// for free even though we don't expect those types to be common.

// Calling this up-front just to make sure we validate before mutating anything.
AssertCanAddAttribute();

if (attributes != null)
{
foreach (var attribute in attributes)
{
// This will call the AddAttribute(int, string, object) overload.
//
// This is fine because we try to make the object overload behave identically
// to the others.
AddAttribute(sequence, attribute.Key, attribute.Value);
}
}
}

/// <summary>
/// Appends a frame representing a child component.
/// </summary>
Expand Down Expand Up @@ -590,13 +644,42 @@ public ArrayRange<RenderTreeFrame> GetFrames() =>

private void Append(in RenderTreeFrame frame)
{
var frameType = frame.FrameType;
if (frameType == RenderTreeFrameType.Attribute)
{
ClearAttributesWithName(frame.AttributeName);
}

_entries.Append(frame);

var frameType = frame.FrameType;
if (frameType != RenderTreeFrameType.Attribute)
{
_lastNonAttributeFrameType = frame.FrameType;
}
}

private void ClearAttributesWithName(string name)
{
// When an AddAttribute or AddMultipleAttributes method is called, we need to clear
// any prior attributes that have the same attribute name for the current element
// or component.
//
// This is how we enforce the *last attribute wins* semantic.
Debug.Assert(_openElementIndices.Count > 0);

// Start at the last open element/component and iterate forward until
// we find a duplicate.
//
// Since we prevent duplicates, we can always stop after finding one.
for (var i = _openElementIndices.Peek(); i < _entries.Count; i++)
{
if (_entries.Buffer[i].FrameType == RenderTreeFrameType.Attribute &&
string.Equals(name, _entries.Buffer[i].AttributeName, StringComparison.OrdinalIgnoreCase))
{
_entries.RemoveAt(i);
break;
}
}
}
}
}
Loading

0 comments on commit 7dd8caf

Please sign in to comment.