diff --git a/src/Components/Components/src/RenderTree/ComponentFlags.cs b/src/Components/Components/src/RenderTree/ComponentFlags.cs deleted file mode 100644 index 4c39fa63eb..0000000000 --- a/src/Components/Components/src/RenderTree/ComponentFlags.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. - -using System; - -namespace Microsoft.AspNetCore.Components.RenderTree -{ - /// - /// Describes flags associated with a whose - /// equals . - /// - [Flags] - public enum ComponentFlags: short - { - LooseKey = 1, - } -} diff --git a/src/Components/Components/src/RenderTree/ElementFlags.cs b/src/Components/Components/src/RenderTree/ElementFlags.cs deleted file mode 100644 index 359adbee4f..0000000000 --- a/src/Components/Components/src/RenderTree/ElementFlags.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. - -using System; - -namespace Microsoft.AspNetCore.Components.RenderTree -{ - /// - /// Describes flags associated with a whose - /// equals . - /// - [Flags] - public enum ElementFlags: short - { - LooseKey = 1, - } -} diff --git a/src/Components/Components/src/RenderTree/RenderTreeBuilder.cs b/src/Components/Components/src/RenderTree/RenderTreeBuilder.cs index a58bcc372a..85c1ffa15a 100644 --- a/src/Components/Components/src/RenderTree/RenderTreeBuilder.cs +++ b/src/Components/Components/src/RenderTree/RenderTreeBuilder.cs @@ -520,25 +520,12 @@ namespace Microsoft.AspNetCore.Components.RenderTree /// /// The value for the key. public void SetKey(object value) - => SetKey(value, looseKey: false); - - /// - /// Assigns the specified key value to the current element or component. - /// - /// The value for the key. - /// If true, null values and duplicates will be ignored. If false, null values and duplicates will be treated as an error. - public void SetKey(object value, bool looseKey) { if (value == null) { - if (looseKey) - { - return; - } - else - { - throw new ArgumentNullException(nameof(value)); - } + // 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; } var parentFrameIndex = GetCurrentParentFrameIndex(); @@ -552,10 +539,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree switch (parentFrame.FrameType) { 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; 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; default: throw new InvalidOperationException($"Cannot set a key on a frame of type {parentFrame.FrameType}."); diff --git a/src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs b/src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs index bc88a3f2c2..933e1f9ab5 100644 --- a/src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs +++ b/src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs @@ -113,10 +113,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree keyedItemInfos = BuildKeyToInfoLookup(diffContext, origOldStartIndex, oldEndIndexExcl, origNewStartIndex, newEndIndexExcl); } - var oldKeyItemInfo = oldKey != null ? keyedItemInfos[oldKey] : new KeyedItemInfo(-1, -1, false); - var newKeyItemInfo = newKey != null ? keyedItemInfos[newKey] : new KeyedItemInfo(-1, -1, false); - var oldKeyIsInNewTree = oldKeyItemInfo.NewIndex >= 0 && oldKeyItemInfo.IsUnique; - var newKeyIsInOldTree = newKeyItemInfo.OldIndex >= 0 && newKeyItemInfo.IsUnique; + var oldKeyItemInfo = oldKey != null ? keyedItemInfos[oldKey] : new KeyedItemInfo(-1, -1); + var newKeyItemInfo = newKey != null ? keyedItemInfos[newKey] : new KeyedItemInfo(-1, -1); + var oldKeyIsInNewTree = oldKeyItemInfo.NewIndex >= 0; + var newKeyIsInOldTree = newKeyItemInfo.OldIndex >= 0; // 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 @@ -306,15 +306,12 @@ namespace Microsoft.AspNetCore.Components.RenderTree var key = KeyValue(ref frame); if (key != null) { - var isUnique = !result.ContainsKey(key); - if (isUnique || KeyIsLoose(ref frame)) - { - result[key] = new KeyedItemInfo(oldStartIndex, -1, isUnique); - } - else + if (result.ContainsKey(key)) { ThrowExceptionForDuplicateKey(key); } + + result[key] = new KeyedItemInfo(oldStartIndex, -1); } oldStartIndex = NextSiblingIndex(frame, oldStartIndex); @@ -328,19 +325,16 @@ namespace Microsoft.AspNetCore.Components.RenderTree { if (!result.TryGetValue(key, out var existingEntry)) { - result[key] = new KeyedItemInfo(-1, newStartIndex, isUnique: true); + result[key] = new KeyedItemInfo(-1, newStartIndex); } else { - var isUnique = existingEntry.NewIndex < 0; - if (isUnique || KeyIsLoose(ref frame)) - { - result[key] = new KeyedItemInfo(existingEntry.OldIndex, newStartIndex, isUnique); - } - else + if (existingEntry.NewIndex >= 0) { ThrowExceptionForDuplicateKey(key); } + + result[key] = new KeyedItemInfo(existingEntry.OldIndex, newStartIndex); } } @@ -352,7 +346,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree 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) @@ -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. // // The diff for attributes is different because we allow attributes to appear in any order. diff --git a/src/Components/Components/src/RenderTree/RenderTreeFrame.cs b/src/Components/Components/src/RenderTree/RenderTreeFrame.cs index 30cbf6d845..1e61f34a1d 100644 --- a/src/Components/Components/src/RenderTree/RenderTreeFrame.cs +++ b/src/Components/Components/src/RenderTree/RenderTreeFrame.cs @@ -53,12 +53,6 @@ namespace Microsoft.AspNetCore.Components.RenderTree // RenderTreeFrameType.Element // -------------------------------------------------------------------------------- - /// - /// If the property equals - /// gets the flags associated with the frame. - /// - [FieldOffset(6)] public readonly ElementFlags ElementFlags; - /// /// If the property equals /// 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 // -------------------------------------------------------------------------------- - /// - /// If the property equals - /// gets the flags associated with the frame. - /// - [FieldOffset(6)] public readonly ComponentFlags ComponentFlags; - /// /// If the property equals /// 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; // 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() { Sequence = sequence; FrameType = RenderTreeFrameType.Element; - ElementFlags = elementFlags; ElementSubtreeLength = elementSubtreeLength; ElementName = elementName; ElementKey = elementKey; } // 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() { Sequence = sequence; FrameType = RenderTreeFrameType.Component; - ComponentFlags = componentFlags; ComponentSubtreeLength = componentSubtreeLength; ComponentType = componentType; ComponentKey = componentKey; @@ -313,7 +299,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree } 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) => 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); 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) - => 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) => new RenderTreeFrame(sequence, regionSubtreeLength: 0); @@ -340,16 +326,16 @@ namespace Microsoft.AspNetCore.Components.RenderTree => new RenderTreeFrame(sequence, componentReferenceCaptureAction: componentReferenceCaptureAction, parentFrameIndex: parentFrameIndex); 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) - => new RenderTreeFrame(Sequence, componentFlags: ComponentFlags, componentSubtreeLength: componentSubtreeLength, ComponentType, ComponentState, ComponentKey); + => new RenderTreeFrame(Sequence, componentSubtreeLength: componentSubtreeLength, ComponentType, ComponentState, ComponentKey); internal RenderTreeFrame WithAttributeSequence(int sequence) => new RenderTreeFrame(sequence, attributeName: AttributeName, AttributeValue, AttributeEventHandlerId, AttributeEventUpdatesAttributeName); 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) => new RenderTreeFrame(Sequence, attributeName: AttributeName, AttributeValue, eventHandlerId, AttributeEventUpdatesAttributeName); @@ -366,17 +352,11 @@ namespace Microsoft.AspNetCore.Components.RenderTree internal RenderTreeFrame WithElementReferenceCaptureId(string elementReferenceCaptureId) => new RenderTreeFrame(Sequence, elementReferenceCaptureAction: ElementReferenceCaptureAction, elementReferenceCaptureId); - internal RenderTreeFrame WithElementKey(object elementKey, bool looseKey) - { - var flags = looseKey ? (ElementFlags | ElementFlags.LooseKey) : ElementFlags; - return new RenderTreeFrame(Sequence, elementFlags: flags, elementSubtreeLength: ElementSubtreeLength, ElementName, elementKey); - } + internal RenderTreeFrame WithElementKey(object elementKey) + => new RenderTreeFrame(Sequence, elementSubtreeLength: ElementSubtreeLength, ElementName, elementKey); - internal RenderTreeFrame WithComponentKey(object componentKey, bool looseKey) - { - var flags = looseKey ? (ComponentFlags | ComponentFlags.LooseKey) : ComponentFlags; - return new RenderTreeFrame(Sequence, componentFlags: flags, componentSubtreeLength: ComponentSubtreeLength, ComponentType, ComponentState, componentKey); - } + internal RenderTreeFrame WithComponentKey(object componentKey) + => new RenderTreeFrame(Sequence, componentSubtreeLength: ComponentSubtreeLength, ComponentType, ComponentState, componentKey); /// // Just to be nice for debugging and unit tests. diff --git a/src/Components/Components/src/Rendering/KeyedItemInfo.cs b/src/Components/Components/src/Rendering/KeyedItemInfo.cs index f2a56341e2..c8a7de5e1a 100644 --- a/src/Components/Components/src/Rendering/KeyedItemInfo.cs +++ b/src/Components/Components/src/Rendering/KeyedItemInfo.cs @@ -10,24 +10,13 @@ namespace Microsoft.AspNetCore.Components.Rendering public readonly int NewIndex; public readonly int OldSiblingIndex; public readonly int NewSiblingIndex; - public readonly bool IsUnique; - public KeyedItemInfo(int oldIndex, int newIndex, bool isUnique) + public KeyedItemInfo(int oldIndex, int newIndex) { OldIndex = oldIndex; NewIndex = newIndex; OldSiblingIndex = -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) diff --git a/src/Components/Components/test/RenderTreeBuilderTest.cs b/src/Components/Components/test/RenderTreeBuilderTest.cs index 4c090d320b..4b5c63ea2c 100644 --- a/src/Components/Components/test/RenderTreeBuilderTest.cs +++ b/src/Components/Components/test/RenderTreeBuilderTest.cs @@ -1475,10 +1475,8 @@ namespace Microsoft.AspNetCore.Components.Test frame => AssertFrame.Element(frame, "elem", 1, 0)); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void CanAddKeyToElement(bool looseKey) + [Fact] + public void CanAddKeyToElement() { // Arrange var builder = new RenderTreeBuilder(new TestRenderer()); @@ -1487,7 +1485,7 @@ namespace Microsoft.AspNetCore.Components.Test // Act builder.OpenElement(0, "elem"); builder.AddAttribute(1, "attribute before", "before value"); - builder.SetKey(keyValue, looseKey); + builder.SetKey(keyValue); builder.AddAttribute(2, "attribute after", "after value"); builder.CloseElement(); @@ -1498,16 +1496,13 @@ namespace Microsoft.AspNetCore.Components.Test { AssertFrame.Element(frame, "elem", 3, 0); 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 after", "after value", 2)); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void CanAddKeyToComponent(bool looseKey) + [Fact] + public void CanAddKeyToComponent() { // Arrange var builder = new RenderTreeBuilder(new TestRenderer()); @@ -1516,7 +1511,7 @@ namespace Microsoft.AspNetCore.Components.Test // Act builder.OpenComponent(0); builder.AddAttribute(1, "param before", 123); - builder.SetKey(keyValue, looseKey); + builder.SetKey(keyValue); builder.AddAttribute(2, "param after", 456); builder.CloseComponent(); @@ -1527,7 +1522,6 @@ namespace Microsoft.AspNetCore.Components.Test { AssertFrame.Component(frame, 3, 0); 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 after", 456, 2)); @@ -1564,33 +1558,14 @@ namespace Microsoft.AspNetCore.Components.Test } [Fact] - public void CannotAddNullKey() - { - // 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(() => - { - builder.OpenElement(0, "elem"); - builder.SetKey(null); - }); - Assert.Equal("value", ex.ParamName); - } - - [Fact] - public void IgnoresNullElementKeyIfLoose() + public void IgnoresNullElementKey() { // Arrange var builder = new RenderTreeBuilder(new TestRenderer()); // Act builder.OpenElement(0, "elem"); - builder.SetKey(null, looseKey: true); + builder.SetKey(null); builder.CloseElement(); // Assert @@ -1604,14 +1579,14 @@ namespace Microsoft.AspNetCore.Components.Test } [Fact] - public void IgnoresNullComponentKeyIfLoose() + public void IgnoresNullComponentKey() { // Arrange var builder = new RenderTreeBuilder(new TestRenderer()); // Act builder.OpenComponent(0); - builder.SetKey(null, looseKey: true); + builder.SetKey(null); builder.CloseComponent(); // Assert diff --git a/src/Components/Components/test/RenderTreeDiffBuilderTest.cs b/src/Components/Components/test/RenderTreeDiffBuilderTest.cs index cc0b29ea5b..00d21eb97c 100644 --- a/src/Components/Components/test/RenderTreeDiffBuilderTest.cs +++ b/src/Components/Components/test/RenderTreeDiffBuilderTest.cs @@ -315,7 +315,7 @@ namespace Microsoft.AspNetCore.Components.Test } [Fact] - public void HandlesClashingKeysInOldTree_Strict() + public void RejectsClashingKeysInOldTree() { // Arrange AddWithKey(oldTree, "key1", "attrib1a"); @@ -328,11 +328,11 @@ namespace Microsoft.AspNetCore.Components.Test // Act/Assert var ex = Assert.Throws(() => 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 HandlesClashingKeysInNewTree_Strict() + public void RejectsClashingKeysInNewTree() { // Arrange AddWithKey(oldTree, "key1", "attrib1a"); @@ -345,100 +345,7 @@ namespace Microsoft.AspNetCore.Components.Test // Act/Assert var ex = Assert.Throws(() => 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); - } - - [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); - }); + Assert.Equal("More than one sibling has the same key value, 'key1'. Key values must be unique.", ex.Message); } [Fact] @@ -2237,19 +2144,10 @@ namespace Microsoft.AspNetCore.Components.Test .Select(x => (T)x.Component) .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"); - - 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); - } + builder.SetKey(key); if (attributeValue != null) { diff --git a/src/Components/test/E2ETest/Tests/KeyTest.cs b/src/Components/test/E2ETest/Tests/KeyTest.cs index 9fbabf5155..0e846b3d5e 100644 --- a/src/Components/test/E2ETest/Tests/KeyTest.cs +++ b/src/Components/test/E2ETest/Tests/KeyTest.cs @@ -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] public async Task CanRetainFocusWhileMovingTextBox() { @@ -291,7 +272,7 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests Browser.Equal(Array.Empty(), 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 rootAfter = new Node(null, "root", after); @@ -302,11 +283,6 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests var textbox = appElem.FindElement(By.TagName("textarea")); var updateButton = appElem.FindElement(By.TagName("button")); - if (useLooseKeys) - { - appElem.FindElement(By.Id("key-loose")).Click(); - } - SetTextAreaValueFast(textbox, jsonBefore); updateButton.Click(); ValidateRenderedOutput(appElem, rootBefore, validatePreservation: false); diff --git a/src/Components/test/testassets/BasicTestApp/KeyCasesComponent.razor b/src/Components/test/testassets/BasicTestApp/KeyCasesComponent.razor index 6e2d887031..582913ae27 100644 --- a/src/Components/test/testassets/BasicTestApp/KeyCasesComponent.razor +++ b/src/Components/test/testassets/BasicTestApp/KeyCasesComponent.razor @@ -4,10 +4,6 @@

Model

-

Output

@@ -73,7 +69,5 @@ // This is so the descendants can detect and display whether they are // newly-instantiated on any given render public int UpdateCount { get; set; } - - public bool UseLooseKeys { get; set; } } } diff --git a/src/Components/test/testassets/BasicTestApp/KeyCasesTreeNode.razor b/src/Components/test/testassets/BasicTestApp/KeyCasesTreeNode.razor index c44d32528a..340e1046cd 100644 --- a/src/Components/test/testassets/BasicTestApp/KeyCasesTreeNode.razor +++ b/src/Components/test/testassets/BasicTestApp/KeyCasesTreeNode.razor @@ -1,4 +1,3 @@ -@using Microsoft.AspNetCore.Components.RenderTree
@Data.Label   [ @@ -17,9 +16,19 @@ @if (Data.Children?.Any() ?? false) { -
- @((RenderFragment)RenderKeyCasesTreeNodes) -
+
@{ + foreach (var child in Data.Children) + { + if (child.Key != null) + { + + } + else + { + + } + } + }
}
@@ -41,31 +50,4 @@ { firstCreatedOnUpdateCount = RenderContext.UpdateCount; } - - void RenderKeyCasesTreeNodes(RenderTreeBuilder builder) - { - // This is equivalent to: - // @foreach (var child in Data.Children) - // { - // if (key != null) - // { - // - // } - // else - // { - // - // } - // } - // TODO: Once the compiler supports @key:loose, eliminate this and just use regular Razor syntax - foreach (var child in Data.Children) - { - builder.OpenComponent(0); - if (child.Key != null) - { - builder.SetKey(child.Key, looseKey: RenderContext.UseLooseKeys); - } - builder.AddAttribute(1, nameof(Data), child); - builder.CloseComponent(); - } - } }