Fiber Reconciler fixes

Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift

@@ -133,6 +133,10 @@ extension FiberReconciler {
       let resultChild: Result
       if let existing = partialResult.nextExisting {
         // If a fiber already exists, simply update it with the new view.
+        let elementParent = partialResult.fiber?.element != nil
+          ? partialResult.fiber
+          : partialResult.fiber?.elementParent
+        existing.elementParent = elementParent
         let key: ObjectIdentifier?
         if let elementParent = existing.elementParent {
           key = ObjectIdentifier(elementParent)
@@ -157,6 +161,7 @@ extension FiberReconciler {
         )
         partialResult.nextExisting = existing.sibling
         partialResult.nextExistingAlternate = partialResult.nextExistingAlternate?.sibling
+        existing.sibling = nil
       } else {
         let elementParent = partialResult.fiber?.element != nil
           ? partialResult.fiber

Sources/TokamakCore/Fiber/FiberReconciler.swift

@@ -270,6 +270,14 @@ public final class FiberReconciler<Renderer: FiberRenderer> {
     self.alternate = current
     current = alternate
 
+    // copy over elements to the alternate, so when an element is
+    // replaced, the old element can still be accessed to emit
+    // removal mutations for its children
+    walk(current) { node in
+      node.alternate?.element = node.element
+      return true
+    }
+
     isReconciling = false
 
     for action in afterReconcileActions {

Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift

@@ -86,7 +86,7 @@ struct ReconcilePass: FiberReconcilerPass {
 
       // If this fiber has an element, set its `elementIndex`
       // and increment the `elementIndices` value for its `elementParent`.
-      if node.fiber?.element != nil,
+      if node.newContent != nil || node.fiber?.element != nil,
          let elementParent = node.fiber?.elementParent
       {
         node.fiber?.elementIndex = caches.elementIndex(for: elementParent, increment: true)
@@ -148,15 +148,19 @@ struct ReconcilePass: FiberReconcilerPass {
         if let parent = node.fiber?.element != nil ? node.fiber : node.fiber?.elementParent {
           invalidateCache(for: parent, in: reconciler, caches: caches)
         }
-        walk(alternateChild) { node in
-          if let element = node.element,
-             let parent = node.elementParent?.element
-          {
-            // Removals must happen in reverse order, so a child element
-            // is removed before its parent.
-            caches.mutations.insert(.remove(element: element, parent: parent), at: 0)
+        var nextChildOrSibling: FiberReconciler.Fiber? = alternateChild
+        while let child = nextChildOrSibling {
+          walk(child) { node in
+            if let element = node.element,
+               let parent = node.elementParent?.element
+            {
+              // Removals must happen in reverse order, so a child element
+              // is removed before its parent.
+              caches.mutations.insert(.remove(element: element, parent: parent), at: 0)
+            }
+            return true
           }
-          return true
+          nextChildOrSibling = child.sibling
         }
       }
       if reducer.result.child == nil {
@@ -189,19 +193,23 @@ struct ReconcilePass: FiberReconcilerPass {
 
         var alternateSibling = node.fiber?.alternate?.sibling
         // The alternate had siblings that no longer exist.
-        while alternateSibling != nil {
-          if let fiber = alternateSibling?.elementParent {
+        while let currentAltSibling = alternateSibling {
+          if let fiber = currentAltSibling.elementParent {
             invalidateCache(for: fiber, in: reconciler, caches: caches)
           }
-          if let element = alternateSibling?.element,
-             let parent = alternateSibling?.elementParent?.element
-          {
-            // Removals happen in reverse order, so a child element is removed before
-            // its parent.
-            caches.mutations.insert(.remove(element: element, parent: parent), at: 0)
+          walk(currentAltSibling) { node in
+            if let element = node.element,
+               let parent = node.elementParent?.element
+            {
+              // Removals happen in reverse order, so a child element is removed before
+              // its parent.
+              caches.mutations.insert(.remove(element: element, parent: parent), at: 0)
+            }
+            return true
           }
-          alternateSibling = alternateSibling?.sibling
+          alternateSibling = currentAltSibling.sibling
         }
+        node.fiber?.alternate?.sibling = nil
         guard let parent = node.parent else { return }
         // When we walk back to the root, exit
         guard parent !== root.fiber?.alternate else { return }
@@ -223,41 +231,73 @@ struct ReconcilePass: FiberReconcilerPass {
     in reconciler: FiberReconciler<R>,
     caches: FiberReconciler<R>.Caches
   ) -> Mutation<R>? {
-    if let element = node.fiber?.element,
-       let index = node.fiber?.elementIndex,
-       let parent = node.fiber?.elementParent?.element
-    {
-      if node.fiber?.alternate == nil { // This didn't exist before (no alternate)
-        if let fiber = node.fiber {
-          invalidateCache(for: fiber, in: reconciler, caches: caches)
-        }
-        return .insert(element: element, parent: parent, index: index)
-      } else if node.fiber?.typeInfo?.type != node.fiber?.alternate?.typeInfo?.type,
-                let previous = node.fiber?.alternate?.element
-      {
-        if let fiber = node.fiber {
-          invalidateCache(for: fiber, in: reconciler, caches: caches)
-        }
-        // This is a completely different type of view.
-        return .replace(parent: parent, previous: previous, replacement: element)
-      } else if let newContent = node.newContent,
-                newContent != element.content
-      {
-        if let fiber = node.fiber {
-          invalidateCache(for: fiber, in: reconciler, caches: caches)
-        }
-        // This is the same type of view, but its backing data has changed.
-        return .update(
-          previous: element,
-          newContent: newContent,
-          geometry: node.fiber?.geometry ?? .init(
-            origin: .init(origin: .zero),
-            dimensions: .init(size: .zero, alignmentGuides: [:]),
-            proposal: .unspecified
-          )
+    guard let fiber = node.fiber else { return nil }
+
+    func canUpdate(_ fiber: FiberReconciler<R>.Fiber) -> Bool {
+      fiber.typeInfo?.type == fiber.alternate?.typeInfo?.type
+    }
+
+    invalidateCache(for: fiber, in: reconciler, caches: caches)
+
+    switch (fiber.element, node.newContent, fiber.alternate?.element) {
+    case let (nil, content?, nil):
+      guard let index = fiber.elementIndex, let parent = fiber.elementParent?.element else { break }
+
+      let el = R.ElementType(from: content)
+      fiber.element = el
+      fiber.alternate?.element = el
+
+      return .insert(element: el, parent: parent, index: index)
+
+    case let (nil, content?, altElement?) where !canUpdate(fiber):
+      guard let parent = fiber.elementParent?.element else { break }
+
+      let el = R.ElementType(from: content)
+      fiber.element = el
+      fiber.alternate?.element = el
+
+      return .replace(parent: parent, previous: altElement, replacement: el)
+
+    case let (nil, content?, element?) where canUpdate(fiber) && content != element.content,
+         let (element?, content?, _) where canUpdate(fiber) && content != element.content:
+      guard fiber.elementParent?.element != nil else { break } // todo: is this needed?
+
+      fiber.element = element
+      return .update(
+        previous: element,
+        newContent: content,
+        geometry: fiber.geometry ?? .init(
+          origin: .init(origin: .zero),
+          dimensions: .init(size: .zero, alignmentGuides: [:]),
+          proposal: .unspecified
         )
-      }
+      )
+
+    case let (_, nil, alt?):
+      guard let parent = fiber.alternate?.elementParent?.element else { break } // todo: name => altParent?
+
+      fiber.element = nil
+      fiber.elementIndex = 0
+      if let p = fiber.elementParent { caches.elementIndices[ObjectIdentifier(p)]? -= 1 }
+      return .remove(element: alt, parent: parent)
+
+    case let (element?, content, nil):
+      guard let parent = fiber.elementParent?.element,
+            let index = fiber.elementIndex
+        else { break }
+
+      if let c = content { element.update(with: c) }
+      return .insert(element: element, parent: parent, index: index)
+
+    case let (element?, _, previous?) where !canUpdate(fiber):
+      guard let parent = fiber.elementParent?.element else { break }
+
+      return .replace(parent: parent, previous: previous, replacement: element)
+
+    default:
+      break
     }
+
     return nil
   }
 

Sources/TokamakDOM/DOMFiberRenderer.swift

@@ -310,7 +310,18 @@ public struct DOMFiberRenderer: FiberRenderer {
           fatalError("The previous element does not exist (trying to replace element).")
         }
         let replacementElement = createElement(replacement)
-        _ = parentElement.replaceChild?(previousElement, replacementElement)
+        var grandchildren: [JSObject] = []
+
+        while let g = previousElement.firstChild.object {
+          grandchildren.append(g)
+          _ = g.remove!()
+        }
+
+         _ = parentElement.replaceChild?(replacementElement, previousElement)
+
+        for g in grandchildren {
+          _ = replacementElement.appendChild!(g)
+        }
       case let .update(previous, newContent, geometry):
         previous.update(with: newContent)
         guard let previousElement = previous.reference

Sources/TokamakTestRenderer/TestFiberRenderer.swift

@@ -125,7 +125,7 @@ public final class TestFiberElement: FiberElement, CustomStringConvertible {
   public static var root: Self { .init(renderedValue: "<root>", closingTag: "</root>") }
 }
 
-public struct TestFiberRenderer: FiberRenderer {
+public final class TestFiberRenderer: FiberRenderer {
   public let sceneSize: CurrentValueSubject<CGSize, Never>
   public let useDynamicLayout: Bool
 
@@ -159,16 +159,20 @@ public struct TestFiberRenderer: FiberRenderer {
     view is TestFiberPrimitive
   }
 
-  public func commit(_ mutations: [Mutation<Self>]) {
+  public func commit(_ mutations: [Mutation<TestFiberRenderer>]) {
     for mutation in mutations {
       switch mutation {
       case let .insert(element, parent, index):
         parent.children.insert(element, at: index)
       case let .remove(element, parent):
-        parent?.children.removeAll(where: { $0 === element })
+        guard let idx = parent?.children.firstIndex(where: { $0 === element })
+          else { fatalError("remove called with element that doesn't belong to its parent") }
+        parent?.children.remove(at: idx)
       case let .replace(parent, previous, replacement):
         guard let index = parent.children.firstIndex(where: { $0 === previous })
         else { continue }
+        let grandchildren = parent.children[index].children
+        replacement.children = grandchildren
         parent.children[index] = replacement
       case let .layout(element, geometry):
         element.geometry = geometry
@@ -178,7 +182,21 @@ public struct TestFiberRenderer: FiberRenderer {
     }
   }
 
+  public func render<V: View>(_ view: V) -> FiberReconciler<TestFiberRenderer> {
+    let ret = FiberReconciler(self, view)
+    flush()
+    return ret
+  }
+
+  var scheduledActions: [() -> Void] = []
+
   public func schedule(_ action: @escaping () -> ()) {
-    action()
+    scheduledActions.append(action)
+  }
+
+  public func flush() {
+    let actions = scheduledActions
+    scheduledActions = []
+    for a in actions { a() }
   }
 }

Sources/TokamakTestRenderer/TestViewProxy.swift

@@ -17,7 +17,7 @@
 
 import Foundation
 
-@_spi(TokamakCore)
+@_spi(TokamakCore) @testable
 import TokamakCore
 
 /// A proxy for an identified view in the `TestFiberRenderer`.
@@ -63,6 +63,11 @@ public struct TestViewProxy<V: View> {
   public subscript<T>(dynamicMember member: KeyPath<V, T>) -> T? {
     self.view?[keyPath: member]
   }
+
+  public func tap() where V == Button<Text> {
+    self.action?()
+    reconciler.renderer.flush()
+  }
 }
 
 /// An erased `IdentifiedView`.