fix 3.0.0 reuse issues (#41)

lkzhao · Feb 5, 2024 · 0375eb4 · 0375eb4
1 parent 232b6a2
commit 0375eb4
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 9 deletions.
diff --git a/Sources/UIComponent/Components/View/ViewComponent.swift b/Sources/UIComponent/Components/View/ViewComponent.swift
@@ -59,7 +59,7 @@ public struct ViewRenderNode<View: UIView>: RenderNode {
     }
     /// The reuse strategy for the view, determining whether it should be reused or automatically managed.
     public var reuseStrategy: ReuseStrategy {
-        view == nil ? .key("\(type(of: self))") : .noReuse
+        view == nil ? .automatic : .noReuse
     }
 
     /// Initializes a `ViewRenderNode` with a specified size, optional view, and optional generator.

diff --git a/Sources/UIComponent/Core/ComponentView/ReuseManager.swift b/Sources/UIComponent/Core/ComponentView/ReuseManager.swift
@@ -6,7 +6,9 @@ import UIKit
 /// - `automatic`: A key generated by the Type information is used to identify the view for reuse.
 /// - `noReuse`: The view should not be reused.
 /// - `key(String)`: A specific key is used to identify the view for reuse.
-public enum ReuseStrategy {
+public enum ReuseStrategy: Equatable {
+    /// A key generated by the Type information is used to identify the view for reuse.
+    case automatic
     /// The view should not be reused.
     case noReuse
     /// A specific key is used to identify the view for reuse.

diff --git a/Sources/UIComponent/Core/Model/RenderNode/RenderNode+Modifiers.swift b/Sources/UIComponent/Core/Model/RenderNode/RenderNode+Modifiers.swift
@@ -10,7 +10,7 @@ public struct UpdateRenderNode<Content: RenderNode>: RenderNodeWrapper {
     public var reuseStrategy: ReuseStrategy {
         // we don't know what the update block did, so we disable
         // reuse so that we don't get inconsistent state
-        .noReuse
+        content.reuseStrategy == .automatic ? .noReuse : content.reuseStrategy
     }
 
     public var shouldRenderView: Bool {
@@ -29,11 +29,6 @@ public struct KeyPathUpdateRenderNode<Value, Content: RenderNode>: RenderNodeWra
     public let valueKeyPath: ReferenceWritableKeyPath<Content.View, Value>
     public let value: Value
 
-    public var reuseStrategy: ReuseStrategy {
-        // not using content's reuseStrategy because our type should be different than the content type
-        .key("\(type(of: self))")
-    }
-
     public func updateView(_ view: Content.View) {
         content.updateView(view)
         view[keyPath: valueKeyPath] = value

diff --git a/Sources/UIComponent/Core/Model/RenderNode/RenderNode.swift b/Sources/UIComponent/Core/Model/RenderNode/RenderNode.swift
@@ -103,7 +103,7 @@ extension RenderNode {
 extension RenderNode {
     public var id: String? { nil }
     public var animator: Animator? { nil }
-    public var reuseStrategy: ReuseStrategy { .key("\(type(of: self))") }
+    public var reuseStrategy: ReuseStrategy { .automatic }
     public var shouldRenderView: Bool { children.isEmpty }
 
     public func makeView() -> View {
@@ -129,6 +129,8 @@ extension RenderNode {
 extension RenderNode {
     internal func _makeView() -> UIView {
         switch reuseStrategy {
+        case .automatic:
+            return ReuseManager.shared.dequeue(identifier: "\(type(of: self))", makeView())
         case .noReuse:
             return makeView()
         case .key(let key):

diff --git a/Tests/UIComponentTests/ReuseTest.swift b/Tests/UIComponentTests/ReuseTest.swift
@@ -110,4 +110,72 @@ final class ReuseTests: XCTestCase {
         // the UILabel should not be reused
         XCTAssertNotEqual(existingLabel, newLabel)
     }
+
+    func testNoReuseWithSameAttributes() {
+        componentView.component = Text("1").reuseStrategy(.noReuse).textColor(.red).id("1")
+        componentView.reloadData()
+        XCTAssertEqual(componentView.renderNode?.reuseStrategy, .noReuse)
+        XCTAssertEqual(componentView.subviews.count, 1)
+        let existingLabel = componentView.subviews.first as? UILabel
+        XCTAssertNotNil(existingLabel)
+        XCTAssertEqual(existingLabel?.text, "1")
+        XCTAssertEqual(existingLabel?.textColor, .red)
+        componentView.component = Text("2").reuseStrategy(.noReuse).textColor(.red).id("2")
+        componentView.reloadData()
+        XCTAssertEqual(componentView.renderNode?.reuseStrategy, .noReuse)
+        XCTAssertEqual(componentView.subviews.count, 1)
+        let newLabel = componentView.subviews.first as? UILabel
+        XCTAssertNotNil(newLabel)
+        XCTAssertEqual(newLabel?.text, "2")
+
+        // the UILabel should not be reused
+        XCTAssertNotEqual(existingLabel, newLabel)
+    }
+
+    func testNoReuseWithViewComponent() {
+        let label1 = UILabel()
+        label1.text = "1"
+        let label2 = UILabel()
+        label2.text = "2"
+        componentView.component = ViewComponent(view: label1).textColor(.red)
+        componentView.reloadData()
+        XCTAssertEqual(componentView.renderNode?.reuseStrategy, .noReuse)
+        XCTAssertEqual(componentView.subviews.count, 1)
+        let existingLabel = componentView.subviews.first as? UILabel
+        XCTAssertNotNil(existingLabel)
+        XCTAssertEqual(existingLabel?.text, "1")
+        XCTAssertEqual(existingLabel?.textColor, .red)
+        XCTAssertEqual(existingLabel, label1)
+        componentView.component = ViewComponent(view: label2).textColor(.red)
+        componentView.reloadData()
+        XCTAssertEqual(componentView.renderNode?.reuseStrategy, .noReuse)
+        XCTAssertEqual(componentView.subviews.count, 1)
+        let newLabel = componentView.subviews.first as? UILabel
+        XCTAssertNotNil(newLabel)
+        XCTAssertEqual(newLabel?.text, "2")
+        XCTAssertEqual(newLabel?.textColor, .red)
+        XCTAssertEqual(newLabel, label2)
+        XCTAssertNotEqual(existingLabel, newLabel)
+    }
+
+    func testStructureIdentity() {
+        componentView.component = Text("1").reuseStrategy(.noReuse).textColor(.red)
+        componentView.reloadData()
+        XCTAssertEqual(componentView.renderNode?.reuseStrategy, .noReuse)
+        XCTAssertEqual(componentView.subviews.count, 1)
+        let existingLabel = componentView.subviews.first as? UILabel
+        XCTAssertNotNil(existingLabel)
+        XCTAssertEqual(existingLabel?.text, "1")
+        XCTAssertEqual(existingLabel?.textColor, .red)
+        componentView.component = Text("2").reuseStrategy(.noReuse).textColor(.red)
+        componentView.reloadData()
+        XCTAssertEqual(componentView.renderNode?.reuseStrategy, .noReuse)
+        XCTAssertEqual(componentView.subviews.count, 1)
+        let newLabel = componentView.subviews.first as? UILabel
+        XCTAssertNotNil(newLabel)
+        XCTAssertEqual(newLabel?.text, "2")
+
+        // Although the UILabels are using no reuse, they have the same structure identity, so they should be the same instance
+        XCTAssertEqual(existingLabel, newLabel)
+    }
 }