From d3e81aaf63a85a2b0bbe463d706c37805783f3ad Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Wed, 16 Aug 2023 11:05:11 +0200 Subject: [PATCH 01/12] fiber fixes attempt 2 --- Sources/TokamakCore/Fiber/Fiber.swift | 33 +- .../Fiber/FiberReconciler+TreeReducer.swift | 94 +++--- .../TokamakCore/Fiber/FiberReconciler.swift | 5 +- Sources/TokamakCore/Fiber/Mutation.swift | 5 - .../Fiber/Passes/ReconcilePass.swift | 155 +++++----- Sources/TokamakCore/Fiber/walk.swift | 59 ++-- Sources/TokamakDOM/DOMFiberRenderer.swift | 8 - .../StaticHTMLFiberRenderer.swift | 4 - .../TestFiberRenderer.swift | 62 +++- .../TokamakTestRenderer/TestViewProxy.swift | 9 +- .../VaryingPrimitivenessTests.swift | 287 ++++++++++++++++++ .../TokamakReconcilerTests/VisitorTests.swift | 12 +- 12 files changed, 526 insertions(+), 207 deletions(-) create mode 100644 Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift diff --git a/Sources/TokamakCore/Fiber/Fiber.swift b/Sources/TokamakCore/Fiber/Fiber.swift index ac3ece280..ded06056c 100644 --- a/Sources/TokamakCore/Fiber/Fiber.swift +++ b/Sources/TokamakCore/Fiber/Fiber.swift @@ -63,9 +63,6 @@ public extension FiberReconciler { /// Stored as an IUO because it uses `bindProperties` to create the underlying instance. var layout: AnyLayout? - /// The identity of this `View` - var id: Identity? - /// The mounted element, if this is a `Renderer` primitive. var element: Renderer.ElementType? @@ -135,11 +132,37 @@ public extension FiberReconciler { } } - public enum Identity: Hashable { + enum Identity: Hashable { case explicit(AnyHashable) case structural(index: Int) } + /// The explicit identity of this `View`, if provided + private var explicitId: AnyHashable? { + guard case let .view(v as _AnyIDView, _) = content else { return nil } + return v.anyId + } + + /// Direct children of this fiber, keyed by their explicit or structural identity + var mappedChildren: [Identity: Fiber] { + var map = [Identity: Fiber]() + + var currentIndex = 0 + var currentChild = child + + while let aChild = currentChild { + if let id = aChild.explicitId { + map[.explicit(id)] = aChild + } else { + map[.structural(index: currentIndex)] = aChild + } + currentIndex += 1 + currentChild = aChild.sibling + } + + return map + } + init( _ view: inout V, element: Renderer.ElementType?, @@ -390,7 +413,7 @@ public extension FiberReconciler { layout = (view as? _AnyLayout)?._erased() ?? DefaultLayout.shared } - if Renderer.isPrimitive(view) { + if Renderer.isPrimitive(view) && alternate?.element != nil && alternate?.typeInfo?.type == typeInfo?.type { return .init(from: view, useDynamicLayout: reconciler?.renderer.useDynamicLayout ?? false) } else { return nil diff --git a/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift b/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift index 1d85dc541..7e9189729 100644 --- a/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift +++ b/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift @@ -27,33 +27,35 @@ extension FiberReconciler { unowned var parent: Result? var child: Result? var sibling: Result? - var newContent: Renderer.ElementType.Content? - var elementIndices: [ObjectIdentifier: Int] var nextTraits: _ViewTraitStore // For reducing var lastSibling: Result? - var nextExisting: Fiber? - var nextExistingAlternate: Fiber? + var processedChildCount: Int + var unclaimedCurrentChildren: [Fiber.Identity: Fiber] + + // Side-effects + var didInsert: Bool + var newContent: Renderer.ElementType.Content? init( fiber: Fiber?, + currentChildren: [Fiber.Identity: Fiber], visitChildren: @escaping (TreeReducer.SceneVisitor) -> (), parent: Result?, - child: Fiber?, - alternateChild: Fiber?, newContent: Renderer.ElementType.Content? = nil, - elementIndices: [ObjectIdentifier: Int], nextTraits: _ViewTraitStore ) { self.fiber = fiber self.visitChildren = visitChildren self.parent = parent - nextExisting = child - nextExistingAlternate = alternateChild - self.newContent = newContent - self.elementIndices = elementIndices self.nextTraits = nextTraits + + processedChildCount = 0 + unclaimedCurrentChildren = currentChildren + + didInsert = false + self.newContent = nil } } @@ -130,33 +132,28 @@ extension FiberReconciler { // Create the node and its element. var nextValue = nextValue + let nextValueSlot: Fiber.Identity + if let ident = nextValue as? _AnyIDView { + nextValueSlot = .explicit(ident.anyId) + } else { + nextValueSlot = .structural(index: partialResult.processedChildCount) + } + let resultChild: Result - if let existing = partialResult.nextExisting { - // If a fiber already exists, simply update it with the new view. - let key: ObjectIdentifier? - if let elementParent = existing.elementParent { - key = ObjectIdentifier(elementParent) - } else { - key = nil - } - let newContent = update( - existing, - &nextValue, - key.map { partialResult.elementIndices[$0, default: 0] }, - partialResult.nextTraits - ) + if let existing = partialResult.unclaimedCurrentChildren[nextValueSlot], + existing.typeInfo?.type == typeInfo(of: T.self)?.type + { + partialResult.unclaimedCurrentChildren.removeValue(forKey: nextValueSlot) + let traits = ((nextValue as? any View).map { Renderer.isPrimitive($0) } ?? false) ? .init() : partialResult.nextTraits + let c = update(existing, &nextValue, nil, traits) resultChild = Result( fiber: existing, + currentChildren: existing.mappedChildren, visitChildren: visitChildren(partialResult.fiber?.reconciler, nextValue), parent: partialResult, - child: existing.child, - alternateChild: existing.alternate?.child, - newContent: newContent, - elementIndices: partialResult.elementIndices, - nextTraits: existing.element != nil ? .init() : partialResult.nextTraits + nextTraits: traits ) - partialResult.nextExisting = existing.sibling - partialResult.nextExistingAlternate = partialResult.nextExistingAlternate?.sibling + resultChild.newContent = c } else { let elementParent = partialResult.fiber?.element != nil ? partialResult.fiber @@ -164,39 +161,36 @@ extension FiberReconciler { let preferenceParent = partialResult.fiber?.preferences != nil ? partialResult.fiber : partialResult.fiber?.preferenceParent - let key: ObjectIdentifier? - if let elementParent = elementParent { - key = ObjectIdentifier(elementParent) - } else { - key = nil - } - // Otherwise, create a new fiber for this child. let fiber = createFiber( &nextValue, - partialResult.nextExistingAlternate?.element, + nil, partialResult.fiber, elementParent, preferenceParent, - key.map { partialResult.elementIndices[$0, default: 0] }, + nil, partialResult.nextTraits, partialResult.fiber?.reconciler ) - - // If a fiber already exists for an alternate, link them. - if let alternate = partialResult.nextExistingAlternate { - fiber.alternate = alternate - partialResult.nextExistingAlternate = alternate.sibling + let traits: _ViewTraitStore + if let view = nextValue as? any View, Renderer.isPrimitive(view) { + traits = .init() + } else { + traits = partialResult.nextTraits } + resultChild = Result( fiber: fiber, + currentChildren: [:], visitChildren: visitChildren(partialResult.fiber?.reconciler, nextValue), parent: partialResult, - child: nil, - alternateChild: fiber.alternate?.child, - elementIndices: partialResult.elementIndices, - nextTraits: fiber.element != nil ? .init() : partialResult.nextTraits + nextTraits: traits ) + + resultChild.didInsert = true } + + partialResult.processedChildCount += 1 + // Get the last child element we've processed, and add the new child as its sibling. if let lastSibling = partialResult.lastSibling { lastSibling.fiber?.sibling = resultChild.fiber diff --git a/Sources/TokamakCore/Fiber/FiberReconciler.swift b/Sources/TokamakCore/Fiber/FiberReconciler.swift index 0fc52e5e3..2d50821e7 100644 --- a/Sources/TokamakCore/Fiber/FiberReconciler.swift +++ b/Sources/TokamakCore/Fiber/FiberReconciler.swift @@ -195,11 +195,10 @@ public final class FiberReconciler { } let rootResult = TreeReducer.Result( fiber: alternateRoot, // The alternate is the WIP node. + currentChildren: root.mappedChildren, visitChildren: visitChildren, parent: nil, - child: alternateRoot?.child, - alternateChild: root.child, - elementIndices: [:], + newContent: nil, nextTraits: .init() ) reconciler.caches.clear() diff --git a/Sources/TokamakCore/Fiber/Mutation.swift b/Sources/TokamakCore/Fiber/Mutation.swift index e7206cf3b..f39f6b91f 100644 --- a/Sources/TokamakCore/Fiber/Mutation.swift +++ b/Sources/TokamakCore/Fiber/Mutation.swift @@ -24,11 +24,6 @@ public enum Mutation { index: Int ) case remove(element: Renderer.ElementType, parent: Renderer.ElementType?) - case replace( - parent: Renderer.ElementType, - previous: Renderer.ElementType, - replacement: Renderer.ElementType - ) case update( previous: Renderer.ElementType, newContent: Renderer.ElementType.Content, diff --git a/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift b/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift index c2efdef4b..f843c5508 100644 --- a/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift +++ b/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift @@ -17,6 +17,25 @@ import Foundation +private extension FiberReconciler.Fiber { + /// calls `onAppear` handler if present on self + func callOnAppear() { + if case let .view(action as AppearanceActionType, _) = content { + action.appear?() + } + } + + /// calls any `onDisappear` handlers on self or children + func callOnDisappearRecursive() { + _ = walk(self) { fiber -> WalkWorkResult in + if case let .view(action as AppearanceActionType, _) = fiber.content { + action.disappear?() + } + return .stepIn + } + } +} + /// Walk the current tree, recomputing at each step to check for discrepancies. /// /// Parent-first depth-first traversal. @@ -71,6 +90,23 @@ struct ReconcilePass: FiberReconcilerPass { // Enabled when we reach the `reconcileRoot`. var shouldReconcile = false + func mutationsForRemoving(_ fiber: FiberReconciler.Fiber) -> [Mutation] { + var elementChildren: [FiberReconciler.Fiber] = [] + _ = walk(fiber) { child -> WalkWorkResult<()> in + if child.element != nil { + elementChildren.append(child) + // we removed this element, so any children are removed with it + // and we don't need to go deeper + return .stepOver + } else { + return .stepIn + } + } + return elementChildren.map { + Mutation.remove(element: $0.element!, parent: $0.elementParent!.element!) + } + } + while true { if !shouldReconcile { if let fiber = node.fiber, @@ -92,15 +128,36 @@ struct ReconcilePass: FiberReconcilerPass { node.fiber?.elementIndex = caches.elementIndex(for: elementParent, increment: true) } - // Perform work on the node. - if shouldReconcile, - let mutation = reconcile(node, in: reconciler, caches: caches) - { - caches.mutations.append(mutation) + if let fiber = node.fiber, shouldReconcile || true { + invalidateCache(for: fiber, in: reconciler, caches: caches) + + if node.didInsert { + fiber.callOnAppear() + } + + if let element = fiber.element, + let parent = fiber.elementParent?.element, + let index = fiber.elementIndex, + node.didInsert + { + caches.mutations.append(.insert(element: element, parent: parent, index: index)) + } + + if let element = fiber.element, let c = node.newContent { + caches.mutations.append( + .update( + previous: element, + newContent: c, + geometry: fiber.geometry ?? .init( + origin: .init(origin: .zero), + dimensions: .init(size: .zero, alignmentGuides: [:]), + proposal: .unspecified + ) + ) + ) + } } - // Ensure the `TreeReducer` can access any necessary state. - node.elementIndices = caches.elementIndices // Pass view traits down to the nearest element fiber. if let traits = node.fiber?.outputs.traits, !traits.values.isEmpty @@ -114,6 +171,11 @@ struct ReconcilePass: FiberReconcilerPass { let reducer = FiberReconciler.TreeReducer.SceneVisitor(initialResult: node) node.visitChildren(reducer) + for orphan in reducer.result.unclaimedCurrentChildren.values { + orphan.callOnDisappearRecursive() + caches.mutations.insert(contentsOf: mutationsForRemoving(orphan), at: 0) + } + node.fiber?.preferences?.reset() if reconciler.renderer.useDynamicLayout, @@ -134,11 +196,6 @@ struct ReconcilePass: FiberReconcilerPass { } } - // Setup the alternate if it doesn't exist yet. - if node.fiber?.alternate == nil { - _ = node.fiber?.createAndBindAlternate?() - } - // Walk down all the way into the deepest child. if let child = reducer.result.child { node = child @@ -148,21 +205,16 @@ 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) - } - return true + var nextChildOrSibling: FiberReconciler.Fiber? = alternateChild + while let child = nextChildOrSibling { + child.callOnDisappearRecursive() + caches.mutations.insert(contentsOf: mutationsForRemoving(child), at: 0) + nextChildOrSibling = child.sibling } } if reducer.result.child == nil { // Make sure we clear the child if there was none node.fiber?.child = nil - node.fiber?.alternate?.child = nil } // If we've made it back to the root, then exit. @@ -189,18 +241,13 @@ 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) - } - alternateSibling = alternateSibling?.sibling + currentAltSibling.callOnDisappearRecursive() + caches.mutations.insert(contentsOf: mutationsForRemoving(currentAltSibling), at: 0) + alternateSibling = currentAltSibling.sibling } guard let parent = node.parent else { return } // When we walk back to the root, exit @@ -217,50 +264,6 @@ struct ReconcilePass: FiberReconcilerPass { } } - /// Compare `node` with its alternate, and add any mutations to the list. - func reconcile( - _ node: FiberReconciler.TreeReducer.Result, - 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 - ) - ) - } - } - return nil - } - /// Remove cached size values if something changed. func invalidateCache( for fiber: FiberReconciler.Fiber, diff --git a/Sources/TokamakCore/Fiber/walk.swift b/Sources/TokamakCore/Fiber/walk.swift index c26060fd6..e0daf9a5d 100644 --- a/Sources/TokamakCore/Fiber/walk.swift +++ b/Sources/TokamakCore/Fiber/walk.swift @@ -17,28 +17,18 @@ @_spi(TokamakCore) public enum WalkWorkResult { - case `continue` + /// continue by recursing into children, if any + case stepIn + /// continue over to next sibling + case stepOver + /// stop iterating by returning result case `break`(with: Success) - case pause } @_spi(TokamakCore) public enum WalkResult { case success(Success) case finished - case paused(at: FiberReconciler.Fiber) -} - -/// Walk a fiber tree from `root` until the `work` predicate returns `false`. -@_spi(TokamakCore) -@discardableResult -public func walk( - _ root: FiberReconciler.Fiber, - _ work: @escaping (FiberReconciler.Fiber) throws -> Bool -) rethrows -> WalkResult { - try walk(root) { - try work($0) ? .continue : .pause - } } /// Parent-first depth-first traversal of a `Fiber` tree. @@ -60,27 +50,28 @@ public func walk( while true { // Perform work on the node switch try work(current) { - case .continue: break case let .break(success): return .success(success) - case .pause: return .paused(at: current) - } - // Walk into the child - if let child = current.child { - current = child - continue - } - // When we walk back to the root, exit - if current === root { - return .finished - } - // Walk back up until we find a sibling - while current.sibling == nil { + case .stepIn: + // Walk into the child + if let child = current.child { + current = child + continue + } + fallthrough + case .stepOver: // When we walk back to the root, exit - guard let parent = current.parent, - parent !== root else { return .finished } - current = parent + if current === root { + return .finished + } + // Walk back up until we find a sibling + while current.sibling == nil { + // When we walk back to the root, exit + guard let parent = current.parent, + parent !== root else { return .finished } + current = parent + } + // Walk the sibling + current = current.sibling! } - // Walk the sibling - current = current.sibling! } } diff --git a/Sources/TokamakDOM/DOMFiberRenderer.swift b/Sources/TokamakDOM/DOMFiberRenderer.swift index ccd7095c5..15eec247e 100644 --- a/Sources/TokamakDOM/DOMFiberRenderer.swift +++ b/Sources/TokamakDOM/DOMFiberRenderer.swift @@ -303,14 +303,6 @@ public struct DOMFiberRenderer: FiberRenderer { } case let .remove(element, _): _ = element.reference?.remove?() - case let .replace(parent, previous, replacement): - guard let parentElement = parent.reference ?? rootElement.reference - else { fatalError("The root element was not bound (trying to replace element).") } - guard let previousElement = previous.reference else { - fatalError("The previous element does not exist (trying to replace element).") - } - let replacementElement = createElement(replacement) - _ = parentElement.replaceChild?(replacementElement, previousElement) case let .update(previous, newContent, geometry): previous.update(with: newContent) guard let previousElement = previous.reference diff --git a/Sources/TokamakStaticHTML/StaticHTMLFiberRenderer.swift b/Sources/TokamakStaticHTML/StaticHTMLFiberRenderer.swift index fa50df5a8..410d2634b 100644 --- a/Sources/TokamakStaticHTML/StaticHTMLFiberRenderer.swift +++ b/Sources/TokamakStaticHTML/StaticHTMLFiberRenderer.swift @@ -201,10 +201,6 @@ public struct StaticHTMLFiberRenderer: FiberRenderer { parent.content.children.insert(element, at: index) case let .remove(element, parent): parent?.content.children.removeAll(where: { $0 === element }) - case let .replace(parent, previous, replacement): - guard let index = parent.content.children.firstIndex(where: { $0 === previous }) - else { continue } - parent.content.children[index] = replacement case let .update(previous, newContent, _): previous.update(with: newContent) case let .layout(element, data): diff --git a/Sources/TokamakTestRenderer/TestFiberRenderer.swift b/Sources/TokamakTestRenderer/TestFiberRenderer.swift index 50cc09e09..e3b959ed7 100644 --- a/Sources/TokamakTestRenderer/TestFiberRenderer.swift +++ b/Sources/TokamakTestRenderer/TestFiberRenderer.swift @@ -106,11 +106,23 @@ public final class TestFiberElement: FiberElement, CustomStringConvertible { } public var description: String { - """ - \(content.renderedValue) - \(children.map { " \($0.description)" }.joined(separator: "\n")) - \(content.closingTag) - """ + let memoryAddress = String(format: "%010p", unsafeBitCast(self, to: Int.self)) + return content.renderedValue + " (\(memoryAddress)) [\(children.count)]" + } + + public var recursiveDescription: String { + var d = description + if !children.isEmpty { + d.append("\n") + d.append( + children + .flatMap { $0.recursiveDescription.components(separatedBy:"\n").map { " \($0)"} } + .joined(separator: "\n") + ) + d.append("\n") + } + d.append(content.closingTag) + return d } public init(renderedValue: String, closingTag: String) { @@ -125,7 +137,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 @@ -156,29 +168,51 @@ public struct TestFiberRenderer: FiberRenderer { } public static func isPrimitive(_ view: V) -> Bool where V: View { - view is TestFiberPrimitive + !(view is AnyOptional) && view is TestFiberPrimitive } - public func commit(_ mutations: [Mutation]) { + public func commit(_ mutations: [Mutation]) { + func isReachable(_ el: TestFiberElement, from parent: TestFiberElement = rootElement) -> Bool { + el === parent || parent.children.contains(where: { isReachable(el, from: $0) }) + } + func assertReachable(_ el: TestFiberElement) { if !isReachable(el) { fatalError("element not reachable from root") }} for mutation in mutations { switch mutation { case let .insert(element, parent, index): + assertReachable(parent) parent.children.insert(element, at: index) case let .remove(element, parent): - parent?.children.removeAll(where: { $0 === element }) - case let .replace(parent, previous, replacement): - guard let index = parent.children.firstIndex(where: { $0 === previous }) - else { continue } - parent.children[index] = replacement + guard let parent else { + fatalError("remove called without parent") + } + assertReachable(parent) + 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 .layout(element, geometry): element.geometry = geometry case let .update(previous, newContent, _): + assertReachable(previous) previous.update(with: newContent) } } } + 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..90cf340ab 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`. @@ -46,7 +46,7 @@ public struct TestViewProxy { !(view is AnyOptional), (view as? IdentifiedViewProtocol)?.id == AnyHashable(id), let child = fiber.child - else { return WalkWorkResult.continue } + else { return WalkWorkResult.stepIn } return WalkWorkResult.break(with: child) } guard case let .success(fiber) = result else { return nil } @@ -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..020b21ac7 --- /dev/null +++ b/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift @@ -0,0 +1,287 @@ +// 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")) + } + + func testCorrectParent() { + final class Router: ObservableObject where R: RawRepresentable, R.RawValue == String { + private static func currentRoute() -> R { + return R(rawValue: "")! + } + + @Published var route: R = Router.currentRoute() + } + + enum Routes: RawRepresentable { + init?(rawValue: String) { + if rawValue == "" { self = .list; return } + let comps = rawValue.components(separatedBy: "/") + if comps.count == 1 { self = .package(comps[0]); return } + if comps.count == 2 { self = .project(comps[0], comps[1]); return } + return nil + } + + var rawValue: String { + switch self { + case .list: return "" + case .package(let id): return id + case .project(let package, let proj): return "\(package)/\(proj)" + } + } + + case list + case package(String) + case project(String, String) + } + + struct IdString: Identifiable, ExpressibleByStringLiteral { + let s: String + + init(stringLiteral value: StringLiteralType) { + s = value + } + + var id: String { s } + } + + struct ListView: View { + let router: Router + + var body: some View { + VStack { + Text("packages:") + let content: [IdString] = ["a", "b", "c"] + ForEach(content) { p in + Button(p.s) { + router.route = .package(p.s) + }.identified(by: p.s) + } + } + } + } + + struct PackageView: View { + let router: Router + let packageId: String + + var body: some View { + VStack { + let content: [IdString] = ["1", "2"] + Text(packageId) + .font(.headline) + Text(packageId) + ForEach(content) { p in + Button(p.s) { + router.route = .project(packageId, p.s) + }.identified(by: p.s) + } + } + } + } + + struct ContentView: View { + @ObservedObject var r = Router() + + var body: some View { + switch r.route { + case .list: ListView(router: r) + case .package(let package): VStack { PackageView(router: r, packageId: package) } + case .project(_, let project): VStack { Text("\(project)") } + } + } + } + + let reconciler = TestFiberRenderer(.root, size: .zero).render(ContentView()) + let root = reconciler.renderer.rootElement + + reconciler.findView(id: "a").tap() + XCTAssert(root.children.count == 1) + reconciler.findView(id: "1").tap() + XCTAssert(root.children.count == 1) + } + + func testPrimitivenessMovingLevel() { + struct Vary: View { + struct C: Identifiable { let id: String } + @State var s: Bool + var body: some View { + if s { + Text("hello") + } else { + ForEach([C(id: "abc")]) { + Text($0.id) + } + } + Text("lol").onAppear { + s = false + } + } + } + struct ContentView: View { + var body: some View { + VStack { + Vary(s: true) + } + } + } + let reconciler = TestFiberRenderer(.root, size: .zero).render(ContentView()) + let root = reconciler.renderer.rootElement + + XCTAssert(root.children[0].description.contains("VStack")) + XCTAssert(root.children[0].children.count == 2) + XCTAssert(!root.children[0].children[0].description.contains("abc")) + XCTAssert(root.children[0].children[0].description.contains("hello")) + + reconciler.renderer.flush() + + XCTAssert(root.children[0].description.contains("VStack")) + XCTAssert(root.children[0].children.count == 2) + XCTAssert(!root.children[0].children[0].description.contains("hello")) + XCTAssert(root.children[0].children[0].description.contains("abc")) + } + + func testInitialPrimitiveInConditional() { + struct V: View { + var body: some View { + if true { + Text("b") + } + } + } + + let reconciler = TestFiberRenderer(.root, size: .zero).render(V()) + let root = reconciler.renderer.rootElement + + + print(root.recursiveDescription) + XCTAssert(root.children.count == 1) + XCTAssert(root.children[0].children.count == 0) + } +} diff --git a/Tests/TokamakReconcilerTests/VisitorTests.swift b/Tests/TokamakReconcilerTests/VisitorTests.swift index f0b3a9748..d3bc6304d 100644 --- a/Tests/TokamakReconcilerTests/VisitorTests.swift +++ b/Tests/TokamakReconcilerTests/VisitorTests.swift @@ -28,6 +28,7 @@ final class VisitorTests: XCTestCase { private var count = 0 var body: some View { + let _ = print("in counter body, count is \(count)") VStack { Text("\(count)") .identified(by: "count") @@ -64,14 +65,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 +100,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 +196,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 +211,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( From 02a1e2b3ade16971babcfde2f51ad1f0041a0c2f Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Fri, 1 Sep 2023 04:44:34 +0200 Subject: [PATCH 02/12] remove errant log statement --- Tests/TokamakReconcilerTests/VisitorTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/TokamakReconcilerTests/VisitorTests.swift b/Tests/TokamakReconcilerTests/VisitorTests.swift index d3bc6304d..a6d7e9f36 100644 --- a/Tests/TokamakReconcilerTests/VisitorTests.swift +++ b/Tests/TokamakReconcilerTests/VisitorTests.swift @@ -28,7 +28,6 @@ final class VisitorTests: XCTestCase { private var count = 0 var body: some View { - let _ = print("in counter body, count is \(count)") VStack { Text("\(count)") .identified(by: "count") From 2c7c295a1be85f4387f088af56d7edf231b08b25 Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sat, 2 Sep 2023 03:35:50 +0200 Subject: [PATCH 03/12] add test for and fix effects of .id(_:) --- Sources/TokamakCore/Fiber/Fiber.swift | 17 +++--- .../Fiber/FiberReconciler+TreeReducer.swift | 20 +++---- .../TokamakReconcilerTests/IdViewTests.swift | 55 +++++++++++++++++++ 3 files changed, 71 insertions(+), 21 deletions(-) create mode 100644 Tests/TokamakReconcilerTests/IdViewTests.swift diff --git a/Sources/TokamakCore/Fiber/Fiber.swift b/Sources/TokamakCore/Fiber/Fiber.swift index ded06056c..770daf624 100644 --- a/Sources/TokamakCore/Fiber/Fiber.swift +++ b/Sources/TokamakCore/Fiber/Fiber.swift @@ -138,24 +138,20 @@ public extension FiberReconciler { } /// The explicit identity of this `View`, if provided - private var explicitId: AnyHashable? { + var explicitId: AnyHashable? { guard case let .view(v as _AnyIDView, _) = content else { return nil } return v.anyId } /// Direct children of this fiber, keyed by their explicit or structural identity - var mappedChildren: [Identity: Fiber] { - var map = [Identity: Fiber]() + var mappedChildren: [Int: Fiber] { + var map = [Int: Fiber]() var currentIndex = 0 var currentChild = child while let aChild = currentChild { - if let id = aChild.explicitId { - map[.explicit(id)] = aChild - } else { - map[.structural(index: currentIndex)] = aChild - } + map[currentIndex] = aChild currentIndex += 1 currentChild = aChild.sibling } @@ -413,8 +409,9 @@ public extension FiberReconciler { layout = (view as? _AnyLayout)?._erased() ?? DefaultLayout.shared } - if Renderer.isPrimitive(view) && alternate?.element != nil && alternate?.typeInfo?.type == typeInfo?.type { - return .init(from: view, useDynamicLayout: reconciler?.renderer.useDynamicLayout ?? false) + if Renderer.isPrimitive(view), let element { + let newContent = Renderer.ElementType.Content(from: view, useDynamicLayout: reconciler?.renderer.useDynamicLayout ?? false) + return (element.content != newContent) ? newContent : nil } else { return nil } diff --git a/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift b/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift index 7e9189729..28362326b 100644 --- a/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift +++ b/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift @@ -32,7 +32,7 @@ extension FiberReconciler { // For reducing var lastSibling: Result? var processedChildCount: Int - var unclaimedCurrentChildren: [Fiber.Identity: Fiber] + var unclaimedCurrentChildren: [Int: Fiber] // Side-effects var didInsert: Bool @@ -40,7 +40,7 @@ extension FiberReconciler { init( fiber: Fiber?, - currentChildren: [Fiber.Identity: Fiber], + currentChildren: [Int: Fiber], visitChildren: @escaping (TreeReducer.SceneVisitor) -> (), parent: Result?, newContent: Renderer.ElementType.Content? = nil, @@ -132,18 +132,16 @@ extension FiberReconciler { // Create the node and its element. var nextValue = nextValue - let nextValueSlot: Fiber.Identity - if let ident = nextValue as? _AnyIDView { - nextValueSlot = .explicit(ident.anyId) - } else { - nextValueSlot = .structural(index: partialResult.processedChildCount) - } + let explicitId = (nextValue as? _AnyIDView)?.anyId + let childIndex = partialResult.processedChildCount let resultChild: Result - if let existing = partialResult.unclaimedCurrentChildren[nextValueSlot], - existing.typeInfo?.type == typeInfo(of: T.self)?.type + if let existing = partialResult.unclaimedCurrentChildren[childIndex], + existing.typeInfo?.type == typeInfo(of: T.self)?.type, + existing.explicitId == explicitId { - partialResult.unclaimedCurrentChildren.removeValue(forKey: nextValueSlot) + partialResult.unclaimedCurrentChildren.removeValue(forKey: childIndex) + existing.sibling = nil let traits = ((nextValue as? any View).map { Renderer.isPrimitive($0) } ?? false) ? .init() : partialResult.nextTraits let c = update(existing, &nextValue, nil, traits) resultChild = Result( diff --git a/Tests/TokamakReconcilerTests/IdViewTests.swift b/Tests/TokamakReconcilerTests/IdViewTests.swift new file mode 100644 index 000000000..d679e8760 --- /dev/null +++ b/Tests/TokamakReconcilerTests/IdViewTests.swift @@ -0,0 +1,55 @@ +// Copyright 2023 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 09/02/23. +// + +import XCTest + +@_spi(TokamakCore) @testable import TokamakCore +import TokamakTestRenderer + +final class IdViewTests: XCTestCase { + func testResettingStateViaId() { + struct Nested: View { + @State var count = 0 + var body: some View { + Text("count \(count)") + Button("inc") { count += 1 }.identified(by: "inc") + } + } + + struct Main: View { + @State var id = 0 + + var body: some View { + Nested().id(id) + Button("reset") { id += 1 }.identified(by: "reset") + } + } + + let reconciler = TestFiberRenderer(.root, size: .zero).render(Main()) + let root = reconciler.renderer.rootElement + + XCTAssert(root.children.count == 3) + XCTAssert(root.children[0].description.contains(#""count 0""#)) + + reconciler.findView(id: "inc").tap() + XCTAssert(root.children[0].description.contains(#""count 1""#)) + + reconciler.findView(id: "reset").tap() + + XCTAssert(root.children[0].description.contains(#""count 0""#)) + } +} From 3f24a4c71a0964e06195d03e3c8694c7974c3f35 Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sat, 2 Sep 2023 06:52:19 +0200 Subject: [PATCH 04/12] add test for appearance actions --- .../AppearanceActionTests.swift | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 Tests/TokamakReconcilerTests/AppearanceActionTests.swift diff --git a/Tests/TokamakReconcilerTests/AppearanceActionTests.swift b/Tests/TokamakReconcilerTests/AppearanceActionTests.swift new file mode 100644 index 000000000..2e915cf9a --- /dev/null +++ b/Tests/TokamakReconcilerTests/AppearanceActionTests.swift @@ -0,0 +1,106 @@ +// Copyright 2023 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 09/02/23. +// + +import XCTest + +@_spi(TokamakCore) @testable import TokamakCore +import TokamakTestRenderer + +final class AppearanceActionTests: XCTestCase { + static var appearACalled = 0 + static var appearBCalled = 0 + static var disappearACalled = 0 + static var disappearBCalled = 0 + + override func setUp() { + Self.appearACalled = 0 + Self.appearBCalled = 0 + Self.disappearACalled = 0 + Self.disappearBCalled = 0 + } + + func testAppearDisappearInCondition() { + struct Test: View { + @State var show = false + var body: some View { + Text("a") + .onAppear { AppearanceActionTests.appearACalled += 1 } + .onDisappear { AppearanceActionTests.disappearACalled += 1 } + if show { + Text("b") + .onAppear { AppearanceActionTests.appearBCalled += 1 } + .onDisappear { AppearanceActionTests.disappearBCalled += 1 } + } + Button("show") { show.toggle() }.identified(by: "show") + } + } + + let reconciler = TestFiberRenderer(.root, size: .zero).render(Test()) + + XCTAssert(Self.appearACalled == 1) + XCTAssert(Self.appearBCalled == 0) + XCTAssert(Self.disappearACalled == 0) + XCTAssert(Self.disappearBCalled == 0) + + reconciler.findView(id: "show").tap() + + XCTAssert(Self.appearACalled == 1) + XCTAssert(Self.appearBCalled == 1) + XCTAssert(Self.disappearACalled == 0) + XCTAssert(Self.disappearBCalled == 0) + + reconciler.findView(id: "show").tap() + + XCTAssert(Self.appearACalled == 1) + XCTAssert(Self.appearBCalled == 1) + XCTAssert(Self.disappearACalled == 0) + XCTAssert(Self.disappearBCalled == 1) + + reconciler.findView(id: "show").tap() + + XCTAssert(Self.appearACalled == 1) + XCTAssert(Self.appearBCalled == 2) + XCTAssert(Self.disappearACalled == 0) + XCTAssert(Self.disappearBCalled == 1) + } + + func testChangingIdCallsActions() { + struct Root: View { + @State var reverse: Bool = true + var body: some View { + let items = reverse ? ["b", "a", "c"] : ["a", "b", "c"] + ForEach(0..<3) { i in + Text("\(items[i])") + .onAppear { AppearanceActionTests.appearACalled += 1 } + .onDisappear { AppearanceActionTests.disappearACalled += 1 } + .id(items[i]) + } + Button("flip") { reverse.toggle() }.identified(by: "flip") + } + } + + let reconciler = TestFiberRenderer(.root, size: .zero).render(Root()) + + XCTAssert(Self.appearACalled == 3) + XCTAssert(Self.disappearACalled == 0) + + reconciler.findView(id: "flip").tap() + + XCTAssert(Self.appearACalled == 5) + XCTAssert(Self.disappearACalled == 2) + } +} From 75854ffdcde1343e6ecd46308baca4f497bf9d51 Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sat, 2 Sep 2023 06:56:00 +0200 Subject: [PATCH 05/12] remove shouldReconcile --- .../Fiber/Passes/ReconcilePass.swift | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift b/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift index f843c5508..d411b4afd 100644 --- a/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift +++ b/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift @@ -87,9 +87,6 @@ struct ReconcilePass: FiberReconcilerPass { ) where R: FiberRenderer { var node = root - // Enabled when we reach the `reconcileRoot`. - var shouldReconcile = false - func mutationsForRemoving(_ fiber: FiberReconciler.Fiber) -> [Mutation] { var elementChildren: [FiberReconciler.Fiber] = [] _ = walk(fiber) { child -> WalkWorkResult<()> in @@ -108,18 +105,6 @@ struct ReconcilePass: FiberReconcilerPass { } while true { - if !shouldReconcile { - if let fiber = node.fiber, - changedFibers.contains(ObjectIdentifier(fiber)) - { - shouldReconcile = true - } else if let alternate = node.fiber?.alternate, - changedFibers.contains(ObjectIdentifier(alternate)) - { - shouldReconcile = true - } - } - // If this fiber has an element, set its `elementIndex` // and increment the `elementIndices` value for its `elementParent`. if node.fiber?.element != nil, @@ -128,7 +113,7 @@ struct ReconcilePass: FiberReconcilerPass { node.fiber?.elementIndex = caches.elementIndex(for: elementParent, increment: true) } - if let fiber = node.fiber, shouldReconcile || true { + if let fiber = node.fiber { invalidateCache(for: fiber, in: reconciler, caches: caches) if node.didInsert { From 69452640ac84e3cabe014658af98db1962afa6e7 Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sat, 2 Sep 2023 07:00:17 +0200 Subject: [PATCH 06/12] remove unused Identity enum --- Sources/TokamakCore/Fiber/Fiber.swift | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Sources/TokamakCore/Fiber/Fiber.swift b/Sources/TokamakCore/Fiber/Fiber.swift index 770daf624..bf9eefa52 100644 --- a/Sources/TokamakCore/Fiber/Fiber.swift +++ b/Sources/TokamakCore/Fiber/Fiber.swift @@ -132,11 +132,6 @@ public extension FiberReconciler { } } - enum Identity: Hashable { - case explicit(AnyHashable) - case structural(index: Int) - } - /// The explicit identity of this `View`, if provided var explicitId: AnyHashable? { guard case let .view(v as _AnyIDView, _) = content else { return nil } From 4a2be04f8c3c22d81489e8622fdf5bbe1e5b59c6 Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sat, 2 Sep 2023 07:00:21 +0200 Subject: [PATCH 07/12] fix comment --- Sources/TokamakCore/Fiber/Fiber.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/TokamakCore/Fiber/Fiber.swift b/Sources/TokamakCore/Fiber/Fiber.swift index bf9eefa52..1ffe77af9 100644 --- a/Sources/TokamakCore/Fiber/Fiber.swift +++ b/Sources/TokamakCore/Fiber/Fiber.swift @@ -138,7 +138,7 @@ public extension FiberReconciler { return v.anyId } - /// Direct children of this fiber, keyed by their explicit or structural identity + /// Direct children of this fiber, keyed by their index var mappedChildren: [Int: Fiber] { var map = [Int: Fiber]() From b4bbccdbc2744e207ebfbcd30f6ae879bc631f77 Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sat, 2 Sep 2023 07:22:35 +0200 Subject: [PATCH 08/12] remove unnecessary code from testCorrectParent --- .../VaryingPrimitivenessTests.swift | 29 ++++--------------- 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift b/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift index 020b21ac7..5a4db873d 100644 --- a/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift +++ b/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift @@ -131,31 +131,12 @@ final class VaryingPrimitivenessTests: XCTestCase { } func testCorrectParent() { - final class Router: ObservableObject where R: RawRepresentable, R.RawValue == String { - private static func currentRoute() -> R { - return R(rawValue: "")! - } - - @Published var route: R = Router.currentRoute() + final class Router: ObservableObject { + init(initial r: R) { route = r } + @Published var route: R } - enum Routes: RawRepresentable { - init?(rawValue: String) { - if rawValue == "" { self = .list; return } - let comps = rawValue.components(separatedBy: "/") - if comps.count == 1 { self = .package(comps[0]); return } - if comps.count == 2 { self = .project(comps[0], comps[1]); return } - return nil - } - - var rawValue: String { - switch self { - case .list: return "" - case .package(let id): return id - case .project(let package, let proj): return "\(package)/\(proj)" - } - } - + enum Routes { case list case package(String) case project(String, String) @@ -207,7 +188,7 @@ final class VaryingPrimitivenessTests: XCTestCase { } struct ContentView: View { - @ObservedObject var r = Router() + @ObservedObject var r = Router(initial: .list) var body: some View { switch r.route { From 93096cd737515517bd6c79662f87832023d92d9d Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sun, 3 Sep 2023 06:57:19 +0200 Subject: [PATCH 09/12] remove dead code in ReconcilePass --- .../Fiber/Passes/ReconcilePass.swift | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift b/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift index d411b4afd..3c1ac6fac 100644 --- a/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift +++ b/Sources/TokamakCore/Fiber/Passes/ReconcilePass.swift @@ -185,18 +185,8 @@ struct ReconcilePass: FiberReconcilerPass { if let child = reducer.result.child { node = child continue - } else if let alternateChild = node.fiber?.alternate?.child { - // The alternate has a child that no longer exists. - if let parent = node.fiber?.element != nil ? node.fiber : node.fiber?.elementParent { - invalidateCache(for: parent, in: reconciler, caches: caches) - } - var nextChildOrSibling: FiberReconciler.Fiber? = alternateChild - while let child = nextChildOrSibling { - child.callOnDisappearRecursive() - caches.mutations.insert(contentsOf: mutationsForRemoving(child), at: 0) - nextChildOrSibling = child.sibling - } } + if reducer.result.child == nil { // Make sure we clear the child if there was none node.fiber?.child = nil @@ -224,16 +214,6 @@ struct ReconcilePass: FiberReconcilerPass { } } - var alternateSibling = node.fiber?.alternate?.sibling - // The alternate had siblings that no longer exist. - while let currentAltSibling = alternateSibling { - if let fiber = currentAltSibling.elementParent { - invalidateCache(for: fiber, in: reconciler, caches: caches) - } - currentAltSibling.callOnDisappearRecursive() - caches.mutations.insert(contentsOf: mutationsForRemoving(currentAltSibling), at: 0) - alternateSibling = currentAltSibling.sibling - } guard let parent = node.parent else { return } // When we walk back to the root, exit guard parent !== root.fiber?.alternate else { return } From 406993ad95292f0b7c4a4ee18427eca76916751b Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sun, 3 Sep 2023 08:57:11 +0200 Subject: [PATCH 10/12] fix swift 5.6 errors --- Sources/TokamakCore/Fiber/Fiber.swift | 2 +- .../Fiber/FiberReconciler+TreeReducer.swift | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/Sources/TokamakCore/Fiber/Fiber.swift b/Sources/TokamakCore/Fiber/Fiber.swift index 1ffe77af9..8057bc994 100644 --- a/Sources/TokamakCore/Fiber/Fiber.swift +++ b/Sources/TokamakCore/Fiber/Fiber.swift @@ -404,7 +404,7 @@ public extension FiberReconciler { layout = (view as? _AnyLayout)?._erased() ?? DefaultLayout.shared } - if Renderer.isPrimitive(view), let element { + if Renderer.isPrimitive(view), let element = element { let newContent = Renderer.ElementType.Content(from: view, useDynamicLayout: reconciler?.renderer.useDynamicLayout ?? false) return (element.content != newContent) ? newContent : nil } else { diff --git a/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift b/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift index 28362326b..966a11eef 100644 --- a/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift +++ b/Sources/TokamakCore/Fiber/FiberReconciler+TreeReducer.swift @@ -77,7 +77,8 @@ extension FiberReconciler { update: { fiber, scene, _, _ in fiber.update(with: &scene) }, - visitChildren: { $1._visitChildren } + visitChildren: { $1._visitChildren }, + isPrimitive: { _ in false } ) } @@ -109,6 +110,9 @@ extension FiberReconciler { }, visitChildren: { reconciler, view in reconciler?.renderer.viewVisitor(for: view) ?? view._visitChildren + }, + isPrimitive: { view in + Renderer.isPrimitive(view) } ) } @@ -127,7 +131,8 @@ extension FiberReconciler { FiberReconciler? ) -> Fiber, update: (Fiber, inout T, Int?, _ViewTraitStore) -> Renderer.ElementType.Content?, - visitChildren: (FiberReconciler?, T) -> (TreeReducer.SceneVisitor) -> () + visitChildren: (FiberReconciler?, T) -> (TreeReducer.SceneVisitor) -> (), + isPrimitive: (T) -> Bool ) { // Create the node and its element. var nextValue = nextValue @@ -142,7 +147,7 @@ extension FiberReconciler { { partialResult.unclaimedCurrentChildren.removeValue(forKey: childIndex) existing.sibling = nil - let traits = ((nextValue as? any View).map { Renderer.isPrimitive($0) } ?? false) ? .init() : partialResult.nextTraits + let traits = isPrimitive(nextValue) ? .init() : partialResult.nextTraits let c = update(existing, &nextValue, nil, traits) resultChild = Result( fiber: existing, @@ -170,7 +175,7 @@ extension FiberReconciler { partialResult.fiber?.reconciler ) let traits: _ViewTraitStore - if let view = nextValue as? any View, Renderer.isPrimitive(view) { + if isPrimitive(nextValue) { traits = .init() } else { traits = partialResult.nextTraits From f1292f838f2f4bb176f3953e1a94c604339452ea Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Sun, 3 Sep 2023 09:01:11 +0200 Subject: [PATCH 11/12] one more swift 5.6 fix --- Sources/TokamakTestRenderer/TestFiberRenderer.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/TokamakTestRenderer/TestFiberRenderer.swift b/Sources/TokamakTestRenderer/TestFiberRenderer.swift index e3b959ed7..6307ff34c 100644 --- a/Sources/TokamakTestRenderer/TestFiberRenderer.swift +++ b/Sources/TokamakTestRenderer/TestFiberRenderer.swift @@ -182,7 +182,7 @@ public final class TestFiberRenderer: FiberRenderer { assertReachable(parent) parent.children.insert(element, at: index) case let .remove(element, parent): - guard let parent else { + guard let parent = parent else { fatalError("remove called without parent") } assertReachable(parent) From f40940344fb9b684c6469d4558e2aec354507579 Mon Sep 17 00:00:00 2001 From: Lukas Stabe Date: Thu, 7 Sep 2023 03:13:41 +0200 Subject: [PATCH 12/12] remove stray log statement --- Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift b/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift index 5a4db873d..79721fbd8 100644 --- a/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift +++ b/Tests/TokamakReconcilerTests/VaryingPrimitivenessTests.swift @@ -260,8 +260,6 @@ final class VaryingPrimitivenessTests: XCTestCase { let reconciler = TestFiberRenderer(.root, size: .zero).render(V()) let root = reconciler.renderer.rootElement - - print(root.recursiveDescription) XCTAssert(root.children.count == 1) XCTAssert(root.children[0].children.count == 0) }