Remove support for loose keys

This commit is contained in:
Steve Sanderson 2019-07-15 16:19:55 +01:00
parent 5141ee930e
commit d5a64fb411
11 changed files with 60 additions and 332 deletions

View File

@ -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.
using System;
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Describes flags associated with a <see cref="RenderTreeFrame"/> whose <see cref="RenderTreeFrame.FrameType"/>
/// equals <see cref="RenderTreeFrameType.Component"/>.
/// </summary>
[Flags]
public enum ComponentFlags: short
{
LooseKey = 1,
}
}

View File

@ -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.
using System;
namespace Microsoft.AspNetCore.Components.RenderTree
{
/// <summary>
/// Describes flags associated with a <see cref="RenderTreeFrame"/> whose <see cref="RenderTreeFrame.FrameType"/>
/// equals <see cref="RenderTreeFrameType.Element"/>.
/// </summary>
[Flags]
public enum ElementFlags: short
{
LooseKey = 1,
}
}

View File

@ -520,25 +520,12 @@ namespace Microsoft.AspNetCore.Components.RenderTree
/// </summary> /// </summary>
/// <param name="value">The value for the key.</param> /// <param name="value">The value for the key.</param>
public void SetKey(object value) public void SetKey(object value)
=> SetKey(value, looseKey: false);
/// <summary>
/// Assigns the specified key value to the current element or component.
/// </summary>
/// <param name="value">The value for the key.</param>
/// <param name="looseKey">If true, null values and duplicates will be ignored. If false, null values and duplicates will be treated as an error.</param>
public void SetKey(object value, bool looseKey)
{ {
if (value == null) if (value == null)
{ {
if (looseKey) // Null is equivalent to not having set a key, which is valuable because Razor syntax doesn't have an
{ // easy way to have conditional directive attributes
return; return;
}
else
{
throw new ArgumentNullException(nameof(value));
}
} }
var parentFrameIndex = GetCurrentParentFrameIndex(); var parentFrameIndex = GetCurrentParentFrameIndex();
@ -552,10 +539,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree
switch (parentFrame.FrameType) switch (parentFrame.FrameType)
{ {
case RenderTreeFrameType.Element: case RenderTreeFrameType.Element:
parentFrame = parentFrame.WithElementKey(value, looseKey); // It's a ref var, so this writes to the array parentFrame = parentFrame.WithElementKey(value); // It's a ref var, so this writes to the array
break; break;
case RenderTreeFrameType.Component: case RenderTreeFrameType.Component:
parentFrame = parentFrame.WithComponentKey(value, looseKey); // It's a ref var, so this writes to the array parentFrame = parentFrame.WithComponentKey(value); // It's a ref var, so this writes to the array
break; break;
default: default:
throw new InvalidOperationException($"Cannot set a key on a frame of type {parentFrame.FrameType}."); throw new InvalidOperationException($"Cannot set a key on a frame of type {parentFrame.FrameType}.");

View File

@ -113,10 +113,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree
keyedItemInfos = BuildKeyToInfoLookup(diffContext, origOldStartIndex, oldEndIndexExcl, origNewStartIndex, newEndIndexExcl); keyedItemInfos = BuildKeyToInfoLookup(diffContext, origOldStartIndex, oldEndIndexExcl, origNewStartIndex, newEndIndexExcl);
} }
var oldKeyItemInfo = oldKey != null ? keyedItemInfos[oldKey] : new KeyedItemInfo(-1, -1, false); var oldKeyItemInfo = oldKey != null ? keyedItemInfos[oldKey] : new KeyedItemInfo(-1, -1);
var newKeyItemInfo = newKey != null ? keyedItemInfos[newKey] : new KeyedItemInfo(-1, -1, false); var newKeyItemInfo = newKey != null ? keyedItemInfos[newKey] : new KeyedItemInfo(-1, -1);
var oldKeyIsInNewTree = oldKeyItemInfo.NewIndex >= 0 && oldKeyItemInfo.IsUnique; var oldKeyIsInNewTree = oldKeyItemInfo.NewIndex >= 0;
var newKeyIsInOldTree = newKeyItemInfo.OldIndex >= 0 && newKeyItemInfo.IsUnique; var newKeyIsInOldTree = newKeyItemInfo.OldIndex >= 0;
// If either key is not in the other tree, we can handle it as an insert or a delete // If either key is not in the other tree, we can handle it as an insert or a delete
// on this iteration. We're only forced to use the move logic that's not the case // on this iteration. We're only forced to use the move logic that's not the case
@ -306,15 +306,12 @@ namespace Microsoft.AspNetCore.Components.RenderTree
var key = KeyValue(ref frame); var key = KeyValue(ref frame);
if (key != null) if (key != null)
{ {
var isUnique = !result.ContainsKey(key); if (result.ContainsKey(key))
if (isUnique || KeyIsLoose(ref frame))
{
result[key] = new KeyedItemInfo(oldStartIndex, -1, isUnique);
}
else
{ {
ThrowExceptionForDuplicateKey(key); ThrowExceptionForDuplicateKey(key);
} }
result[key] = new KeyedItemInfo(oldStartIndex, -1);
} }
oldStartIndex = NextSiblingIndex(frame, oldStartIndex); oldStartIndex = NextSiblingIndex(frame, oldStartIndex);
@ -328,19 +325,16 @@ namespace Microsoft.AspNetCore.Components.RenderTree
{ {
if (!result.TryGetValue(key, out var existingEntry)) if (!result.TryGetValue(key, out var existingEntry))
{ {
result[key] = new KeyedItemInfo(-1, newStartIndex, isUnique: true); result[key] = new KeyedItemInfo(-1, newStartIndex);
} }
else else
{ {
var isUnique = existingEntry.NewIndex < 0; if (existingEntry.NewIndex >= 0)
if (isUnique || KeyIsLoose(ref frame))
{
result[key] = new KeyedItemInfo(existingEntry.OldIndex, newStartIndex, isUnique);
}
else
{ {
ThrowExceptionForDuplicateKey(key); ThrowExceptionForDuplicateKey(key);
} }
result[key] = new KeyedItemInfo(existingEntry.OldIndex, newStartIndex);
} }
} }
@ -352,7 +346,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree
private static void ThrowExceptionForDuplicateKey(object key) private static void ThrowExceptionForDuplicateKey(object key)
{ {
throw new InvalidOperationException($"More than one sibling has the same key value, '{key}'. Key values must be unique, or 'loose' key mode must be used."); throw new InvalidOperationException($"More than one sibling has the same key value, '{key}'. Key values must be unique.");
} }
private static object KeyValue(ref RenderTreeFrame frame) private static object KeyValue(ref RenderTreeFrame frame)
@ -368,19 +362,6 @@ namespace Microsoft.AspNetCore.Components.RenderTree
} }
} }
private static bool KeyIsLoose(ref RenderTreeFrame frame)
{
switch (frame.FrameType)
{
case RenderTreeFrameType.Element:
return frame.ElementFlags.HasFlag(ElementFlags.LooseKey);
case RenderTreeFrameType.Component:
return frame.ComponentFlags.HasFlag(ComponentFlags.LooseKey);
default:
return false;
}
}
// Handles the diff for attribute nodes only - this invariant is enforced by the caller. // Handles the diff for attribute nodes only - this invariant is enforced by the caller.
// //
// The diff for attributes is different because we allow attributes to appear in any order. // The diff for attributes is different because we allow attributes to appear in any order.

