Allow lifecycle callbacks to safety trigger re-renders (square#425)

* Allow lifecycle callbacks to safety trigger re-renders

* Update BlueprintUI/Sources/BlueprintView/BlueprintView.swift

Co-authored-by: Robert MacEachern <[email protected]>

* Linter fix

* Allow app extension API in test target

---------

Co-authored-by: Robert MacEachern <[email protected]>

BlueprintUI.podspec

@@ -27,5 +27,8 @@ Pod::Spec.new do |s|
     test_spec.library = 'swiftsimd'
     test_spec.source_files = 'BlueprintUI/Tests/**/*.swift'
     test_spec.framework = 'XCTest'
+    test_spec.pod_target_xcconfig = {
+      'APPLICATION_EXTENSION_API_ONLY' => 'NO',
+    }
   end
 end

BlueprintUI/Sources/BlueprintView/BlueprintView.swift

@@ -410,17 +410,21 @@ public final class BlueprintView: UIView {
             )
         )
 
+        hasUpdatedViewHierarchy = true
+        isInsideUpdate = false
+
+        /// We intentionally deliver our lifecycle callbacks (eg, `onAppear`,
+        /// `onDisappear`, etc, _after_ we've marked our view as updated.
+        /// This is in case the `onAppear` callback triggers a re-render,
+        /// we don't hit our recurisve update precondition.
+
         for callback in updateResult.lifecycleCallbacks {
             callback()
         }
 
         Logger.logViewUpdateEnd(view: self)
         let viewUpdateEndTime = CACurrentMediaTime()
 
