From 4d04eb04807dc3176515184abe08c3adcbb04713 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Thu, 22 Aug 2024 16:19:46 -0700 Subject: [PATCH] Custom Transaction Execution (#212) * doesnt work * works * doesn't work This reverts commit f2eb2d5ed59b2260289c4623d82aa9c57aa5e066. * wip * wip * Revert "wip" This reverts commit dff88f51b9c17fe19a4fcf496d2ba5ccbd235bce. * Revert "wip" This reverts commit 361345caca7edd1497f85debb5b159afbb04b2eb. * Revert "doesn't work" This reverts commit 64a9c38bc289f695f9ef28593a540f0109f9a77c. * wip * wip * wip * wip * wip * clean up tests * clean up * fix test --------- Co-authored-by: Brandon Williams --- Package.resolved | 9 ++ Package.swift | 2 + Package@swift-6.0.swift | 2 + .../Internal/AssumeIsolated.swift | 2 +- .../NSObject+Observe.swift} | 35 +---- Sources/SwiftNavigation/Observe.swift | 19 ++- Sources/SwiftNavigation/UITransaction.swift | 28 +++- Sources/UIKitNavigation/UITransaction.swift | 39 ++++- Tests/SwiftNavigationTests/ObserveTests.swift | 24 ++- .../UITransactionTests.swift | 137 ++++++++++++++++++ .../UITransactionTests.swift | 53 +++++++ 11 files changed, 304 insertions(+), 46 deletions(-) rename Sources/{UIKitNavigation => SwiftNavigation}/Internal/AssumeIsolated.swift (94%) rename Sources/{UIKitNavigation/Observe.swift => SwiftNavigation/NSObject+Observe.swift} (81%) create mode 100644 Tests/SwiftNavigationTests/UITransactionTests.swift create mode 100644 Tests/UIKitNavigationTests/UITransactionTests.swift diff --git a/Package.resolved b/Package.resolved index 051560bc3a..6113fe9e77 100644 --- a/Package.resolved +++ b/Package.resolved @@ -9,6 +9,15 @@ "version" : "1.5.4" } }, + { + "identity" : "swift-collections", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-collections", + "state" : { + "revision" : "3d2dc41a01f9e49d84f0a3925fb858bed64f702d", + "version" : "1.1.2" + } + }, { "identity" : "swift-concurrency-extras", "kind" : "remoteSourceControl", diff --git a/Package.swift b/Package.swift index 46c1d7edbe..3494649647 100644 --- a/Package.swift +++ b/Package.swift @@ -25,6 +25,7 @@ let package = Package( ), ], dependencies: [ + .package(url: "https://github.com/apple/swift-collections", from: "1.0.0"), .package(url: "https://github.com/swiftlang/swift-docc-plugin", from: "1.0.0"), .package(url: "https://github.com/pointfreeco/swift-case-paths", from: "1.5.4"), .package(url: "https://github.com/pointfreeco/swift-concurrency-extras", from: "1.1.0"), @@ -39,6 +40,7 @@ let package = Package( .product(name: "CasePaths", package: "swift-case-paths"), .product(name: "CustomDump", package: "swift-custom-dump"), .product(name: "ConcurrencyExtras", package: "swift-concurrency-extras"), + .product(name: "OrderedCollections", package: "swift-collections"), .product(name: "Perception", package: "swift-perception"), ] ), diff --git a/Package@swift-6.0.swift b/Package@swift-6.0.swift index 66578eb92e..fc6b81739d 100644 --- a/Package@swift-6.0.swift +++ b/Package@swift-6.0.swift @@ -25,6 +25,7 @@ let package = Package( ), ], dependencies: [ + .package(url: "https://github.com/apple/swift-collections", from: "1.0.0"), .package(url: "https://github.com/swiftlang/swift-docc-plugin", from: "1.0.0"), .package(url: "https://github.com/pointfreeco/swift-case-paths", from: "1.5.4"), .package(url: "https://github.com/pointfreeco/swift-concurrency-extras", from: "1.1.0"), @@ -39,6 +40,7 @@ let package = Package( .product(name: "CasePaths", package: "swift-case-paths"), .product(name: "CustomDump", package: "swift-custom-dump"), .product(name: "ConcurrencyExtras", package: "swift-concurrency-extras"), + .product(name: "OrderedCollections", package: "swift-collections"), .product(name: "Perception", package: "swift-perception"), ] ), diff --git a/Sources/UIKitNavigation/Internal/AssumeIsolated.swift b/Sources/SwiftNavigation/Internal/AssumeIsolated.swift similarity index 94% rename from Sources/UIKitNavigation/Internal/AssumeIsolated.swift rename to Sources/SwiftNavigation/Internal/AssumeIsolated.swift index fea107e3ae..192b7596a9 100644 --- a/Sources/UIKitNavigation/Internal/AssumeIsolated.swift +++ b/Sources/SwiftNavigation/Internal/AssumeIsolated.swift @@ -2,7 +2,7 @@ import Foundation extension MainActor { // NB: This functionality was not back-deployed in Swift 5.9 - static func _assumeIsolated( + package static func _assumeIsolated( _ operation: @MainActor () throws -> T, file: StaticString = #fileID, line: UInt = #line diff --git a/Sources/UIKitNavigation/Observe.swift b/Sources/SwiftNavigation/NSObject+Observe.swift similarity index 81% rename from Sources/UIKitNavigation/Observe.swift rename to Sources/SwiftNavigation/NSObject+Observe.swift index 9b98406e86..580e9574d3 100644 --- a/Sources/UIKitNavigation/Observe.swift +++ b/Sources/SwiftNavigation/NSObject+Observe.swift @@ -1,6 +1,6 @@ -#if canImport(UIKit) - @_spi(Internals) import SwiftNavigation - import UIKit +#if canImport(ObjectiveC) + import Dispatch + import ObjectiveC @MainActor extension NSObject { @@ -123,34 +123,7 @@ ) -> ObserveToken { let token = SwiftNavigation.observe { transaction in MainActor._assumeIsolated { - withUITransaction(transaction) { - #if os(watchOS) - apply(transaction) - #else - if transaction.uiKit.disablesAnimations { - UIView.performWithoutAnimation { apply(transaction) } - for completion in transaction.uiKit.animationCompletions { - completion(true) - } - } else if let animation = transaction.uiKit.animation { - return animation.perform( - { apply(transaction) }, - completion: transaction.uiKit.animationCompletions.isEmpty - ? nil - : { - for completion in transaction.uiKit.animationCompletions { - completion($0) - } - } - ) - } else { - apply(transaction) - for completion in transaction.uiKit.animationCompletions { - completion(true) - } - } - #endif - } + apply(transaction) } } task: { transaction, work in DispatchQueue.main.async { diff --git a/Sources/SwiftNavigation/Observe.swift b/Sources/SwiftNavigation/Observe.swift index 030a843180..ccd5913e24 100644 --- a/Sources/SwiftNavigation/Observe.swift +++ b/Sources/SwiftNavigation/Observe.swift @@ -102,8 +102,7 @@ private actor ActorProxy { } } -@_spi(Internals) -public func observe( +func observe( _ apply: @escaping @Sendable (_ transaction: UITransaction) -> Void, task: @escaping @Sendable ( _ transaction: UITransaction, _ operation: @escaping @Sendable () -> Void @@ -118,7 +117,21 @@ public func observe( let token, !token.isCancelled else { return } - apply(transaction) + + var perform: @Sendable () -> Void = { apply(transaction) } + for key in transaction.storage.keys { + guard let keyType = key.keyType as? any _UICustomTransactionKey.Type + else { continue } + func open(_: K.Type) { + perform = { [perform] in + K.perform(value: transaction[K.self]) { + perform() + } + } + } + open(keyType) + } + perform() }, task: task ) diff --git a/Sources/SwiftNavigation/UITransaction.swift b/Sources/SwiftNavigation/UITransaction.swift index a910897409..3c81d45c2f 100644 --- a/Sources/SwiftNavigation/UITransaction.swift +++ b/Sources/SwiftNavigation/UITransaction.swift @@ -1,3 +1,5 @@ +import OrderedCollections + /// Executes a closure with the specified transaction and returns the result. /// /// - Parameters: @@ -8,7 +10,10 @@ public func withUITransaction( _ transaction: UITransaction, _ body: () throws -> Result ) rethrows -> Result { - try UITransaction.$current.withValue(transaction, operation: body) + try UITransaction.$current.withValue( + UITransaction.current.merging(transaction), + operation: body + ) } /// Executes a closure with the specified transaction key path and value and returns the result. @@ -24,7 +29,7 @@ public func withUITransaction( _ value: V, _ body: () throws -> R ) rethrows -> R { - var transaction = UITransaction.current + var transaction = UITransaction() transaction[keyPath: keyPath] = value return try withUITransaction(transaction, body) } @@ -36,7 +41,7 @@ public func withUITransaction( public struct UITransaction: Sendable { @TaskLocal package static var current = Self() - private var storage: [Key: any Sendable] = [:] + var storage: OrderedDictionary = [:] /// Creates a transaction. public init() {} @@ -68,7 +73,15 @@ public struct UITransaction: Sendable { storage.isEmpty } - private struct Key: Hashable { + fileprivate func merging(_ other: Self) -> Self { + Self(storage: storage.merging(other.storage, uniquingKeysWith: { $1 })) + } + + private init(storage: OrderedDictionary) { + self.storage = storage + } + + struct Key: Hashable { let keyType: Any.Type init(_ keyType: K.Type) { self.keyType = keyType @@ -92,3 +105,10 @@ public protocol UITransactionKey { /// The default value for the transaction key. static var defaultValue: Value { get } } + +public protocol _UICustomTransactionKey: UITransactionKey, Sendable { + static func perform( + value: Value, + operation: @Sendable () -> Void + ) +} diff --git a/Sources/UIKitNavigation/UITransaction.swift b/Sources/UIKitNavigation/UITransaction.swift index af1bb0957c..c73873e284 100644 --- a/Sources/UIKitNavigation/UITransaction.swift +++ b/Sources/UIKitNavigation/UITransaction.swift @@ -1,4 +1,7 @@ #if canImport(UIKit) && !os(watchOS) + import SwiftNavigation + import UIKit + extension UITransaction { /// Creates a transaction and assigns its animation property. /// @@ -14,8 +17,42 @@ set { self[UIKitKey.self] = newValue } } - private enum UIKitKey: UITransactionKey { + private enum UIKitKey: _UICustomTransactionKey { static let defaultValue = UIKit() + + static func perform( + value: UIKit, + operation: @Sendable () -> Void + ) { + MainActor._assumeIsolated { + #if os(watchOS) + operation() + #else + if value.disablesAnimations { + UIView.performWithoutAnimation { operation() } + for completion in value.animationCompletions { + completion(true) + } + } else if let animation = value.animation { + return animation.perform( + { operation() }, + completion: value.animationCompletions.isEmpty + ? nil + : { + for completion in value.animationCompletions { + completion($0) + } + } + ) + } else { + operation() + for completion in value.animationCompletions { + completion(true) + } + } + #endif + } + } } /// UIKit-specific data associated with a ``UITransaction``. diff --git a/Tests/SwiftNavigationTests/ObserveTests.swift b/Tests/SwiftNavigationTests/ObserveTests.swift index cffc2f911c..d85f7bf249 100644 --- a/Tests/SwiftNavigationTests/ObserveTests.swift +++ b/Tests/SwiftNavigationTests/ObserveTests.swift @@ -1,10 +1,10 @@ -#if swift(>=6) - import SwiftNavigation - import XCTest +import SwiftNavigation +import XCTest - class ObserveTests: XCTestCase { +class ObserveTests: XCTestCase { + #if swift(>=6) @MainActor - func testCompiles() { + func testIsolation() { var count = 0 let token = SwiftNavigation.observe { count = 1 @@ -12,5 +12,17 @@ XCTAssertEqual(count, 1) _ = token } + #endif + + @MainActor + func testTokenStorage() { + var count = 0 + observe { + count += 1 + } + observe { + count += 1 + } + XCTAssertEqual(count, 2) } -#endif +} diff --git a/Tests/SwiftNavigationTests/UITransactionTests.swift b/Tests/SwiftNavigationTests/UITransactionTests.swift new file mode 100644 index 0000000000..3c5ceb5d5b --- /dev/null +++ b/Tests/SwiftNavigationTests/UITransactionTests.swift @@ -0,0 +1,137 @@ +import SwiftNavigation +import XCTest + +class UITransactionTests: XCTestCase { + @MainActor + func testTransactionKeyPropagates() async { + let expectation = expectation(description: "onChange") + expectation.expectedFulfillmentCount = 2 + + let model = Model() + XCTAssertEqual(UITransaction.current.isSet, false) + + observe { + if model.count == 0 { + XCTAssertEqual(UITransaction.current.isSet, false) + } else if model.count == 1 { + XCTAssertEqual(UITransaction.current.isSet, true) + } else { + XCTFail() + } + expectation.fulfill() + } + + withUITransaction(\.isSet, true) { + model.count += 1 + } + await fulfillment(of: [expectation], timeout: 1) + XCTAssertEqual(model.count, 1) + XCTAssertEqual(UITransaction.current.isSet, false) + } + + @MainActor + func testTransactionMerging() async { + observe { transaction in + XCTAssertFalse(transaction.isSet) + XCTAssertFalse(transaction.isAlsoSet) + } + withUITransaction(\.isSet, true) { + observe { transaction in + XCTAssertTrue(transaction.isSet) + XCTAssertFalse(transaction.isAlsoSet) + } + _ = withUITransaction(\.isAlsoSet, true) { + observe { transaction in + XCTAssertTrue(transaction.isSet) + XCTAssertTrue(transaction.isAlsoSet) + } + } + observe { transaction in + XCTAssertTrue(transaction.isSet) + XCTAssertFalse(transaction.isAlsoSet) + } + } + observe { transaction in + XCTAssertFalse(transaction.isSet) + XCTAssertFalse(transaction.isAlsoSet) + } + } + + @MainActor + func testSynchronousTransactionKey() async { + let expectation = expectation(description: "onChange") + + let model = Model() + XCTAssertEqual(UITransaction.current.isSet, false) + + _ = withUITransaction(\.isSet, true) { + observe { + XCTAssertEqual(model.count, 0) + XCTAssertEqual(UITransaction.current.isSet, true) + expectation.fulfill() + } + } + + await fulfillment(of: [expectation], timeout: 1) + XCTAssertEqual(UITransaction.current.isSet, false) + } + + @MainActor + func testOverrideTransactionKey() async { + XCTAssertEqual(UITransaction.current.isSet, false) + withUITransaction(\.isSet, true) { + XCTAssertEqual(UITransaction.current.isSet, true) + withUITransaction(\.isSet, false) { + XCTAssertEqual(UITransaction.current.isSet, false) + } + } + } + + @MainActor + func testBindingTransactionKey() async { + let expectation = expectation(description: "onChange") + expectation.expectedFulfillmentCount = 2 + + @UIBinding var count = 0 + var transaction = UITransaction() + transaction.isSet = true + + observe { + if count == 0 { + XCTAssertEqual(UITransaction.current.isSet, false) + } else if count == 1 { + XCTAssertEqual(UITransaction.current.isSet, true) + } else { + XCTFail() + } + expectation.fulfill() + } + + let bindingWithTransaction = $count.transaction(transaction) + bindingWithTransaction.wrappedValue = 1 + + await fulfillment(of: [expectation], timeout: 1) + } +} + +@Perceptible +private class Model { + var count = 0 +} + +extension UITransaction { + var isSet: Bool { + get { self[IsSetKey.self] } + set { self[IsSetKey.self] = newValue } + } + var isAlsoSet: Bool { + get { self[IsAlsoSetKey.self] } + set { self[IsAlsoSetKey.self] = newValue } + } +} +private enum IsSetKey: UITransactionKey { + static let defaultValue = false +} +private enum IsAlsoSetKey: UITransactionKey { + static let defaultValue = false +} diff --git a/Tests/UIKitNavigationTests/UITransactionTests.swift b/Tests/UIKitNavigationTests/UITransactionTests.swift new file mode 100644 index 0000000000..22e8e98484 --- /dev/null +++ b/Tests/UIKitNavigationTests/UITransactionTests.swift @@ -0,0 +1,53 @@ +#if canImport(UIKit) + import SwiftNavigation + import UIKitNavigation + import XCTest + + class UITransactionTests: XCTestCase { + #if !os(watchOS) + @MainActor + func testTransactionKeyPropagatesWithAnimation() async { + let expectation = expectation(description: "onChange") + expectation.expectedFulfillmentCount = 2 + + let model = Model() + XCTAssertEqual(UITransaction.current.isSet, false) + + observe { + if model.count == 0 { + XCTAssertEqual(UITransaction.current.isSet, false) + } else if model.count == 1 { + XCTAssertEqual(UITransaction.current.isSet, true) + } else { + XCTFail() + } + expectation.fulfill() + } + + withUITransaction(\.isSet, true) { + withUIKitAnimation { + model.count += 1 + } + } + await fulfillment(of: [expectation], timeout: 1) + XCTAssertEqual(model.count, 1) + XCTAssertEqual(UITransaction.current.isSet, false) + } + #endif + } + + @Perceptible + private class Model { + var count = 0 + } + + private enum IsSetKey: UITransactionKey { + static let defaultValue = false + } + extension UITransaction { + var isSet: Bool { + get { self[IsSetKey.self] } + set { self[IsSetKey.self] = newValue } + } + } +#endif