diff --git a/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift b/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift index 1d85dc541..cc4c37e08 100644 --- 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 diff --git a/Sources/TokamakCore/Fiber/FiberReconciler.swift b/Sources/TokamakCore/Fiber/FiberReconciler.swift index 0fc52e5e3..c5b1bc910 100644 --- a/Sources/TokamakCore/Fiber/FiberReconciler.swift +++ b/Sources/TokamakCore/Fiber/FiberReconciler.swift @@ -270,6 +270,14 @@ public final class FiberReconciler { 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 { diff --git a/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift b/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift index c2efdef4b..64f9a16f3 100644 --- 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, caches: FiberReconciler.Caches ) -> Mutation? { - 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.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 } diff --git a/Sources/TokamakDOM/DOMFiberRenderer.swift b/Sources/TokamakDOM/DOMFiberRenderer.swift index 450b4d882..ea57ca416 100644 --- 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 diff --git a/Sources/TokamakTestRenderer/TestFiberRenderer.swift b/Sources/TokamakTestRenderer/TestFiberRenderer.swift index 50cc09e09..d66f541e4 100644 --- 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: "", closingTag: "") } } -public struct TestFiberRenderer: FiberRenderer { +public final class TestFiberRenderer: FiberRenderer { public let sceneSize: CurrentValueSubject public let useDynamicLayout: Bool @@ -159,16 +159,20 @@ public struct TestFiberRenderer: FiberRenderer { view is TestFiberPrimitive } - public func commit(_ mutations: [Mutation]) { + public func commit(_ mutations: [Mutation]) { 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(_ view: V) -> FiberReconciler { + 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() } } } diff --git a/Sources/TokamakTestRenderer/TestViewProxy.swift b/Sources/TokamakTestRenderer/TestViewProxy.swift index 34dd51c73..10843edd2 100644 --- 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 { public subscript(dynamicMember member: KeyPath) -> T? { self.view?[keyPath: member] } + + public func tap() where V == Button { + self.action?() + reconciler.renderer.flush() + } } /// An erased `IdentifiedView`. diff --git a/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift b/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift new file mode 100644 index 000000000..6fb1b6c81 --- /dev/null +++ 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.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.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.self).tap() + + XCTAssertEqual(root.children[0].children.count, 3) // stack content + + reconciler.findView(id: "d.back", as: Button.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.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.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")) + } +} diff --git a/Tests/TokamakReconcilerTests/VisitorTests.swift b/Tests/TokamakReconcilerTests/VisitorTests.swift index f0b3a9748..a6d7e9f36 100644 --- 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.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.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.self ) XCTAssertEqual(stateObjectButton.label, Text("0")) - stateObjectButton.action?() - stateObjectButton.action?() + stateObjectButton.tap() XCTAssertEqual(stateObjectButton.label, Text("5")) XCTAssertEqual(