From f04c0b787e83ba8959b591f046ce487e499b5c69 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Tue, 18 Jul 2023 12:47:43 -0700 Subject: [PATCH] Allow lifecycle callbacks to safety trigger re-renders (#425) * Allow lifecycle callbacks to safety trigger re-renders * Update BlueprintUI/Sources/BlueprintView/BlueprintView.swift Co-authored-by: Robert MacEachern * Linter fix * Allow app extension API in test target --------- Co-authored-by: Robert MacEachern --- BlueprintUI.podspec | 3 + .../Sources/BlueprintView/BlueprintView.swift | 12 ++- .../Sources/Internal/PassthroughView.swift | 6 +- .../Sources/Layout}/LifecycleObserver.swift | 1 - BlueprintUI/Tests/BlueprintViewTests.swift | 34 ++++++- BlueprintUI/Tests/XCTestCase+AppHost.swift | 94 +++++++++++++++++++ .../Sources/TransitionContainer.swift | 2 +- CHANGELOG.md | 2 + SampleApp/Podfile.lock | 2 +- 9 files changed, 144 insertions(+), 12 deletions(-) rename {BlueprintUICommonControls => BlueprintUI}/Sources/Internal/PassthroughView.swift (72%) rename {BlueprintUICommonControls/Sources => BlueprintUI/Sources/Layout}/LifecycleObserver.swift (99%) create mode 100644 BlueprintUI/Tests/XCTestCase+AppHost.swift diff --git a/BlueprintUI.podspec b/BlueprintUI.podspec index ae2774375..86732e2bd 100644 --- a/BlueprintUI.podspec +++ b/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 diff --git a/BlueprintUI/Sources/BlueprintView/BlueprintView.swift b/BlueprintUI/Sources/BlueprintView/BlueprintView.swift index 5ac95b97d..785cd9517 100644 --- a/BlueprintUI/Sources/BlueprintView/BlueprintView.swift +++ b/BlueprintUI/Sources/BlueprintView/BlueprintView.swift @@ -410,6 +410,14 @@ 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() } @@ -417,10 +425,6 @@ public final class BlueprintView: UIView { Logger.logViewUpdateEnd(view: self) let viewUpdateEndTime = CACurrentMediaTime() - hasUpdatedViewHierarchy = true - - isInsideUpdate = false - metricsDelegate?.blueprintView( self, completedRenderWith: .init( diff --git a/BlueprintUICommonControls/Sources/Internal/PassthroughView.swift b/BlueprintUI/Sources/Internal/PassthroughView.swift similarity index 72% rename from BlueprintUICommonControls/Sources/Internal/PassthroughView.swift rename to BlueprintUI/Sources/Internal/PassthroughView.swift index 1c95d135c..a791da7e7 100644 --- a/BlueprintUICommonControls/Sources/Internal/PassthroughView.swift +++ b/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 } diff --git a/BlueprintUICommonControls/Sources/LifecycleObserver.swift b/BlueprintUI/Sources/Layout/LifecycleObserver.swift similarity index 99% rename from BlueprintUICommonControls/Sources/LifecycleObserver.swift rename to BlueprintUI/Sources/Layout/LifecycleObserver.swift index 00b5f549f..2c232e5d1 100644 --- a/BlueprintUICommonControls/Sources/LifecycleObserver.swift +++ b/BlueprintUI/Sources/Layout/LifecycleObserver.swift @@ -1,4 +1,3 @@ -import BlueprintUI /// Allows element lifecycle callbacks to be inserted anywhere into the element tree. /// diff --git a/BlueprintUI/Tests/BlueprintViewTests.swift b/BlueprintUI/Tests/BlueprintViewTests.swift index ab0255923..c36af8583 100755 --- a/BlueprintUI/Tests/BlueprintViewTests.swift +++ b/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) } } diff --git a/BlueprintUI/Tests/XCTestCase+AppHost.swift b/BlueprintUI/Tests/XCTestCase+AppHost.swift new file mode 100644 index 000000000..207826baa --- /dev/null +++ b/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( + 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) + } + } +} diff --git a/BlueprintUICommonControls/Sources/TransitionContainer.swift b/BlueprintUICommonControls/Sources/TransitionContainer.swift index 13f59dd69..f71d961f8 100644 --- a/BlueprintUICommonControls/Sources/TransitionContainer.swift +++ b/BlueprintUICommonControls/Sources/TransitionContainer.swift @@ -1,4 +1,4 @@ -import BlueprintUI +@_spi(BlueprintPassthroughView) import BlueprintUI import UIKit diff --git a/CHANGELOG.md b/CHANGELOG.md index 8710175be..2d463a340 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/SampleApp/Podfile.lock b/SampleApp/Podfile.lock index 1dd57ae9d..cdf254bda 100644 --- a/SampleApp/Podfile.lock +++ b/SampleApp/Podfile.lock @@ -16,7 +16,7 @@ EXTERNAL SOURCES: :path: "../BlueprintUICommonControls.podspec" SPEC CHECKSUMS: - BlueprintUI: a1adeb91f76c88dd499ad693251aa96919b1c2a4 + BlueprintUI: 4f8d092313c4b5c35d2528b3b7655ff669a057a0 BlueprintUICommonControls: 9e02875bc6b8ef64aa9634c32d7156bd50c7b88d PODFILE CHECKSUM: c795e247aa1c5eb825186ef8192821306c59c891