View File

@ -53,12 +53,6 @@ namespace Microsoft.AspNetCore.Components.RenderTree
// RenderTreeFrameType.Element // RenderTreeFrameType.Element
// -------------------------------------------------------------------------------- // --------------------------------------------------------------------------------
/// <summary>
/// If the <see cref="FrameType"/> property equals <see cref="RenderTreeFrameType.Component"/>
/// gets the flags associated with the frame.
/// </summary>
[FieldOffset(6)] public readonly ElementFlags ElementFlags;
/// <summary> /// <summary>
/// If the <see cref="FrameType"/> property equals <see cref="RenderTreeFrameType.Element"/> /// If the <see cref="FrameType"/> property equals <see cref="RenderTreeFrameType.Element"/>
/// gets the number of frames in the subtree for which this frame is the root. /// gets the number of frames in the subtree for which this frame is the root.
@ -122,12 +116,6 @@ namespace Microsoft.AspNetCore.Components.RenderTree
// RenderTreeFrameType.Component // RenderTreeFrameType.Component
// -------------------------------------------------------------------------------- // --------------------------------------------------------------------------------
/// <summary>
/// If the <see cref="FrameType"/> property equals <see cref="RenderTreeFrameType.Component"/>
/// gets the flags associated with the frame.
/// </summary>
[FieldOffset(6)] public readonly ComponentFlags ComponentFlags;
/// <summary> /// <summary>
/// If the <see cref="FrameType"/> property equals <see cref="RenderTreeFrameType.Component"/> /// If the <see cref="FrameType"/> property equals <see cref="RenderTreeFrameType.Component"/>
/// gets the number of frames in the subtree for which this frame is the root. /// gets the number of frames in the subtree for which this frame is the root.
@ -225,24 +213,22 @@ namespace Microsoft.AspNetCore.Components.RenderTree
[FieldOffset(16)] public readonly string MarkupContent; [FieldOffset(16)] public readonly string MarkupContent;
// Element constructor // Element constructor
private RenderTreeFrame(int sequence, ElementFlags elementFlags, int elementSubtreeLength, string elementName, object elementKey) private RenderTreeFrame(int sequence, int elementSubtreeLength, string elementName, object elementKey)
: this() : this()
{ {
Sequence = sequence; Sequence = sequence;
FrameType = RenderTreeFrameType.Element; FrameType = RenderTreeFrameType.Element;
ElementFlags = elementFlags;
ElementSubtreeLength = elementSubtreeLength; ElementSubtreeLength = elementSubtreeLength;
ElementName = elementName; ElementName = elementName;
ElementKey = elementKey; ElementKey = elementKey;
} }
// Component constructor // Component constructor
private RenderTreeFrame(int sequence, ComponentFlags componentFlags, int componentSubtreeLength, Type componentType, ComponentState componentState, object componentKey) private RenderTreeFrame(int sequence, int componentSubtreeLength, Type componentType, ComponentState componentState, object componentKey)
: this() : this()
{ {
Sequence = sequence; Sequence = sequence;
FrameType = RenderTreeFrameType.Component; FrameType = RenderTreeFrameType.Component;
ComponentFlags = componentFlags;
ComponentSubtreeLength = componentSubtreeLength; ComponentSubtreeLength = componentSubtreeLength;
ComponentType = componentType; ComponentType = componentType;
ComponentKey = componentKey; ComponentKey = componentKey;
@ -313,7 +299,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree
} }
internal static RenderTreeFrame Element(int sequence, string elementName) internal static RenderTreeFrame Element(int sequence, string elementName)
=> new RenderTreeFrame(sequence, elementFlags: default, elementSubtreeLength: 0, elementName, null); => new RenderTreeFrame(sequence, elementSubtreeLength: 0, elementName, null);
internal static RenderTreeFrame Text(int sequence, string textContent) internal static RenderTreeFrame Text(int sequence, string textContent)
=> new RenderTreeFrame(sequence, isMarkup: false, textOrMarkup: textContent); => new RenderTreeFrame(sequence, isMarkup: false, textOrMarkup: textContent);
@ -325,10 +311,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree
=> new RenderTreeFrame(sequence, attributeName: name, attributeValue: value, attributeEventHandlerId: 0, attributeEventUpdatesAttributeName: null); => new RenderTreeFrame(sequence, attributeName: name, attributeValue: value, attributeEventHandlerId: 0, attributeEventUpdatesAttributeName: null);
internal static RenderTreeFrame ChildComponent(int sequence, Type componentType) internal static RenderTreeFrame ChildComponent(int sequence, Type componentType)
=> new RenderTreeFrame(sequence, componentFlags: default, componentSubtreeLength: 0, componentType, null, null); => new RenderTreeFrame(sequence, componentSubtreeLength: 0, componentType, null, null);
internal static RenderTreeFrame PlaceholderChildComponentWithSubtreeLength(int subtreeLength) internal static RenderTreeFrame PlaceholderChildComponentWithSubtreeLength(int subtreeLength)
=> new RenderTreeFrame(0, componentFlags: default, componentSubtreeLength: subtreeLength, typeof(IComponent), null, null); => new RenderTreeFrame(0, componentSubtreeLength: subtreeLength, typeof(IComponent), null, null);
internal static RenderTreeFrame Region(int sequence) internal static RenderTreeFrame Region(int sequence)
=> new RenderTreeFrame(sequence, regionSubtreeLength: 0); => new RenderTreeFrame(sequence, regionSubtreeLength: 0);
@ -340,16 +326,16 @@ namespace Microsoft.AspNetCore.Components.RenderTree
=> new RenderTreeFrame(sequence, componentReferenceCaptureAction: componentReferenceCaptureAction, parentFrameIndex: parentFrameIndex); => new RenderTreeFrame(sequence, componentReferenceCaptureAction: componentReferenceCaptureAction, parentFrameIndex: parentFrameIndex);
internal RenderTreeFrame WithElementSubtreeLength(int elementSubtreeLength) internal RenderTreeFrame WithElementSubtreeLength(int elementSubtreeLength)
=> new RenderTreeFrame(Sequence, elementFlags: ElementFlags, elementSubtreeLength: elementSubtreeLength, ElementName, ElementKey); => new RenderTreeFrame(Sequence, elementSubtreeLength: elementSubtreeLength, ElementName, ElementKey);
internal RenderTreeFrame WithComponentSubtreeLength(int componentSubtreeLength) internal RenderTreeFrame WithComponentSubtreeLength(int componentSubtreeLength)
=> new RenderTreeFrame(Sequence, componentFlags: ComponentFlags, componentSubtreeLength: componentSubtreeLength, ComponentType, ComponentState, ComponentKey); => new RenderTreeFrame(Sequence, componentSubtreeLength: componentSubtreeLength, ComponentType, ComponentState, ComponentKey);
internal RenderTreeFrame WithAttributeSequence(int sequence) internal RenderTreeFrame WithAttributeSequence(int sequence)
=> new RenderTreeFrame(sequence, attributeName: AttributeName, AttributeValue, AttributeEventHandlerId, AttributeEventUpdatesAttributeName); => new RenderTreeFrame(sequence, attributeName: AttributeName, AttributeValue, AttributeEventHandlerId, AttributeEventUpdatesAttributeName);
internal RenderTreeFrame WithComponent(ComponentState componentState) internal RenderTreeFrame WithComponent(ComponentState componentState)
=> new RenderTreeFrame(Sequence, componentFlags: ComponentFlags, componentSubtreeLength: ComponentSubtreeLength, ComponentType, componentState, ComponentKey); => new RenderTreeFrame(Sequence, componentSubtreeLength: ComponentSubtreeLength, ComponentType, componentState, ComponentKey);
internal RenderTreeFrame WithAttributeEventHandlerId(int eventHandlerId) internal RenderTreeFrame WithAttributeEventHandlerId(int eventHandlerId)
=> new RenderTreeFrame(Sequence, attributeName: AttributeName, AttributeValue, eventHandlerId, AttributeEventUpdatesAttributeName); => new RenderTreeFrame(Sequence, attributeName: AttributeName, AttributeValue, eventHandlerId, AttributeEventUpdatesAttributeName);
@ -366,17 +352,11 @@ namespace Microsoft.AspNetCore.Components.RenderTree
internal RenderTreeFrame WithElementReferenceCaptureId(string elementReferenceCaptureId) internal RenderTreeFrame WithElementReferenceCaptureId(string elementReferenceCaptureId)
=> new RenderTreeFrame(Sequence, elementReferenceCaptureAction: ElementReferenceCaptureAction, elementReferenceCaptureId); => new RenderTreeFrame(Sequence, elementReferenceCaptureAction: ElementReferenceCaptureAction, elementReferenceCaptureId);
internal RenderTreeFrame WithElementKey(object elementKey, bool looseKey) internal RenderTreeFrame WithElementKey(object elementKey)
{ => new RenderTreeFrame(Sequence, elementSubtreeLength: ElementSubtreeLength, ElementName, elementKey);
var flags = looseKey ? (ElementFlags | ElementFlags.LooseKey) : ElementFlags;
return new RenderTreeFrame(Sequence, elementFlags: flags, elementSubtreeLength: ElementSubtreeLength, ElementName, elementKey);
}
internal RenderTreeFrame WithComponentKey(object componentKey, bool looseKey) internal RenderTreeFrame WithComponentKey(object componentKey)
{ => new RenderTreeFrame(Sequence, componentSubtreeLength: ComponentSubtreeLength, ComponentType, ComponentState, componentKey);
var flags = looseKey ? (ComponentFlags | ComponentFlags.LooseKey) : ComponentFlags;
return new RenderTreeFrame(Sequence, componentFlags: flags, componentSubtreeLength: ComponentSubtreeLength, ComponentType, ComponentState, componentKey);
}
/// <inheritdoc /> /// <inheritdoc />
// Just to be nice for debugging and unit tests. // Just to be nice for debugging and unit tests.

