Fix ordering issue with nested logical element insertion

src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/LogicalElements.ts

@@ -66,22 +66,14 @@ export function insertLogicalChild(child: Node, parent: LogicalElement, childInd
   }
 
   const newSiblings = getLogicalChildrenArray(parent);
-  const newPhysicalParent = getClosestDomElement(parent);
   if (childIndex < newSiblings.length) {
-    newPhysicalParent.insertBefore(child, newSiblings[childIndex] as any as Node);
+    // Insert
+    const nextSibling = newSiblings[childIndex] as any as Node;
+    nextSibling.parentNode!.insertBefore(child, nextSibling);
     newSiblings.splice(childIndex, 0, childAsLogicalElement);
   } else {
-    if (parent instanceof Comment) {
+    // Append
-      const parentLogicalNextSibling = getLogicalNextSibling(parent);
+    appendDomNode(child, parent);
-      if (parentLogicalNextSibling) {
-        newPhysicalParent.insertBefore(child, parentLogicalNextSibling as any as Node);
-      } else {
-        newPhysicalParent.appendChild(child);
-      }
-    } else {
-      newPhysicalParent.appendChild(child);
-    }
-
     newSiblings.push(childAsLogicalElement);
   }
 
@@ -140,6 +132,27 @@ function getClosestDomElement(logicalElement: LogicalElement) {
   }
 }
 
+function appendDomNode(child: Node, parent: LogicalElement) {
+  // This function only puts 'child' into the DOM in the right place relative to 'parent'
+  // It does not update the logical children array of anything
+  if (parent instanceof Element) {
+    parent.appendChild(child);
+  } else if (parent instanceof Comment) {
+    const parentLogicalNextSibling = getLogicalNextSibling(parent) as any as Node;
+    if (parentLogicalNextSibling) {
+      // Since the parent has a logical next-sibling, its appended child goes right before that
+      parentLogicalNextSibling.parentNode!.insertBefore(child, parentLogicalNextSibling);
+    } else {
+      // Since the parent has no logical next-sibling, keep recursing upwards until we find
+      // a logical ancestor that does have a next-sibling or is a physical element.
+      appendDomNode(child, getLogicalParent(parent)!);
+    }
+  } else {
+    // Should never happen
+    throw new Error(`Cannot append node because the parent is not a valid logical element. Parent: ${parent}`);
+  }
+}
+
 function createSymbolOrFallback(fallback: string): symbol | string {
   return typeof Symbol === 'function' ? Symbol() : fallback;
 }

test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/ComponentRenderingTest.cs

@@ -292,5 +292,12 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
             var svgCircleElement = appElement.FindElement(By.XPath("//*[local-name()='circle' and namespace-uri()='http://www.w3.org/2000/svg']"));
             Assert.NotNull(svgCircleElement);
         }
+
+        [Fact]
+        public void LogicalElementInsertionWorksHierarchically()
+        {
+            var appElement = MountTestComponent<LogicalElementInsertionCases>();
+            Assert.Equal("First Second Third", appElement.Text);
+        }
     }
 }

test/testapps/BasicTestApp/LogicalElementInsertionCases.cshtml

@@ -0,0 +1,10 @@
+<!--
+    The lack of whitespace between the nested components, and the fact that PassThroughContentComponent
+    renders its child content without any surrounding nodes, triggers a difficult scenario for the
+    logical element insertion logic. It has to recurse upwards through the logical elements hierarchy
+    to determine where to append nested component output nodes because there are no other nodes for
+    the output to be inserted before.
+-->
+First
+<PassThroughContentComponent><PassThroughContentComponent>Second</PassThroughContentComponent></PassThroughContentComponent>
+Third

test/testapps/BasicTestApp/PassThroughContentComponent.cshtml

@@ -0,0 +1,8 @@
+@using Microsoft.AspNetCore.Blazor
+@ChildContent
+@functions {
+    // Note: The lack of any whitespace or other output besides @ChildContent is important for
+    // what scenarios this component is used for in E2E tests.
+
+    public RenderFragment ChildContent { get; set; }
+}

test/testapps/BasicTestApp/wwwroot/index.html

@@ -28,6 +28,7 @@
       <option value="BasicTestApp.ExternalContentPackage">External content package</option>
       <option value="BasicTestApp.SvgComponent">SVG</option>
       <option value="BasicTestApp.SvgWithChildComponent">SVG with child component</option>
+      <option value="BasicTestApp.LogicalElementInsertionCases">Logical element insertion cases</option>
       <!--<option value="BasicTestApp.RouterTest.Default">Router</option> Excluded because it requires additional setup to work correctly when loaded manually -->
     </select>
     &nbsp;