Merge e15de26f0f into f1cbfcf073

2023-01-23 19:40:26 +00:00 · 2023-01-23 19:40:26 +00:00 · 9be4ca2928
parent f1cbfcf073 e15de26f0f
commit 9be4ca2928
8 changed files with 282 additions and 64 deletions
--- a/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift
+++ b/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
--- a/Sources/TokamakCore/Fiber/FiberReconciler.swift
+++ b/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 {
--- a/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift
+++ b/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
  }

--- a/Sources/TokamakDOM/DOMFiberRenderer.swift
+++ b/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
--- a/Sources/TokamakTestRenderer/TestFiberRenderer.swift
+++ b/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() }
  }
 }
--- a/Sources/TokamakTestRenderer/TestViewProxy.swift
+++ b/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`.
--- a/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift
+++ b/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift
@ -0,0 +1,132 @@
+// Copyright 2022 Tokamak contributors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+//  Created by Lukas Stabe on 10/30/22.
+//
+
+import XCTest
+
+@_spi(TokamakCore) @testable import TokamakCore
+import TokamakTestRenderer
+
+final class VaryingPrimitivenessTests: XCTestCase {
+  func testVaryingPrimitiveness() {
+    enum State {
+      case a
+      case b([String])
+      case c(String, [Int])
+      case d(String, [Int], String)
+    }
+
+    final class StateManager: ObservableObject {
+      private init() { }
+      static let shared = StateManager()
+
+      @Published var state = State.a //b(["eins", "2", "III"])
+    }
+
+    struct ContentView: View {
+      @ObservedObject var sm = StateManager.shared
+
+      var body: some View {
+        switch sm.state {
+        case .a:
+          Button("go to b") {
+            sm.state = .b(["eins", "zwei", "drei"])
+          }.identified(by: "a.1")
+        case .b(let arr):
+          VStack {
+            Text("b:")
+            ForEach(arr, id: \.self) { s in
+              Button(s) {
+                sm.state = .c(s, s == "zwei" ? [1, 2] : [1])
+              }.identified(by: "b.\(s)")
+            }
+          }
+        case .c(let str, let ints):
+          VStack {
+            Text("c \(str)")
+              .font(.headline)
+            Text("hello there")
+            ForEach(ints, id: \.self) { i in
+              let d = "i = \(i)"
+              Button(d) {
+                sm.state = .d(str, ints, d)
+              }.identified(by: "c." + d)
+            }
+          }
+        case .d(_, let ints, let other):
+          VStack {
+            Text("d \(other)")
+            Text("count \(ints.count)")
+            Button("back") {
+              sm.state = .a
+            }.identified(by: "d.back")
+          }
+        }
+      }
+    }
+
+    let reconciler = TestFiberRenderer(.root, size: .zero).render(ContentView())
+    let root = reconciler.renderer.rootElement
+
+    XCTAssertEqual(root.children.count, 1) // button style
+    XCTAssertEqual(root.children[0].children.count, 1) // text
+
+    reconciler.findView(id: "a.1", as: Button<Text>.self).tap()
+
+    XCTAssertEqual(root.children.count, 1)
+    XCTAssert(root.children[0].description.contains("VStack"))
+    XCTAssertEqual(root.children[0].children.count, 4) // stack content
+    XCTAssert(root.children[0].children[0].description.contains("Text"))
+    XCTAssert(root.children[0].children[1].description.contains("ButtonStyle"))
+
+    reconciler.findView(id: "b.zwei", as: Button<Text>.self).tap()
+
+    XCTAssertEqual(root.children.count, 1)
+    XCTAssert(root.children[0].description.contains("VStack"))
+    XCTAssertEqual(root.children[0].children.count, 4) // stack content
+    XCTAssert(root.children[0].children[0].description.contains("Text"))
+    XCTAssert(root.children[0].children[1].description.contains("Text"))
+    XCTAssert(root.children[0].children[2].description.contains("ButtonStyle"))
+
+    reconciler.findView(id: "c.i = 2", as: Button<Text>.self).tap()
+
+    XCTAssertEqual(root.children[0].children.count, 3) // stack content
+
+    reconciler.findView(id: "d.back", as: Button<Text>.self).tap()
+
+    XCTAssertEqual(root.children.count, 1)
+    XCTAssert(root.children[0].description.contains("ButtonStyle"))
+    XCTAssertEqual(root.children[0].children.count, 1)
+    XCTAssert(root.children[0].children[0].description.contains("Text"))
+
+    reconciler.findView(id: "a.1", as: Button<Text>.self).tap()
+
+    XCTAssertEqual(root.children.count, 1)
+    XCTAssert(root.children[0].description.contains("VStack"))
+    XCTAssertEqual(root.children[0].children.count, 4) // stack content
+    XCTAssert(root.children[0].children[0].description.contains("Text"))
+    XCTAssert(root.children[0].children[1].description.contains("ButtonStyle"))
+
+    reconciler.findView(id: "b.zwei", as: Button<Text>.self).tap()
+
+    XCTAssertEqual(root.children.count, 1)
+    XCTAssert(root.children[0].description.contains("VStack"))
+    XCTAssertEqual(root.children[0].children.count, 4) // stack content
+    XCTAssert(root.children[0].children[0].description.contains("Text"))
+    XCTAssert(root.children[0].children[1].description.contains("Text"))
+    XCTAssert(root.children[0].children[2].description.contains("ButtonStyle"))
+  }
+}
--- a/Tests/TokamakReconcilerTests/VisitorTests.swift
+++ b/Tests/TokamakReconcilerTests/VisitorTests.swift
@ -64,14 +64,14 @@ final class VisitorTests: XCTestCase {
    // Count up to 5
    for i in 0..<5 {
      XCTAssertEqual(countText.view, Text("\(i)"))
-      incrementButton.action?()
+      incrementButton.tap()
    }
    XCTAssertNil(incrementButton.view, "'Increment' should be hidden when count >= 5")
    XCTAssertNotNil(decrementButton.view, "'Decrement' should be visible when count > 0")
    // Count down to 0.
    for i in 0..<5 {
      XCTAssertEqual(countText.view, Text("\(5 - i)"))
-      decrementButton.action?()
+      decrementButton.tap()
    }
    XCTAssertNil(decrementButton.view, "'Decrement' should be hidden when count <= 0")
    XCTAssertNotNil(incrementButton.view, "'Increment' should be visible when count < 5")
@ -99,7 +99,7 @@ final class VisitorTests: XCTestCase {
    let addItemButton = reconciler.findView(id: "addItem", as: Button<Text>.self)
    XCTAssertNotNil(addItemButton)
    for i in 0..<10 {
-      addItemButton.action?()
+      addItemButton.tap()
      XCTAssertEqual(reconciler.findView(id: i).view, Text("Item \(i)"))
    }
  }
@ -195,7 +195,7 @@ final class VisitorTests: XCTestCase {
    // State
    let button = reconciler.findView(id: DynamicPropertyTest.state, as: Button<Text>.self)
    XCTAssertEqual(button.label, Text("0"))
-    button.action?()
+    button.tap()
    XCTAssertEqual(button.label, Text("1"))

    // Environment
@ -210,8 +210,7 @@ final class VisitorTests: XCTestCase {
      as: Button<Text>.self
    )
    XCTAssertEqual(stateObjectButton.label, Text("0"))
-    stateObjectButton.action?()
-    stateObjectButton.action?()
+    stateObjectButton.tap()
    XCTAssertEqual(stateObjectButton.label, Text("5"))

    XCTAssertEqual(