View File

@ -10,24 +10,13 @@ namespace Microsoft.AspNetCore.Components.Rendering
public readonly int NewIndex; public readonly int NewIndex;
public readonly int OldSiblingIndex; public readonly int OldSiblingIndex;
public readonly int NewSiblingIndex; public readonly int NewSiblingIndex;
public readonly bool IsUnique;
public KeyedItemInfo(int oldIndex, int newIndex, bool isUnique) public KeyedItemInfo(int oldIndex, int newIndex)
{ {
OldIndex = oldIndex; OldIndex = oldIndex;
NewIndex = newIndex; NewIndex = newIndex;
OldSiblingIndex = -1; OldSiblingIndex = -1;
NewSiblingIndex = -1; NewSiblingIndex = -1;
// Non-unique keys are problematic, because there's no way to know which instance
// should match with which other, plus they would force us to keep track of which
// usages have been consumed as we proceed through the diff. Since this is such
// an edge case, we "tolerate" it just by tracking which keys have duplicates, and
// for those ones, we never treat them as moved. Instead for those we fall back on
// insert+delete behavior, i.e., not preserving elements/components.
//
// Guidance for developers is therefore to use distinct keys.
IsUnique = isUnique;
} }
private KeyedItemInfo(in KeyedItemInfo copyFrom, int oldSiblingIndex, int newSiblingIndex) private KeyedItemInfo(in KeyedItemInfo copyFrom, int oldSiblingIndex, int newSiblingIndex)