-        hasUpdatedViewHierarchy = true
-
-        isInsideUpdate = false
-
         metricsDelegate?.blueprintView(
             self,
             completedRenderWith: .init(

BlueprintUICommonControls/Sources/Internal/PassthroughView.swift → BlueprintUI/Sources/Internal/PassthroughView.swift

@@ -1,16 +1,16 @@
 import UIKit
 
 /// View which does not personally receive touches but allows its subviews to receive touches.
-final class PassthroughView: UIView {
+@_spi(BlueprintPassthroughView) public final class PassthroughView: UIView {
     /// This is an optimization to prevent unnecessary drawing of this view,
     /// since `CATransformLayer` doesn't draw its own contents, only child layers.
-    override class var layerClass: AnyClass {
+    public override class var layerClass: AnyClass {
         CATransformLayer.self
     }
 
     /// Ignore any touches on this view and (pass through) by returning nil if the
     /// default `hitTest` implementation returns this view.
-    override func hitTest(_ point: CGPoint, with event: UIEvent?) -> UIView? {
+    public override func hitTest(_ point: CGPoint, with event: UIEvent?) -> UIView? {
         let result = super.hitTest(point, with: event)
         return result == self ? nil : result
     }

BlueprintUICommonControls/Sources/LifecycleObserver.swift → BlueprintUI/Sources/Layout/LifecycleObserver.swift

@@ -1,4 +1,3 @@
-import BlueprintUI
 
 /// Allows element lifecycle callbacks to be inserted anywhere into the element tree.
 ///

BlueprintUI/Tests/BlueprintViewTests.swift

@@ -205,7 +205,7 @@ class BlueprintViewTests: XCTestCase {
                         // make sure UIKit knows we want a chance for layout
                         view.setNeedsLayout()
                     }
-                    config[\.onLayoutSubviews] = self.onLayoutSubviews
+                    config[\.onLayoutSubviews] = onLayoutSubviews
                 }
             }
 
@@ -692,6 +692,36 @@ class BlueprintViewTests: XCTestCase {
         XCTAssertEqual(measureCount, 3)
     }
 
+    func test_lifecycleCallbacks_dont_cause_crash() {
+
+        let expectation = expectation(description: "Re-rendered")
+
+        withHostedView { view in
+
+            var hasRerendered = false
+
+            func render() {
+                view.element = SimpleViewElement(color: .black).onAppear {
+
+                    /// Simulate an onAppear event triggering a re-render.
+
+                    if hasRerendered == false {
+                        hasRerendered = true
+                        render()
+
+                        expectation.fulfill()
+                    }
+                }
+
+                view.layoutIfNeeded()
+            }
+
+            render()
+        }
+
+        waitForExpectations(timeout: 1)
+    }
+
     func test_metrics_delegate_completedRenderWith() {
         let testMetricsDelegate = TestMetricsDelegate()
 
@@ -725,7 +755,7 @@ fileprivate struct MeasurableElement: Element {
 
     var content: ElementContent {
         ElementContent { constraint -> CGSize in
-            self.validate(constraint)
+            validate(constraint)
         }
     }
 

BlueprintUI/Tests/XCTestCase+AppHost.swift

@@ -0,0 +1,94 @@
+import BlueprintUI
+import UIKit
+import XCTest
+
+
+extension XCTestCase {
+
+    ///
+    /// Call this method to show a view controller in the test host application
+    /// during a unit test. The view controller will be the size of host application's device.
+    ///
+    /// After the test runs, the view controller will be removed from the view hierarchy.
+    ///
+    /// A test failure will occur if the host application does not exist, or does not have a root view controller.
+    ///
+    public func show<ViewController: UIViewController>(
+        vc viewController: ViewController,
+        loadAndPlaceView: Bool = true,
+        test: (ViewController) throws -> Void
+    ) rethrows {
+
+        var temporaryWindow: UIWindow? = nil
+
+        func rootViewController() -> UIViewController {
+            if let rootVC = UIApplication.shared.delegate?.window??.rootViewController {
+                return rootVC
+            } else {
+                let window = UIWindow(frame: UIScreen.main.bounds)
+                let rootVC = UIViewController()
+                window.rootViewController = rootVC
+                window.makeKeyAndVisible()
+
+                temporaryWindow = window
+
+                return rootVC
+            }
+        }
+
+        let rootVC = rootViewController()
+
+        rootVC.addChild(viewController)
+        viewController.didMove(toParent: rootVC)
+
+        if loadAndPlaceView {
+            viewController.view.frame = rootVC.view.bounds
+            viewController.view.layoutIfNeeded()
+
+            rootVC.beginAppearanceTransition(true, animated: false)
+            rootVC.view.addSubview(viewController.view)
+            rootVC.endAppearanceTransition()
+        }
+
+        defer {
+            if loadAndPlaceView {
+                viewController.beginAppearanceTransition(false, animated: false)
+                viewController.view.removeFromSuperview()
+                viewController.endAppearanceTransition()
+            }
+
+            viewController.willMove(toParent: nil)
+            viewController.removeFromParent()
+
+            if let window = temporaryWindow {
+                window.resignKey()
+                window.isHidden = true
+
+                window.rootViewController = nil
+            }
+        }
+
+        try autoreleasepool {
+            try test(viewController)
+        }
+    }
+
+    /// Runs the given block with a `BlueprintView` that is hosted in in a view controller in the
+    /// app host's window. You can use this to test elements that require some UIKit interaction,
+    /// like focus.
+    public func withHostedView(test: (BlueprintView) -> Void) {
+        final class TestViewController: UIViewController {
+            let blueprintView = BlueprintView()
+
+            override func loadView() {
+                view = blueprintView
+            }
+        }
+
+        let viewController = TestViewController()
+
+        show(vc: viewController) { _ in
+            test(viewController.blueprintView)
+        }
+    }
+}

BlueprintUICommonControls/Sources/TransitionContainer.swift

@@ -1,4 +1,4 @@
-import BlueprintUI
+@_spi(BlueprintPassthroughView) import BlueprintUI
 import UIKit
 
 

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Lifecycle callbacks like `onAppear` and `onDisappear` now occur outside of the layout pass; allowing, eg, `onAppear` to safely trigger a re-render.
+
 ### Deprecated
 
 ### Security

SampleApp/Podfile.lock

@@ -16,7 +16,7 @@ EXTERNAL SOURCES:
     :path: "../BlueprintUICommonControls.podspec"
 
 SPEC CHECKSUMS:
-  BlueprintUI: a1adeb91f76c88dd499ad693251aa96919b1c2a4
+  BlueprintUI: 4f8d092313c4b5c35d2528b3b7655ff669a057a0
   BlueprintUICommonControls: 9e02875bc6b8ef64aa9634c32d7156bd50c7b88d
 
 PODFILE CHECKSUM: c795e247aa1c5eb825186ef8192821306c59c891