View File

@ -1475,10 +1475,8 @@ namespace Microsoft.AspNetCore.Components.Test
frame => AssertFrame.Element(frame, "elem", 1, 0)); frame => AssertFrame.Element(frame, "elem", 1, 0));
} }
[Theory] [Fact]
[InlineData(true)] public void CanAddKeyToElement()
[InlineData(false)]
public void CanAddKeyToElement(bool looseKey)
{ {
// Arrange // Arrange
var builder = new RenderTreeBuilder(new TestRenderer()); var builder = new RenderTreeBuilder(new TestRenderer());
@ -1487,7 +1485,7 @@ namespace Microsoft.AspNetCore.Components.Test
// Act // Act
builder.OpenElement(0, "elem"); builder.OpenElement(0, "elem");
builder.AddAttribute(1, "attribute before", "before value"); builder.AddAttribute(1, "attribute before", "before value");
builder.SetKey(keyValue, looseKey); builder.SetKey(keyValue);
builder.AddAttribute(2, "attribute after", "after value"); builder.AddAttribute(2, "attribute after", "after value");
builder.CloseElement(); builder.CloseElement();
@ -1498,16 +1496,13 @@ namespace Microsoft.AspNetCore.Components.Test
{ {
AssertFrame.Element(frame, "elem", 3, 0); AssertFrame.Element(frame, "elem", 3, 0);
Assert.Same(keyValue, frame.ElementKey); Assert.Same(keyValue, frame.ElementKey);
Assert.Equal(looseKey, frame.ElementFlags.HasFlag(ElementFlags.LooseKey));
}, },
frame => AssertFrame.Attribute(frame, "attribute before", "before value", 1), frame => AssertFrame.Attribute(frame, "attribute before", "before value", 1),
frame => AssertFrame.Attribute(frame, "attribute after", "after value", 2)); frame => AssertFrame.Attribute(frame, "attribute after", "after value", 2));
} }
[Theory] [Fact]
[InlineData(true)] public void CanAddKeyToComponent()
[InlineData(false)]
public void CanAddKeyToComponent(bool looseKey)
{ {
// Arrange // Arrange
var builder = new RenderTreeBuilder(new TestRenderer()); var builder = new RenderTreeBuilder(new TestRenderer());
@ -1516,7 +1511,7 @@ namespace Microsoft.AspNetCore.Components.Test
// Act // Act
builder.OpenComponent<TestComponent>(0); builder.OpenComponent<TestComponent>(0);
builder.AddAttribute(1, "param before", 123); builder.AddAttribute(1, "param before", 123);
builder.SetKey(keyValue, looseKey); builder.SetKey(keyValue);
builder.AddAttribute(2, "param after", 456); builder.AddAttribute(2, "param after", 456);
builder.CloseComponent(); builder.CloseComponent();
@ -1527,7 +1522,6 @@ namespace Microsoft.AspNetCore.Components.Test
{ {
AssertFrame.Component<TestComponent>(frame, 3, 0); AssertFrame.Component<TestComponent>(frame, 3, 0);
Assert.Same(keyValue, frame.ComponentKey); Assert.Same(keyValue, frame.ComponentKey);
Assert.Equal(looseKey, frame.ComponentFlags.HasFlag(ComponentFlags.LooseKey));
}, },
frame => AssertFrame.Attribute(frame, "param before", 123, 1), frame => AssertFrame.Attribute(frame, "param before", 123, 1),
frame => AssertFrame.Attribute(frame, "param after", 456, 2)); frame => AssertFrame.Attribute(frame, "param after", 456, 2));
@ -1564,33 +1558,14 @@ namespace Microsoft.AspNetCore.Components.Test
} }
[Fact] [Fact]
public void CannotAddNullKey() public void IgnoresNullElementKey()
{
// Although we could translate 'null' into either some default "null key"
// instance, or just no-op the call, it almost certainly indicates a programming
// error so it's better to fail.
// Arrange
var builder = new RenderTreeBuilder(new TestRenderer());
// Act/Assert
var ex = Assert.Throws<ArgumentNullException>(() =>
{
builder.OpenElement(0, "elem");
builder.SetKey(null);
});
Assert.Equal("value", ex.ParamName);
}
[Fact]
public void IgnoresNullElementKeyIfLoose()
{ {
// Arrange // Arrange
var builder = new RenderTreeBuilder(new TestRenderer()); var builder = new RenderTreeBuilder(new TestRenderer());
// Act // Act
builder.OpenElement(0, "elem"); builder.OpenElement(0, "elem");
builder.SetKey(null, looseKey: true); builder.SetKey(null);
builder.CloseElement(); builder.CloseElement();
// Assert // Assert
@ -1604,14 +1579,14 @@ namespace Microsoft.AspNetCore.Components.Test
} }
[Fact] [Fact]
public void IgnoresNullComponentKeyIfLoose() public void IgnoresNullComponentKey()
{ {
// Arrange // Arrange
var builder = new RenderTreeBuilder(new TestRenderer()); var builder = new RenderTreeBuilder(new TestRenderer());
// Act // Act
builder.OpenComponent<TestComponent>(0); builder.OpenComponent<TestComponent>(0);
builder.SetKey(null, looseKey: true); builder.SetKey(null);
builder.CloseComponent(); builder.CloseComponent();
// Assert // Assert

View File

@ -315,7 +315,7 @@ namespace Microsoft.AspNetCore.Components.Test
} }
[Fact] [Fact]
public void HandlesClashingKeysInOldTree_Strict() public void RejectsClashingKeysInOldTree()
{ {
// Arrange // Arrange
AddWithKey(oldTree, "key1", "attrib1a"); AddWithKey(oldTree, "key1", "attrib1a");
@ -328,11 +328,11 @@ namespace Microsoft.AspNetCore.Components.Test
// Act/Assert // Act/Assert
var ex = Assert.Throws<InvalidOperationException>(() => GetSingleUpdatedComponent()); var ex = Assert.Throws<InvalidOperationException>(() => GetSingleUpdatedComponent());
Assert.Equal("More than one sibling has the same key value, 'key1'. Key values must be unique, or 'loose' key mode must be used.", ex.Message); Assert.Equal("More than one sibling has the same key value, 'key1'. Key values must be unique.", ex.Message);
} }
[Fact] [Fact]
public void HandlesClashingKeysInNewTree_Strict() public void RejectsClashingKeysInNewTree()
{ {
// Arrange // Arrange
AddWithKey(oldTree, "key1", "attrib1a"); AddWithKey(oldTree, "key1", "attrib1a");
@ -345,100 +345,7 @@ namespace Microsoft.AspNetCore.Components.Test
// Act/Assert // Act/Assert
var ex = Assert.Throws<InvalidOperationException>(() => GetSingleUpdatedComponent()); var ex = Assert.Throws<InvalidOperationException>(() => GetSingleUpdatedComponent());
Assert.Equal("More than one sibling has the same key value, 'key1'. Key values must be unique, or 'loose' key mode must be used.", ex.Message); Assert.Equal("More than one sibling has the same key value, 'key1'. Key values must be unique.", ex.Message);
}
[Fact]
public void HandlesClashingKeys_Loose_FirstUsage()
{
// This scenario is problematic for the algorithm if it uses a "first key
// usage wins" policy for duplicate keys. It would not end up with attrib1b
// anywhere in the output, because whenever it sees key1 in oldTree, it tries
// to diff against the first usage of key1 in newTree, which has attrib1a.
// However, because of the actual "duplicated keys are excluded from the
// dictionary match" policy, we don't preserve any of the key1 items, and
// the diff is valid.
// Since we only pass the "loose" flag for the key that would otherwise
// trigger an exception due to a clash, this shows that "loose" only matters
// in that case. It would be fine to pass loose:true for all the others too,
// as would normally be done in .razor, but it makes no difference.
// Arrange
AddWithKey(oldTree, "key3", "attrib3");
AddWithKey(oldTree, "key1", "attrib1a");
AddWithKey(oldTree, "key1", "attrib1a", looseKey: true);
AddWithKey(oldTree, "key2", "attrib2");
AddWithKey(newTree, "key1", "attrib1a");
AddWithKey(newTree, "key2", "attrib2");
AddWithKey(newTree, "key1", "attrib1b", looseKey: true);
// Act
var (result, referenceFrames) = GetSingleUpdatedComponent();
// Assert
Assert.Collection(result.Edits,
// Insert key1+attrib1a at the top
edit =>
{
AssertEdit(edit, RenderTreeEditType.PrependFrame, 0);
Assert.Equal("attrib1a", referenceFrames[edit.ReferenceFrameIndex + 1].AttributeValue);
},
// Delete key3+attrib3
edit => AssertEdit(edit, RenderTreeEditType.RemoveFrame, 1),
// Delete key1+attrib1a
edit => AssertEdit(edit, RenderTreeEditType.RemoveFrame, 1),
// Delete the other key1+attrib1a
edit => AssertEdit(edit, RenderTreeEditType.RemoveFrame, 1),
// Insert key1+attrib1b at the bottom
edit =>
{
AssertEdit(edit, RenderTreeEditType.PrependFrame, 2);
Assert.Equal("attrib1b", referenceFrames[edit.ReferenceFrameIndex + 1].AttributeValue);
});
}
[Fact]
public void HandlesClashingKeys_Loose_LastUsage()
{
// This scenario is problematic for the algorithm if it uses a "last key
// usage wins" policy for duplicate keys. It would not end up with attrib1b
// anywhere in the output, because when it sees key1 in oldTree, it tries
// to diff against the last usage of key1 in newTree, which has attrib1a.
// However, because of the actual "duplicated keys are excluded from the
// dictionary match" policy, we don't preserve any of the key1 items, and
// the diff is valid.
// Since we only pass the "loose" flag for the key that would otherwise
// trigger an exception due to a clash, this shows that "loose" only matters
// in that case. It would be fine to pass loose:true for all the others too,
// as would normally be done in .razor, but it makes no difference.
// Arrange
AddWithKey(oldTree, "key1", "attrib1a");
AddWithKey(oldTree, "key2", "attrib2");
AddWithKey(oldTree, "key1", "attrib1b", looseKey: true);
AddWithKey(newTree, "key2", "attrib2");
AddWithKey(newTree, "key1", "attrib1b");
AddWithKey(newTree, "key1", "attrib1a", looseKey: true);
// Act
var (result, referenceFrames) = GetSingleUpdatedComponent();
// Assert
Assert.Collection(result.Edits,
// Delete key1+attrib1a
edit => AssertEdit(edit, RenderTreeEditType.RemoveFrame, 0),
// Insert a new key1+attrib1a at the bottom
edit =>
{
AssertEdit(edit, RenderTreeEditType.PrependFrame, 2);
Assert.Equal("attrib1a", referenceFrames[edit.ReferenceFrameIndex + 1].AttributeValue);
});
} }
[Fact] [Fact]
@ -2237,19 +2144,10 @@ namespace Microsoft.AspNetCore.Components.Test
.Select(x => (T)x.Component) .Select(x => (T)x.Component)
.ToList(); .ToList();
private static void AddWithKey(RenderTreeBuilder builder, object key, string attributeValue = null, bool looseKey = false) private static void AddWithKey(RenderTreeBuilder builder, object key, string attributeValue = null)
{ {
builder.OpenElement(0, "el"); builder.OpenElement(0, "el");
builder.SetKey(key);
if (looseKey)
{
builder.SetKey(key, looseKey: true);
}
else
{
// Using this overload here (not explicitly passing any flag) to show that the default is strict
builder.SetKey(key);
}
if (attributeValue != null) if (attributeValue != null)
{ {

View File

@ -206,25 +206,6 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests
}); });
} }
[Fact]
public void CanDuplicateKeysInLooseMode()
{
PerformTest(
useLooseKeys: true,
before: new[]
{
new Node("orig1", "A"),
new Node("orig2", "B"),
},
after: new[]
{
new Node("orig1", "A edited"),
new Node("orig1", "A inserted") { IsNew = true },
new Node("orig2", "B edited"),
new Node("orig2", "B inserted") { IsNew = true },
});
}
[Fact] [Fact]
public async Task CanRetainFocusWhileMovingTextBox() public async Task CanRetainFocusWhileMovingTextBox()
{ {
@ -291,7 +272,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests
Browser.Equal(Array.Empty<bool>(), completeItemStates); Browser.Equal(Array.Empty<bool>(), completeItemStates);
} }
private void PerformTest(Node[] before, Node[] after, bool useLooseKeys = false) private void PerformTest(Node[] before, Node[] after)
{ {
var rootBefore = new Node(null, "root", before); var rootBefore = new Node(null, "root", before);
var rootAfter = new Node(null, "root", after); var rootAfter = new Node(null, "root", after);
@ -302,11 +283,6 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests
var textbox = appElem.FindElement(By.TagName("textarea")); var textbox = appElem.FindElement(By.TagName("textarea"));
var updateButton = appElem.FindElement(By.TagName("button")); var updateButton = appElem.FindElement(By.TagName("button"));
if (useLooseKeys)
{
appElem.FindElement(By.Id("key-loose")).Click();
}
SetTextAreaValueFast(textbox, jsonBefore); SetTextAreaValueFast(textbox, jsonBefore);
updateButton.Click(); updateButton.Click();
ValidateRenderedOutput(appElem, rootBefore, validatePreservation: false); ValidateRenderedOutput(appElem, rootBefore, validatePreservation: false);

View File

@ -4,10 +4,6 @@
<p>Model</p> <p>Model</p>
<textarea @bind="modelJson" id="key-model"></textarea> <textarea @bind="modelJson" id="key-model"></textarea>
<button @onclick="Update">Update</button> <button @onclick="Update">Update</button>
<label>
<input type="checkbox" @bind="@renderContext.UseLooseKeys" id="key-loose" />
Use loose keys
</label>
</div> </div>
<div class="render-output"> <div class="render-output">
<p>Output</p> <p>Output</p>
@ -73,7 +69,5 @@
// This is so the descendants can detect and display whether they are // This is so the descendants can detect and display whether they are
// newly-instantiated on any given render // newly-instantiated on any given render
public int UpdateCount { get; set; } public int UpdateCount { get; set; }
public bool UseLooseKeys { get; set; }
} }
} }

View File

@ -1,4 +1,3 @@
@using Microsoft.AspNetCore.Components.RenderTree
<div class="node"> <div class="node">
<strong class="label">@Data.Label</strong> <strong class="label">@Data.Label</strong>
&nbsp;&nbsp;[ &nbsp;&nbsp;[
@ -17,9 +16,19 @@
@if (Data.Children?.Any() ?? false) @if (Data.Children?.Any() ?? false)
{ {
<div class="children"> <div class="children">@{
@((RenderFragment)RenderKeyCasesTreeNodes) foreach (var child in Data.Children)
</div> {
if (child.Key != null)
{
<KeyCasesTreeNode @key="@child.Key" Data="@child" />
}
else
{
<KeyCasesTreeNode Data="@child" />
}
}
}</div>
} }
</div> </div>
@ -41,31 +50,4 @@
{ {
firstCreatedOnUpdateCount = RenderContext.UpdateCount; firstCreatedOnUpdateCount = RenderContext.UpdateCount;
} }
void RenderKeyCasesTreeNodes(RenderTreeBuilder builder)
{
// This is equivalent to:
// @foreach (var child in Data.Children)
// {
// if (key != null)
// {
// <KeyCasesTreeNode @key="@child.Key" @key:loose="@looseKey" Data="@child" />
// }
// else
// {
// <KeyCasesTreeNode Data="@child" />
// }
// }
// TODO: Once the compiler supports @key:loose, eliminate this and just use regular Razor syntax
foreach (var child in Data.Children)
{
builder.OpenComponent<KeyCasesTreeNode>(0);
if (child.Key != null)
{
builder.SetKey(child.Key, looseKey: RenderContext.UseLooseKeys);
}
builder.AddAttribute(1, nameof(Data), child);
builder.CloseComponent();
}
}
} }