From d73a3494bfadfec7ccbadf2f295ff4a1156153ec Mon Sep 17 00:00:00 2001 From: Brandon Williams <135203+mbrandonw@users.noreply.github.com> Date: Wed, 15 Mar 2023 08:25:47 -0700 Subject: [PATCH 1/7] Better type names in WithViewStore.debug. (#1973) --- .../xcshareddata/swiftpm/Package.resolved | 4 ++-- Package.swift | 2 +- Sources/ComposableArchitecture/SwiftUI/WithViewStore.swift | 7 ------- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved index e56aca305172..7eb9dbfbf5f8 100644 --- a/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -113,8 +113,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/pointfreeco/xctest-dynamic-overlay", "state" : { - "revision" : "62041e6016a30f56952f5d7d3f12a3fd7029e1cd", - "version" : "0.8.3" + "revision" : "ab8c9f45843694dd16be4297e6d44c0634fd9913", + "version" : "0.8.4" } } ], diff --git a/Package.swift b/Package.swift index 2673112b8c65..0586d0ea3119 100644 --- a/Package.swift +++ b/Package.swift @@ -26,7 +26,7 @@ let package = Package( .package(url: "https://github.com/pointfreeco/swift-dependencies", from: "0.2.0"), .package(url: "https://github.com/pointfreeco/swift-identified-collections", from: "0.7.0"), .package(url: "https://github.com/pointfreeco/swiftui-navigation", from: "0.7.0"), - .package(url: "https://github.com/pointfreeco/xctest-dynamic-overlay", from: "0.8.3"), + .package(url: "https://github.com/pointfreeco/xctest-dynamic-overlay", from: "0.8.4"), ], targets: [ .target( diff --git a/Sources/ComposableArchitecture/SwiftUI/WithViewStore.swift b/Sources/ComposableArchitecture/SwiftUI/WithViewStore.swift index 2515970c5f5e..0f11d4dc5540 100644 --- a/Sources/ComposableArchitecture/SwiftUI/WithViewStore.swift +++ b/Sources/ComposableArchitecture/SwiftUI/WithViewStore.swift @@ -163,13 +163,6 @@ public struct WithViewStore: View { ?? "(No difference in state detected)" } ?? "(Initial state)\n\(stateDump)" - func typeName(_ type: Any.Type) -> String { - var name = String(reflecting: type) - if let index = name.firstIndex(of: ".") { - name.removeSubrange(...index) - } - return name - } print( """ \(prefix.isEmpty ? "" : "\(prefix): ")\ From b09d6f9db956ea4eb6a4d51a216a58f71aaec901 Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Wed, 15 Mar 2023 11:38:09 -0700 Subject: [PATCH 2/7] Fix library evo --- Package.resolved | 4 ++-- .../Reducer/Reducers/Presentation.swift | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Package.resolved b/Package.resolved index 7eb9dbfbf5f8..a2709131a87d 100644 --- a/Package.resolved +++ b/Package.resolved @@ -104,8 +104,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/pointfreeco/swiftui-navigation", "state" : { - "revision" : "0a0e1b321d70ee6a464ecfe6b0136d9eff77ebfc", - "version" : "0.7.0" + "revision" : "47dd574b900ba5ba679f56ea00d4d282fc7305a6", + "version" : "0.7.1" } }, { diff --git a/Sources/ComposableArchitecture/Reducer/Reducers/Presentation.swift b/Sources/ComposableArchitecture/Reducer/Reducers/Presentation.swift index 655733557c83..0f57e7adce41 100644 --- a/Sources/ComposableArchitecture/Reducer/Reducers/Presentation.swift +++ b/Sources/ComposableArchitecture/Reducer/Reducers/Presentation.swift @@ -331,7 +331,12 @@ struct OnFirstAppearID: Hashable {} public struct _PresentedID: Hashable { @inlinable - public init() {} + public init() { + self.init(internal: ()) + } + + @usableFromInline + init(internal: Void) {} } extension Task where Success == Never, Failure == Never { From da6e07c1ec5d29ef04a1950dcd30eef311352fdc Mon Sep 17 00:00:00 2001 From: Brandon Williams <135203+mbrandonw@users.noreply.github.com> Date: Wed, 15 Mar 2023 12:50:20 -0700 Subject: [PATCH 3/7] Cancellation cleanup (#1977) * Clean up effect cancellation. * wip * wip * wip * wip * wip * wip * wip * cancel id test * nb * benchmark * wip * wip * wip * wip * wip * clean up * wip --- .../xcshareddata/swiftpm/Package.resolved | 4 +- Package.swift | 2 +- .../Effects/Cancellation.swift | 92 ++++++++++------ .../StoreSuite.swift | 101 ++++++++++++++++++ .../main.swift | 1 + .../BindingLocalTests.swift | 2 +- .../BindingTests.swift | 2 +- .../CompatibilityTests.swift | 2 +- .../ComposableArchitectureTests.swift | 2 +- .../DebugTests.swift | 43 ++++++-- .../DependencyKeyWritingReducerTests.swift | 2 +- .../DeprecatedTests.swift | 2 +- .../EffectCancellationTests.swift | 25 +++-- .../EffectDebounceTests.swift | 2 +- .../EffectDeferredTests.swift | 2 +- .../EffectFailureTests.swift | 2 +- .../EffectOperationTests.swift | 2 +- .../EffectRunTests.swift | 2 +- .../EffectTaskTests.swift | 2 +- .../EffectTests.swift | 2 +- .../EffectThrottleTests.swift | 26 +++-- .../ForEachReducerTests.swift | 2 +- .../IfCaseLetReducerTests.swift | 2 +- .../IfLetReducerTests.swift | 2 +- .../Internal/BaseTCATestCase.swift | 10 ++ .../MemoryManagementTests.swift | 2 +- .../ReducerTests.swift | 2 +- .../RuntimeWarningTests.swift | 2 +- .../ScopeTests.swift | 2 +- .../StoreFilterTests.swift | 2 +- .../StoreTests.swift | 2 +- .../TaskCancellationTests.swift | 71 ++++++------ .../TaskResultTests.swift | 2 +- .../TestStoreFailureTests.swift | 2 +- .../TestStoreNonExhaustiveTests.swift | 2 +- .../TestStoreTests.swift | 2 +- .../TimerTests.swift | 9 +- .../ViewStoreTests.swift | 2 +- 38 files changed, 311 insertions(+), 127 deletions(-) create mode 100644 Sources/swift-composable-architecture-benchmark/StoreSuite.swift create mode 100644 Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift diff --git a/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved index 7eb9dbfbf5f8..a2709131a87d 100644 --- a/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -104,8 +104,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/pointfreeco/swiftui-navigation", "state" : { - "revision" : "0a0e1b321d70ee6a464ecfe6b0136d9eff77ebfc", - "version" : "0.7.0" + "revision" : "47dd574b900ba5ba679f56ea00d4d282fc7305a6", + "version" : "0.7.1" } }, { diff --git a/Package.swift b/Package.swift index 0586d0ea3119..df3dd1f8560d 100644 --- a/Package.swift +++ b/Package.swift @@ -25,7 +25,7 @@ let package = Package( .package(url: "https://github.com/pointfreeco/swift-custom-dump", from: "0.9.1"), .package(url: "https://github.com/pointfreeco/swift-dependencies", from: "0.2.0"), .package(url: "https://github.com/pointfreeco/swift-identified-collections", from: "0.7.0"), - .package(url: "https://github.com/pointfreeco/swiftui-navigation", from: "0.7.0"), + .package(url: "https://github.com/pointfreeco/swiftui-navigation", from: "0.7.1"), .package(url: "https://github.com/pointfreeco/xctest-dynamic-overlay", from: "0.8.4"), ], targets: [ diff --git a/Sources/ComposableArchitecture/Effects/Cancellation.swift b/Sources/ComposableArchitecture/Effects/Cancellation.swift index 20d06ea28780..7fea2a53f318 100644 --- a/Sources/ComposableArchitecture/Effects/Cancellation.swift +++ b/Sources/ComposableArchitecture/Effects/Cancellation.swift @@ -45,36 +45,30 @@ extension EffectPublisher { _cancellablesLock.lock() defer { _cancellablesLock.unlock() } - let id = _CancelToken(id: id) if cancelInFlight { - _cancellationCancellables[id]?.forEach { $0.cancel() } + _cancellationCancellables.cancel(id: id) } let cancellationSubject = PassthroughSubject() - var cancellationCancellable: AnyCancellable! - cancellationCancellable = AnyCancellable { + var cancellable: AnyCancellable! + cancellable = AnyCancellable { _cancellablesLock.sync { cancellationSubject.send(()) cancellationSubject.send(completion: .finished) - _cancellationCancellables[id]?.remove(cancellationCancellable) - if _cancellationCancellables[id]?.isEmpty == .some(true) { - _cancellationCancellables[id] = nil - } + _cancellationCancellables.remove(cancellable, at: id) } } return publisher.prefix(untilOutputFrom: cancellationSubject) .handleEvents( receiveSubscription: { _ in - _ = _cancellablesLock.sync { - _cancellationCancellables[id, default: []].insert( - cancellationCancellable - ) + _cancellablesLock.sync { + _cancellationCancellables.insert(cancellable, at: id) } }, - receiveCompletion: { _ in cancellationCancellable.cancel() }, - receiveCancel: cancellationCancellable.cancel + receiveCompletion: { _ in cancellable.cancel() }, + receiveCancel: cancellable.cancel ) } .eraseToAnyPublisher() @@ -113,7 +107,7 @@ extension EffectPublisher { public static func cancel(id: AnyHashable) -> Self { .fireAndForget { _cancellablesLock.sync { - _cancellationCancellables[.init(id: id)]?.forEach { $0.cancel() } + _cancellationCancellables.cancel(id: id) } } } @@ -201,22 +195,18 @@ extension EffectPublisher { cancelInFlight: Bool = false, operation: @Sendable @escaping () async throws -> T ) async rethrows -> T { - let id = _CancelToken(id: id) let (cancellable, task) = _cancellablesLock.sync { () -> (AnyCancellable, Task) in if cancelInFlight { - _cancellationCancellables[id]?.forEach { $0.cancel() } + _cancellationCancellables.cancel(id: id) } let task = Task { try await operation() } let cancellable = AnyCancellable { task.cancel() } - _cancellationCancellables[id, default: []].insert(cancellable) + _cancellationCancellables.insert(cancellable, at: id) return (cancellable, task) } defer { _cancellablesLock.sync { - _cancellationCancellables[id]?.remove(cancellable) - if _cancellationCancellables[id]?.isEmpty == .some(true) { - _cancellationCancellables[id] = nil - } + _cancellationCancellables.remove(cancellable, at: id) } } do { @@ -231,22 +221,18 @@ extension EffectPublisher { cancelInFlight: Bool = false, operation: @Sendable @escaping () async throws -> T ) async rethrows -> T { - let id = _CancelToken(id: id) let (cancellable, task) = _cancellablesLock.sync { () -> (AnyCancellable, Task) in if cancelInFlight { - _cancellationCancellables[id]?.forEach { $0.cancel() } + _cancellationCancellables.cancel(id: id) } let task = Task { try await operation() } let cancellable = AnyCancellable { task.cancel() } - _cancellationCancellables[id, default: []].insert(cancellable) + _cancellationCancellables.insert(cancellable, at: id) return (cancellable, task) } defer { _cancellablesLock.sync { - _cancellationCancellables[id]?.remove(cancellable) - if _cancellationCancellables[id]?.isEmpty == .some(true) { - _cancellationCancellables[id] = nil - } + _cancellationCancellables.remove(cancellable, at: id) } } do { @@ -301,7 +287,9 @@ extension Task where Success == Never, Failure == Never { /// /// - Parameter id: An identifier. public static func cancel(id: ID) { - _cancellablesLock.sync { _cancellationCancellables[.init(id: id)]?.forEach { $0.cancel() } } + _cancellablesLock.sync { + _cancellationCancellables.cancel(id: id) + } } /// Cancel any currently in-flight operation with the given identifier. @@ -315,7 +303,7 @@ extension Task where Success == Never, Failure == Never { } } -@_spi(Internals) public struct _CancelToken: Hashable { +@_spi(Internals) public struct _CancelID: Hashable { let id: AnyHashable let discriminator: ObjectIdentifier @@ -325,8 +313,8 @@ extension Task where Success == Never, Failure == Never { } } -@_spi(Internals) public var _cancellationCancellables: [_CancelToken: Set] = [:] -@_spi(Internals) public let _cancellablesLock = NSRecursiveLock() +@_spi(Internals) public var _cancellationCancellables = CancellablesCollection() +private let _cancellablesLock = NSRecursiveLock() @rethrows private protocol _ErrorMechanism { @@ -346,3 +334,41 @@ extension _ErrorMechanism { } extension Result: _ErrorMechanism {} + +@_spi(Internals) +public class CancellablesCollection { + var storage: [_CancelID: Set] = [:] + + func insert( + _ cancellable: AnyCancellable, + at id: AnyHashable + ) { + let cancelID = _CancelID(id: id) + self.storage[cancelID, default: []].insert(cancellable) + } + + func remove( + _ cancellable: AnyCancellable, + at id: AnyHashable + ) { + let cancelID = _CancelID(id: id) + self.storage[cancelID]?.remove(cancellable) + if self.storage[cancelID]?.isEmpty == true { + self.storage[cancelID] = nil + } + } + + func cancel(id: AnyHashable) { + let cancelID = _CancelID(id: id) + self.storage[cancelID]?.forEach { $0.cancel() } + self.storage[cancelID] = nil + } + + public func exists(at id: AnyHashable) -> Bool { + return self.storage[_CancelID(id: id)] != nil + } + + public var count: Int { + return self.storage.count + } +} diff --git a/Sources/swift-composable-architecture-benchmark/StoreSuite.swift b/Sources/swift-composable-architecture-benchmark/StoreSuite.swift new file mode 100644 index 000000000000..f6f2cab7a080 --- /dev/null +++ b/Sources/swift-composable-architecture-benchmark/StoreSuite.swift @@ -0,0 +1,101 @@ +import Benchmark +import Combine +@_spi(Internals) import ComposableArchitecture +import Foundation + +let storeSuite = BenchmarkSuite(name: "Store") { + var store: StoreOf! + let levels = 5 + + for level in 1...levels { + $0.benchmark("Nested send tap: \(level)") { + _ = store.send(tap(level: level)) + } setUp: { + store = Store( + initialState: state(level: level), + reducer: Feature() + ) + } tearDown: { + precondition(count(of: store.state.value, level: level) == 1) + precondition(_cancellationCancellables.count == 0) + } + } + for level in 1...levels { + $0.benchmark("Nested send none: \(level)") { + _ = store.send(none(level: level)) + } setUp: { + store = Store( + initialState: state(level: level), + reducer: Feature() + ) + } tearDown: { + precondition(count(of: store.state.value, level: level) == 0) + precondition(_cancellationCancellables.count == 0) + } + } +} + +private struct Feature: ReducerProtocol { + struct State { + @Box var child: State? + var count = 0 + } + enum Action { + indirect case child(Action) + case tap + case none + } + var body: some ReducerProtocolOf { + Reduce { state, action in + switch action { + case .child: + return .none + case .tap: + state.count = 1 + return Empty(completeImmediately: true) + .eraseToEffect() + .cancellable(id: UUID()) + case .none: + return .none + } + } + .ifLet(\.child, action: /Action.child) { + Feature() + } + } +} + +@propertyWrapper +private struct Box { + private var value: [Value] + var wrappedValue: Value? { + get { self.value.first } + set { self.value = newValue.map { [$0] } ?? [] } + } + init(wrappedValue: Value?) { + self.value = wrappedValue.map { [$0] } ?? [] + } +} + +private func state(level: Int) -> Feature.State { + Feature.State( + child: level == 0 + ? nil + : state(level: level - 1) + ) +} +private func tap(level: Int) -> Feature.Action { + level == 0 + ? .tap + : Feature.Action.child(tap(level: level - 1)) +} +private func none(level: Int) -> Feature.Action { + level == 0 + ? .none + : Feature.Action.child(none(level: level - 1)) +} +private func count(of state: Feature.State?, level: Int) -> Int? { + level == 0 + ? state?.count + : count(of: state?.child, level: level - 1) +} diff --git a/Sources/swift-composable-architecture-benchmark/main.swift b/Sources/swift-composable-architecture-benchmark/main.swift index 39d9f5942d91..7a5fb7c2fd76 100644 --- a/Sources/swift-composable-architecture-benchmark/main.swift +++ b/Sources/swift-composable-architecture-benchmark/main.swift @@ -6,5 +6,6 @@ Benchmark.main([ dependenciesSuite, effectSuite, storeScopeSuite, + storeSuite, viewStoreSuite, ]) diff --git a/Tests/ComposableArchitectureTests/BindingLocalTests.swift b/Tests/ComposableArchitectureTests/BindingLocalTests.swift index 62a4326e57d6..06f9572b758a 100644 --- a/Tests/ComposableArchitectureTests/BindingLocalTests.swift +++ b/Tests/ComposableArchitectureTests/BindingLocalTests.swift @@ -4,7 +4,7 @@ @testable import ComposableArchitecture @MainActor - final class BindingLocalTests: XCTestCase { + final class BindingLocalTests: BaseTCATestCase { public func testBindingLocalIsActive() { XCTAssertFalse(BindingLocal.isActive) diff --git a/Tests/ComposableArchitectureTests/BindingTests.swift b/Tests/ComposableArchitectureTests/BindingTests.swift index be2ef4cdce37..de4807ac3a97 100644 --- a/Tests/ComposableArchitectureTests/BindingTests.swift +++ b/Tests/ComposableArchitectureTests/BindingTests.swift @@ -2,7 +2,7 @@ import ComposableArchitecture import XCTest @MainActor -final class BindingTests: XCTestCase { +final class BindingTests: BaseTCATestCase { #if swift(>=5.7) func testNestedBindingState() { struct BindingTest: ReducerProtocol { diff --git a/Tests/ComposableArchitectureTests/CompatibilityTests.swift b/Tests/ComposableArchitectureTests/CompatibilityTests.swift index 574e48ac8090..60cd1632df5a 100644 --- a/Tests/ComposableArchitectureTests/CompatibilityTests.swift +++ b/Tests/ComposableArchitectureTests/CompatibilityTests.swift @@ -3,7 +3,7 @@ import ComposableArchitecture import XCTest @MainActor -final class CompatibilityTests: XCTestCase { +final class CompatibilityTests: BaseTCATestCase { var cancellables: Set = [] // Actions can be re-entrantly sent into the store if an action is sent that holds an object diff --git a/Tests/ComposableArchitectureTests/ComposableArchitectureTests.swift b/Tests/ComposableArchitectureTests/ComposableArchitectureTests.swift index e613218e75e9..e9db69682ccf 100644 --- a/Tests/ComposableArchitectureTests/ComposableArchitectureTests.swift +++ b/Tests/ComposableArchitectureTests/ComposableArchitectureTests.swift @@ -4,7 +4,7 @@ import ComposableArchitecture import XCTest @MainActor -final class ComposableArchitectureTests: XCTestCase { +final class ComposableArchitectureTests: BaseTCATestCase { var cancellables: Set = [] func testScheduling() async { diff --git a/Tests/ComposableArchitectureTests/DebugTests.swift b/Tests/ComposableArchitectureTests/DebugTests.swift index 2672d692ab40..1ce978ed33f0 100644 --- a/Tests/ComposableArchitectureTests/DebugTests.swift +++ b/Tests/ComposableArchitectureTests/DebugTests.swift @@ -5,7 +5,7 @@ @testable import ComposableArchitecture - final class DebugTests: XCTestCase { + final class DebugTests: BaseTCATestCase { func testDebugCaseOutput() { enum Action { case action1(Bool, label: String) @@ -50,15 +50,40 @@ let action = BindingAction.set(\State.$width, 50) var dump = "" customDump(action, to: &dump) - XCTAssertEqual( - dump, - #""" - BindingAction.set( - WritableKeyPath>, - 50 + + #if swift(>=5.8) + if #available(macOS 13.3, iOS 16.4, watchOS 9.4, tvOS 16.4, *) { + XCTAssertEqual( + dump, + #""" + BindingAction.set( + \State.$width, + 50 + ) + """# + ) + } else { + XCTAssertEqual( + dump, + #""" + BindingAction.set( + WritableKeyPath>, + 50 + ) + """# + ) + } + #else + XCTAssertEqual( + dump, + #""" + BindingAction.set( + WritableKeyPath>, + 50 + ) + """# ) - """# - ) + #endif } @MainActor diff --git a/Tests/ComposableArchitectureTests/DependencyKeyWritingReducerTests.swift b/Tests/ComposableArchitectureTests/DependencyKeyWritingReducerTests.swift index 8649f13eaf76..debc60cbb3f5 100644 --- a/Tests/ComposableArchitectureTests/DependencyKeyWritingReducerTests.swift +++ b/Tests/ComposableArchitectureTests/DependencyKeyWritingReducerTests.swift @@ -2,7 +2,7 @@ import ComposableArchitecture import XCTest @MainActor -final class DependencyKeyWritingReducerTests: XCTestCase { +final class DependencyKeyWritingReducerTests: BaseTCATestCase { func testWritingFusion() async { let reducer: _DependencyKeyWritingReducer = Feature() .dependency(\.myValue, 42) diff --git a/Tests/ComposableArchitectureTests/DeprecatedTests.swift b/Tests/ComposableArchitectureTests/DeprecatedTests.swift index 0e3c304208cf..5c98323c667d 100644 --- a/Tests/ComposableArchitectureTests/DeprecatedTests.swift +++ b/Tests/ComposableArchitectureTests/DeprecatedTests.swift @@ -2,7 +2,7 @@ import ComposableArchitecture import XCTest @available(*, deprecated) -final class DeprecatedTests: XCTestCase { +final class DeprecatedTests: BaseTCATestCase { func testUncheckedStore() { var expectations: [XCTestExpectation] = [] for n in 1...100 { diff --git a/Tests/ComposableArchitectureTests/EffectCancellationTests.swift b/Tests/ComposableArchitectureTests/EffectCancellationTests.swift index 0f6e8613f535..4bfc32852e40 100644 --- a/Tests/ComposableArchitectureTests/EffectCancellationTests.swift +++ b/Tests/ComposableArchitectureTests/EffectCancellationTests.swift @@ -2,7 +2,7 @@ import Combine @_spi(Internals) import ComposableArchitecture import XCTest -final class EffectCancellationTests: XCTestCase { +final class EffectCancellationTests: BaseTCATestCase { struct CancelID: Hashable {} var cancellables: Set = [] @@ -51,6 +51,7 @@ final class EffectCancellationTests: XCTestCase { subject.send(2) XCTAssertEqual(values, [1, 2]) + defer { Task.cancel(id: CancelID()) } EffectPublisher(subject) .cancellable(id: CancelID(), cancelInFlight: true) .sink { values.append($0) } @@ -116,7 +117,7 @@ final class EffectCancellationTests: XCTestCase { .sink(receiveValue: { _ in }) .store(in: &self.cancellables) - XCTAssertNil(_cancellationCancellables[_CancelToken(id: id)]) + XCTAssertEqual(_cancellationCancellables.exists(at: id), false) } func testCancellablesCleanUp_OnCancel() { @@ -134,7 +135,7 @@ final class EffectCancellationTests: XCTestCase { .sink(receiveValue: { _ in }) .store(in: &self.cancellables) - XCTAssertNil(_cancellationCancellables[_CancelToken(id: id)]) + XCTAssertEqual(_cancellationCancellables.exists(at: id), false) } func testDoubleCancellation() { @@ -226,8 +227,9 @@ final class EffectCancellationTests: XCTestCase { self.wait(for: [expectation], timeout: 999) for id in ids { - XCTAssertNil( - _cancellationCancellables[_CancelToken(id: id)], + XCTAssertEqual( + _cancellationCancellables.exists(at: id), + false, "cancellationCancellables should not contain id \(id)" ) } @@ -250,7 +252,7 @@ final class EffectCancellationTests: XCTestCase { cancellables.removeAll() - XCTAssertNil(_cancellationCancellables[_CancelToken(id: id)]) + XCTAssertEqual(_cancellationCancellables.exists(at: id), false) } func testSharedId() { @@ -341,4 +343,15 @@ final class EffectCancellationTests: XCTestCase { mainQueue.advance(by: 1) XCTAssertEqual(output, [B()]) } + + func testCancelIDHash() { + struct CancelID1: Hashable {} + struct CancelID2: Hashable {} + let id1 = _CancelID(id: CancelID1()) + let id2 = _CancelID(id: CancelID2()) + XCTAssertNotEqual(id1, id2) + // NB: We hash the type of the cancel ID to give more variance in the hash since all empty + // structs in Swift have the same hash value. + XCTAssertNotEqual(id1.hashValue, id2.hashValue) + } } diff --git a/Tests/ComposableArchitectureTests/EffectDebounceTests.swift b/Tests/ComposableArchitectureTests/EffectDebounceTests.swift index 4672643a0797..c4a7e51760be 100644 --- a/Tests/ComposableArchitectureTests/EffectDebounceTests.swift +++ b/Tests/ComposableArchitectureTests/EffectDebounceTests.swift @@ -3,7 +3,7 @@ import ComposableArchitecture import XCTest @MainActor -final class EffectDebounceTests: XCTestCase { +final class EffectDebounceTests: BaseTCATestCase { var cancellables: Set = [] func testDebounce() async { diff --git a/Tests/ComposableArchitectureTests/EffectDeferredTests.swift b/Tests/ComposableArchitectureTests/EffectDeferredTests.swift index 198707f94c50..922ed6ed00ae 100644 --- a/Tests/ComposableArchitectureTests/EffectDeferredTests.swift +++ b/Tests/ComposableArchitectureTests/EffectDeferredTests.swift @@ -2,7 +2,7 @@ import Combine import ComposableArchitecture import XCTest -final class EffectDeferredTests: XCTestCase { +final class EffectDeferredTests: BaseTCATestCase { var cancellables: Set = [] func testDeferred() { diff --git a/Tests/ComposableArchitectureTests/EffectFailureTests.swift b/Tests/ComposableArchitectureTests/EffectFailureTests.swift index f67e9cd417eb..e3044eb6e3d4 100644 --- a/Tests/ComposableArchitectureTests/EffectFailureTests.swift +++ b/Tests/ComposableArchitectureTests/EffectFailureTests.swift @@ -4,7 +4,7 @@ import XCTest @MainActor - final class EffectFailureTests: XCTestCase { + final class EffectFailureTests: BaseTCATestCase { var cancellables: Set = [] func testTaskUnexpectedThrows() async { diff --git a/Tests/ComposableArchitectureTests/EffectOperationTests.swift b/Tests/ComposableArchitectureTests/EffectOperationTests.swift index 06fce69e3d74..a2878d7d4015 100644 --- a/Tests/ComposableArchitectureTests/EffectOperationTests.swift +++ b/Tests/ComposableArchitectureTests/EffectOperationTests.swift @@ -4,7 +4,7 @@ @testable import ComposableArchitecture @MainActor - class EffectOperationTests: XCTestCase { + class EffectOperationTests: BaseTCATestCase { func testMergeDiscardsNones() async { var effect = EffectTask.none .merge(with: .none) diff --git a/Tests/ComposableArchitectureTests/EffectRunTests.swift b/Tests/ComposableArchitectureTests/EffectRunTests.swift index 9bde4a3cec6b..a77504bca074 100644 --- a/Tests/ComposableArchitectureTests/EffectRunTests.swift +++ b/Tests/ComposableArchitectureTests/EffectRunTests.swift @@ -3,7 +3,7 @@ import ComposableArchitecture import XCTest @MainActor -final class EffectRunTests: XCTestCase { +final class EffectRunTests: BaseTCATestCase { func testRun() async { struct State: Equatable {} enum Action: Equatable { case tapped, response } diff --git a/Tests/ComposableArchitectureTests/EffectTaskTests.swift b/Tests/ComposableArchitectureTests/EffectTaskTests.swift index 7c9dd56a5575..443faac89adc 100644 --- a/Tests/ComposableArchitectureTests/EffectTaskTests.swift +++ b/Tests/ComposableArchitectureTests/EffectTaskTests.swift @@ -3,7 +3,7 @@ import ComposableArchitecture import XCTest @MainActor -final class EffectTaskTests: XCTestCase { +final class EffectTaskTests: BaseTCATestCase { func testTask() async { struct State: Equatable {} enum Action: Equatable { case tapped, response } diff --git a/Tests/ComposableArchitectureTests/EffectTests.swift b/Tests/ComposableArchitectureTests/EffectTests.swift index 0fb02cf71101..4eff54cea0b8 100644 --- a/Tests/ComposableArchitectureTests/EffectTests.swift +++ b/Tests/ComposableArchitectureTests/EffectTests.swift @@ -3,7 +3,7 @@ import Combine import XCTest @MainActor -final class EffectTests: XCTestCase { +final class EffectTests: BaseTCATestCase { var cancellables: Set = [] let mainQueue = DispatchQueue.test diff --git a/Tests/ComposableArchitectureTests/EffectThrottleTests.swift b/Tests/ComposableArchitectureTests/EffectThrottleTests.swift index d96ba275a0a5..8edc9d718852 100644 --- a/Tests/ComposableArchitectureTests/EffectThrottleTests.swift +++ b/Tests/ComposableArchitectureTests/EffectThrottleTests.swift @@ -3,17 +3,19 @@ import ComposableArchitecture import XCTest @MainActor -final class EffectThrottleTests: XCTestCase { +final class EffectThrottleTests: BaseTCATestCase { var cancellables: Set = [] let mainQueue = DispatchQueue.test func testThrottleLatest() async { + struct CancelID: Hashable {} + defer { Task.cancel(id: CancelID()) } + var values: [Int] = [] var effectRuns = 0 // NB: Explicit @MainActor is needed for Swift 5.5.2 @MainActor func runThrottledEffect(value: Int) { - enum CancelToken {} Deferred { () -> Just in effectRuns += 1 @@ -21,7 +23,7 @@ final class EffectThrottleTests: XCTestCase { } .eraseToEffect() .throttle( - id: CancelToken.self, for: 1, scheduler: mainQueue.eraseToAnyScheduler(), latest: true + id: CancelID(), for: 1, scheduler: mainQueue.eraseToAnyScheduler(), latest: true ) .sink { values.append($0) } .store(in: &self.cancellables) @@ -63,12 +65,14 @@ final class EffectThrottleTests: XCTestCase { } func testThrottleFirst() async { + struct CancelID: Hashable {} + defer { Task.cancel(id: CancelID()) } + var values: [Int] = [] var effectRuns = 0 // NB: Explicit @MainActor is needed for Swift 5.5.2 @MainActor func runThrottledEffect(value: Int) { - enum CancelToken {} Deferred { () -> Just in effectRuns += 1 @@ -76,7 +80,7 @@ final class EffectThrottleTests: XCTestCase { } .eraseToEffect() .throttle( - id: CancelToken.self, for: 1, scheduler: mainQueue.eraseToAnyScheduler(), latest: false + id: CancelID(), for: 1, scheduler: mainQueue.eraseToAnyScheduler(), latest: false ) .sink { values.append($0) } .store(in: &self.cancellables) @@ -131,12 +135,13 @@ final class EffectThrottleTests: XCTestCase { } func testThrottleAfterInterval() async { + struct CancelID: Hashable {} + var values: [Int] = [] var effectRuns = 0 // NB: Explicit @MainActor is needed for Swift 5.5.2 @MainActor func runThrottledEffect(value: Int) { - enum CancelToken {} Deferred { () -> Just in effectRuns += 1 @@ -144,7 +149,7 @@ final class EffectThrottleTests: XCTestCase { } .eraseToEffect() .throttle( - id: CancelToken.self, for: 1, scheduler: mainQueue.eraseToAnyScheduler(), latest: true + id: CancelID(), for: 1, scheduler: mainQueue.eraseToAnyScheduler(), latest: true ) .sink { values.append($0) } .store(in: &self.cancellables) @@ -177,20 +182,21 @@ final class EffectThrottleTests: XCTestCase { } func testThrottleEmitsFirstValueOnce() async { + struct CancelID: Hashable {} + defer { Task.cancel(id: CancelID()) } + var values: [Int] = [] var effectRuns = 0 // NB: Explicit @MainActor is needed for Swift 5.5.2 @MainActor func runThrottledEffect(value: Int) { - enum CancelToken {} - Deferred { () -> Just in effectRuns += 1 return Just(value) } .eraseToEffect() .throttle( - id: CancelToken.self, for: 1, scheduler: mainQueue.eraseToAnyScheduler(), latest: false + id: CancelID(), for: 1, scheduler: mainQueue.eraseToAnyScheduler(), latest: false ) .sink { values.append($0) } .store(in: &self.cancellables) diff --git a/Tests/ComposableArchitectureTests/ForEachReducerTests.swift b/Tests/ComposableArchitectureTests/ForEachReducerTests.swift index 5a3f8337be55..ef274aea70f8 100644 --- a/Tests/ComposableArchitectureTests/ForEachReducerTests.swift +++ b/Tests/ComposableArchitectureTests/ForEachReducerTests.swift @@ -2,7 +2,7 @@ import ComposableArchitecture import XCTest @MainActor -final class ForEachReducerTests: XCTestCase { +final class ForEachReducerTests: BaseTCATestCase { func testElementAction() async { let store = TestStore( initialState: Elements.State( diff --git a/Tests/ComposableArchitectureTests/IfCaseLetReducerTests.swift b/Tests/ComposableArchitectureTests/IfCaseLetReducerTests.swift index b93b0146edba..0a2eaaca1453 100644 --- a/Tests/ComposableArchitectureTests/IfCaseLetReducerTests.swift +++ b/Tests/ComposableArchitectureTests/IfCaseLetReducerTests.swift @@ -2,7 +2,7 @@ import ComposableArchitecture import XCTest @MainActor -final class IfCaseLetReducerTests: XCTestCase { +final class IfCaseLetReducerTests: BaseTCATestCase { func testChildAction() async { struct SomeError: Error, Equatable {} diff --git a/Tests/ComposableArchitectureTests/IfLetReducerTests.swift b/Tests/ComposableArchitectureTests/IfLetReducerTests.swift index f4a95f365ed5..f4c381b39554 100644 --- a/Tests/ComposableArchitectureTests/IfLetReducerTests.swift +++ b/Tests/ComposableArchitectureTests/IfLetReducerTests.swift @@ -2,7 +2,7 @@ import ComposableArchitecture import XCTest @MainActor -final class IfLetReducerTests: XCTestCase { +final class IfLetReducerTests: BaseTCATestCase { #if DEBUG func testNilChild() async { let store = TestStore( diff --git a/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift b/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift new file mode 100644 index 000000000000..9f51c962c61f --- /dev/null +++ b/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift @@ -0,0 +1,10 @@ +import XCTest + +@_spi(Internals) import ComposableArchitecture + +class BaseTCATestCase: XCTestCase { + override func tearDown() { + super.tearDown() + XCTAssertEqual(_cancellationCancellables.count, 0) + } +} diff --git a/Tests/ComposableArchitectureTests/MemoryManagementTests.swift b/Tests/ComposableArchitectureTests/MemoryManagementTests.swift index 1e8e37f1d759..213250a7e230 100644 --- a/Tests/ComposableArchitectureTests/MemoryManagementTests.swift +++ b/Tests/ComposableArchitectureTests/MemoryManagementTests.swift @@ -2,7 +2,7 @@ import Combine import ComposableArchitecture import XCTest -final class MemoryManagementTests: XCTestCase { +final class MemoryManagementTests: BaseTCATestCase { var cancellables: Set = [] func testOwnership_ScopeHoldsOntoParent() { diff --git a/Tests/ComposableArchitectureTests/ReducerTests.swift b/Tests/ComposableArchitectureTests/ReducerTests.swift index e86997176fb5..b36b1d5828cc 100644 --- a/Tests/ComposableArchitectureTests/ReducerTests.swift +++ b/Tests/ComposableArchitectureTests/ReducerTests.swift @@ -5,7 +5,7 @@ import XCTest import os.signpost @MainActor -final class ReducerTests: XCTestCase { +final class ReducerTests: BaseTCATestCase { var cancellables: Set = [] func testCallableAsFunction() { diff --git a/Tests/ComposableArchitectureTests/RuntimeWarningTests.swift b/Tests/ComposableArchitectureTests/RuntimeWarningTests.swift index 871ace1b4748..737fed042fc0 100644 --- a/Tests/ComposableArchitectureTests/RuntimeWarningTests.swift +++ b/Tests/ComposableArchitectureTests/RuntimeWarningTests.swift @@ -3,7 +3,7 @@ import ComposableArchitecture import XCTest - final class RuntimeWarningTests: XCTestCase { + final class RuntimeWarningTests: BaseTCATestCase { func testStoreCreationMainThread() { XCTExpectFailure { $0.compactDescription == """ diff --git a/Tests/ComposableArchitectureTests/ScopeTests.swift b/Tests/ComposableArchitectureTests/ScopeTests.swift index 939b7740bbea..62daadb6f629 100644 --- a/Tests/ComposableArchitectureTests/ScopeTests.swift +++ b/Tests/ComposableArchitectureTests/ScopeTests.swift @@ -2,7 +2,7 @@ import ComposableArchitecture import XCTest @MainActor -final class ScopeTests: XCTestCase { +final class ScopeTests: BaseTCATestCase { func testStructChild() async { let store = TestStore( initialState: Feature.State(), diff --git a/Tests/ComposableArchitectureTests/StoreFilterTests.swift b/Tests/ComposableArchitectureTests/StoreFilterTests.swift index 63ea7a3ad183..802f8db38f0b 100644 --- a/Tests/ComposableArchitectureTests/StoreFilterTests.swift +++ b/Tests/ComposableArchitectureTests/StoreFilterTests.swift @@ -5,7 +5,7 @@ @testable import ComposableArchitecture @MainActor - final class StoreFilterTests: XCTestCase { + final class StoreFilterTests: BaseTCATestCase { var cancellables: Set = [] func testFilter() { diff --git a/Tests/ComposableArchitectureTests/StoreTests.swift b/Tests/ComposableArchitectureTests/StoreTests.swift index cd914156587f..576708d68f3e 100644 --- a/Tests/ComposableArchitectureTests/StoreTests.swift +++ b/Tests/ComposableArchitectureTests/StoreTests.swift @@ -3,7 +3,7 @@ import Combine import XCTest @MainActor -final class StoreTests: XCTestCase { +final class StoreTests: BaseTCATestCase { var cancellables: Set = [] func testCancellableIsRemovedOnImmediatelyCompletingEffect() { diff --git a/Tests/ComposableArchitectureTests/TaskCancellationTests.swift b/Tests/ComposableArchitectureTests/TaskCancellationTests.swift index b275397043ff..8f95a6412d70 100644 --- a/Tests/ComposableArchitectureTests/TaskCancellationTests.swift +++ b/Tests/ComposableArchitectureTests/TaskCancellationTests.swift @@ -1,46 +1,41 @@ -#if DEBUG - import Combine - import XCTest - @_spi(Internals) import ComposableArchitecture +import Combine +@_spi(Internals) import ComposableArchitecture +import XCTest - final class TaskCancellationTests: XCTestCase { - func testCancellation() async throws { - _cancellablesLock.sync { - _cancellationCancellables.removeAll() - } - enum ID {} - let (stream, continuation) = AsyncStream.streamWithContinuation() - let task = Task { - try await withTaskCancellation(id: ID.self) { - continuation.yield() - continuation.finish() - try await Task.never() - } - } - await stream.first(where: { true }) - Task.cancel(id: ID.self) - await Task.megaYield(count: 20) - XCTAssertEqual(_cancellablesLock.sync { _cancellationCancellables }, [:]) - do { - try await task.cancellableValue - XCTFail() - } catch { +final class TaskCancellationTests: BaseTCATestCase { + func testCancellation() async throws { + enum ID {} + let (stream, continuation) = AsyncStream.streamWithContinuation() + let task = Task { + try await withTaskCancellation(id: ID.self) { + continuation.yield() + continuation.finish() + try await Task.never() } } + await stream.first(where: { true }) + Task.cancel(id: ID.self) + await Task.megaYield(count: 20) + XCTAssertEqual(_cancellationCancellables.count, 0) + do { + try await task.cancellableValue + XCTFail() + } catch { + } + } - func testWithTaskCancellationCleansUpTask() async throws { - let task = Task { - try await withTaskCancellation(id: 0) { - try await Task.sleep(nanoseconds: NSEC_PER_SEC * 1000) - } + func testWithTaskCancellationCleansUpTask() async throws { + let task = Task { + try await withTaskCancellation(id: 0) { + try await Task.sleep(nanoseconds: NSEC_PER_SEC * 1000) } + } - try await Task.sleep(nanoseconds: NSEC_PER_SEC / 3) - XCTAssertEqual(_cancellationCancellables.count, 1) + try await Task.sleep(nanoseconds: NSEC_PER_SEC / 3) + XCTAssertEqual(_cancellationCancellables.count, 1) - task.cancel() - try await Task.sleep(nanoseconds: NSEC_PER_SEC / 3) - XCTAssertEqual(_cancellationCancellables.count, 0) - } + task.cancel() + try await Task.sleep(nanoseconds: NSEC_PER_SEC / 3) + XCTAssertEqual(_cancellationCancellables.count, 0) } -#endif +} diff --git a/Tests/ComposableArchitectureTests/TaskResultTests.swift b/Tests/ComposableArchitectureTests/TaskResultTests.swift index b98aa0e2f3c0..22c88c1e95d1 100644 --- a/Tests/ComposableArchitectureTests/TaskResultTests.swift +++ b/Tests/ComposableArchitectureTests/TaskResultTests.swift @@ -1,7 +1,7 @@ import ComposableArchitecture import XCTest -final class TaskResultTests: XCTestCase { +final class TaskResultTests: BaseTCATestCase { #if DEBUG func testEqualityNonEquatableError() { struct Failure: Error { diff --git a/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift b/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift index 8ad8a727164c..a29cf3272013 100644 --- a/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift +++ b/Tests/ComposableArchitectureTests/TestStoreFailureTests.swift @@ -3,7 +3,7 @@ import XCTest @MainActor - final class TestStoreFailureTests: XCTestCase { + final class TestStoreFailureTests: BaseTCATestCase { func testNoStateChangeFailure() { enum Action { case first, second } let store = TestStore( diff --git a/Tests/ComposableArchitectureTests/TestStoreNonExhaustiveTests.swift b/Tests/ComposableArchitectureTests/TestStoreNonExhaustiveTests.swift index 13c5bb0484ed..dd13b8ef3f0e 100644 --- a/Tests/ComposableArchitectureTests/TestStoreNonExhaustiveTests.swift +++ b/Tests/ComposableArchitectureTests/TestStoreNonExhaustiveTests.swift @@ -3,7 +3,7 @@ import XCTest @MainActor - final class TestStoreNonExhaustiveTests: XCTestCase { + final class TestStoreNonExhaustiveTests: BaseTCATestCase { func testSkipReceivedActions_NonStrict() async { let store = TestStore( initialState: 0, diff --git a/Tests/ComposableArchitectureTests/TestStoreTests.swift b/Tests/ComposableArchitectureTests/TestStoreTests.swift index ff6aead4b9eb..d290147094fb 100644 --- a/Tests/ComposableArchitectureTests/TestStoreTests.swift +++ b/Tests/ComposableArchitectureTests/TestStoreTests.swift @@ -3,7 +3,7 @@ import ComposableArchitecture import XCTest @MainActor -final class TestStoreTests: XCTestCase { +final class TestStoreTests: BaseTCATestCase { func testEffectConcatenation() async { struct State: Equatable {} diff --git a/Tests/ComposableArchitectureTests/TimerTests.swift b/Tests/ComposableArchitectureTests/TimerTests.swift index 7a6a4aafbad8..edcd07b04ac7 100644 --- a/Tests/ComposableArchitectureTests/TimerTests.swift +++ b/Tests/ComposableArchitectureTests/TimerTests.swift @@ -3,7 +3,7 @@ import ComposableArchitecture import XCTest @MainActor -final class TimerTests: XCTestCase { +final class TimerTests: BaseTCATestCase { var cancellables: Set = [] func testTimer() async { @@ -11,6 +11,7 @@ final class TimerTests: XCTestCase { var count = 0 + defer { Task.cancel(id: 1) } EffectPublisher.timer(id: 1, every: .seconds(1), on: mainQueue) .sink { _ in count += 1 } .store(in: &self.cancellables) @@ -34,6 +35,10 @@ final class TimerTests: XCTestCase { var count2 = 0 var count3 = 0 + defer { + Task.cancel(id: 1) + Task.cancel(id: 2) + } EffectPublisher.merge( EffectPublisher.timer(id: 1, every: .seconds(2), on: mainQueue) .handleEvents(receiveOutput: { _ in count2 += 1 }) @@ -67,6 +72,7 @@ final class TimerTests: XCTestCase { struct CancelToken: Hashable {} + defer { Task.cancel(id: CancelToken()) } EffectPublisher.timer(id: CancelToken(), every: .seconds(2), on: mainQueue) .handleEvents(receiveOutput: { _ in firstCount += 1 }) .eraseToEffect() @@ -103,6 +109,7 @@ final class TimerTests: XCTestCase { var count = 0 + defer { Task.cancel(id: 1) } EffectPublisher.timer(id: 1, every: .seconds(1), on: mainQueue) .prefix(3) .sink { _ in count += 1 } diff --git a/Tests/ComposableArchitectureTests/ViewStoreTests.swift b/Tests/ComposableArchitectureTests/ViewStoreTests.swift index 1199db170b6d..52863cbeb197 100644 --- a/Tests/ComposableArchitectureTests/ViewStoreTests.swift +++ b/Tests/ComposableArchitectureTests/ViewStoreTests.swift @@ -3,7 +3,7 @@ import ComposableArchitecture import XCTest @MainActor -final class ViewStoreTests: XCTestCase { +final class ViewStoreTests: BaseTCATestCase { var cancellables: Set = [] override func setUp() { From 1a5876c4cfab10607fe8b237480cc8a7e6707a93 Mon Sep 17 00:00:00 2001 From: mbrandonw Date: Wed, 15 Mar 2023 20:03:36 +0000 Subject: [PATCH 4/7] Run swift-format --- .../swift-composable-architecture-benchmark/StoreSuite.swift | 4 ++-- .../Internal/BaseTCATestCase.swift | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Sources/swift-composable-architecture-benchmark/StoreSuite.swift b/Sources/swift-composable-architecture-benchmark/StoreSuite.swift index f6f2cab7a080..11e790f9955a 100644 --- a/Sources/swift-composable-architecture-benchmark/StoreSuite.swift +++ b/Sources/swift-composable-architecture-benchmark/StoreSuite.swift @@ -96,6 +96,6 @@ private func none(level: Int) -> Feature.Action { } private func count(of state: Feature.State?, level: Int) -> Int? { level == 0 - ? state?.count - : count(of: state?.child, level: level - 1) + ? state?.count + : count(of: state?.child, level: level - 1) } diff --git a/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift b/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift index 9f51c962c61f..c10cc4e8e476 100644 --- a/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift +++ b/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift @@ -1,6 +1,5 @@ -import XCTest - @_spi(Internals) import ComposableArchitecture +import XCTest class BaseTCATestCase: XCTestCase { override func tearDown() { From c2aa5dbf811144f2ace9d755922b45197dbf8185 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 15 Mar 2023 16:23:15 -0700 Subject: [PATCH 5/7] Update cancellation stuff from main --- .../Effects/Cancellation.swift | 94 ++++++++++++------- .../Internal/EphemeralState.swift | 1 + .../Internal/NavigationID.swift | 1 + .../Reducer/Reducers/Presentation.swift | 54 ++++++++--- .../StoreSuite.swift | 26 ++--- .../main.swift | 10 +- .../EffectCancellationTests.swift | 42 +++++---- .../Internal/BaseTCATestCase.swift | 3 +- .../PresentationReducerTests.swift | 59 +++++++----- 9 files changed, 179 insertions(+), 111 deletions(-) diff --git a/Sources/ComposableArchitecture/Effects/Cancellation.swift b/Sources/ComposableArchitecture/Effects/Cancellation.swift index 7fea2a53f318..af9027411d47 100644 --- a/Sources/ComposableArchitecture/Effects/Cancellation.swift +++ b/Sources/ComposableArchitecture/Effects/Cancellation.swift @@ -29,6 +29,8 @@ extension EffectPublisher { /// canceled before starting this new one. /// - Returns: A new effect that is capable of being canceled by an identifier. public func cancellable(id: AnyHashable, cancelInFlight: Bool = false) -> Self { + @Dependency(\.navigationIDPath) var navigationIDPath + switch self.operation { case .none: return .none @@ -46,7 +48,7 @@ extension EffectPublisher { defer { _cancellablesLock.unlock() } if cancelInFlight { - _cancellationCancellables.cancel(id: id) + _cancellationCancellables.cancel(id: id, path: navigationIDPath) } let cancellationSubject = PassthroughSubject() @@ -56,7 +58,7 @@ extension EffectPublisher { _cancellablesLock.sync { cancellationSubject.send(()) cancellationSubject.send(completion: .finished) - _cancellationCancellables.remove(cancellable, at: id) + _cancellationCancellables.remove(cancellable, at: id, path: navigationIDPath) } } @@ -64,7 +66,7 @@ extension EffectPublisher { .handleEvents( receiveSubscription: { _ in _cancellablesLock.sync { - _cancellationCancellables.insert(cancellable, at: id) + _cancellationCancellables.insert(cancellable, at: id, path: navigationIDPath) } }, receiveCompletion: { _ in cancellable.cancel() }, @@ -75,13 +77,17 @@ extension EffectPublisher { ) ) case let .run(priority, operation): - return Self( - operation: .run(priority) { send in - await withTaskCancellation(id: id, cancelInFlight: cancelInFlight) { - await operation(send) + return withEscapedDependencies { continuation in + return Self( + operation: .run(priority) { send in + await continuation.yield { + await withTaskCancellation(id: id, cancelInFlight: cancelInFlight) { + await operation(send) + } + } } - } - ) + ) + } } } @@ -105,9 +111,11 @@ extension EffectPublisher { /// - Returns: A new effect that will cancel any currently in-flight effect with the given /// identifier. public static func cancel(id: AnyHashable) -> Self { - .fireAndForget { + @Dependency(\.navigationIDPath) var navigationIDPath + + return .fireAndForget { _cancellablesLock.sync { - _cancellationCancellables.cancel(id: id) + _cancellationCancellables.cancel(id: id, path: navigationIDPath) } } } @@ -195,18 +203,20 @@ extension EffectPublisher { cancelInFlight: Bool = false, operation: @Sendable @escaping () async throws -> T ) async rethrows -> T { + @Dependency(\.navigationIDPath) var navigationIDPath + let (cancellable, task) = _cancellablesLock.sync { () -> (AnyCancellable, Task) in if cancelInFlight { - _cancellationCancellables.cancel(id: id) + _cancellationCancellables.cancel(id: id, path: navigationIDPath) } let task = Task { try await operation() } let cancellable = AnyCancellable { task.cancel() } - _cancellationCancellables.insert(cancellable, at: id) + _cancellationCancellables.insert(cancellable, at: id, path: navigationIDPath) return (cancellable, task) } defer { _cancellablesLock.sync { - _cancellationCancellables.remove(cancellable, at: id) + _cancellationCancellables.remove(cancellable, at: id, path: navigationIDPath) } } do { @@ -287,8 +297,10 @@ extension Task where Success == Never, Failure == Never { /// /// - Parameter id: An identifier. public static func cancel(id: ID) { - _cancellablesLock.sync { - _cancellationCancellables.cancel(id: id) + @Dependency(\.navigationIDPath) var navigationIDPath + + return _cancellablesLock.sync { + _cancellationCancellables.cancel(id: id, path: navigationIDPath) } } @@ -304,12 +316,14 @@ extension Task where Success == Never, Failure == Never { } @_spi(Internals) public struct _CancelID: Hashable { - let id: AnyHashable let discriminator: ObjectIdentifier + let id: AnyHashable + let navigationIDPath: NavigationIDPath - public init(id: AnyHashable) { - self.id = id + init(id: AnyHashable, navigationIDPath: NavigationIDPath) { self.discriminator = ObjectIdentifier(type(of: id.base)) + self.id = id + self.navigationIDPath = navigationIDPath } } @@ -337,38 +351,54 @@ extension Result: _ErrorMechanism {} @_spi(Internals) public class CancellablesCollection { - var storage: [_CancelID: Set] = [:] + var storage: [_CancelID: Set] = [:] func insert( _ cancellable: AnyCancellable, - at id: AnyHashable + at id: AnyHashable, + path: NavigationIDPath ) { - let cancelID = _CancelID(id: id) - self.storage[cancelID, default: []].insert(cancellable) + for navigationIDPath in path.prefixes { + let cancelID = _CancelID(id: id, navigationIDPath: navigationIDPath) + self.storage[cancelID, default: []].insert(cancellable) + } } func remove( _ cancellable: AnyCancellable, - at id: AnyHashable + at id: AnyHashable, + path: NavigationIDPath ) { - let cancelID = _CancelID(id: id) - self.storage[cancelID]?.remove(cancellable) - if self.storage[cancelID]?.isEmpty == true { - self.storage[cancelID] = nil + for navigationIDPath in path.prefixes { + let cancelID = _CancelID(id: id, navigationIDPath: navigationIDPath) + self.storage[cancelID]?.remove(cancellable) + if self.storage[cancelID]?.isEmpty == true { + self.storage[cancelID] = nil + } } } - func cancel(id: AnyHashable) { - let cancelID = _CancelID(id: id) + func cancel( + id: AnyHashable, + path: NavigationIDPath + ) { + let cancelID = _CancelID(id: id, navigationIDPath: path) self.storage[cancelID]?.forEach { $0.cancel() } self.storage[cancelID] = nil } - public func exists(at id: AnyHashable) -> Bool { - return self.storage[_CancelID(id: id)] != nil + func exists( + at id: AnyHashable, + path: NavigationIDPath + ) -> Bool { + return self.storage[_CancelID(id: id, navigationIDPath: path)] != nil } public var count: Int { return self.storage.count } + + public func removeAll() { + self.storage.removeAll() + } } diff --git a/Sources/ComposableArchitecture/Internal/EphemeralState.swift b/Sources/ComposableArchitecture/Internal/EphemeralState.swift index 7b29e5149178..2cc2da027593 100644 --- a/Sources/ComposableArchitecture/Internal/EphemeralState.swift +++ b/Sources/ComposableArchitecture/Internal/EphemeralState.swift @@ -11,6 +11,7 @@ extension AlertState: _EphemeralState {} @available(iOS 13, macOS 12, tvOS 13, watchOS 6, *) extension ConfirmationDialogState: _EphemeralState {} +@usableFromInline func isEphemeral(_ state: State) -> Bool { if State.self is _EphemeralState.Type { return true diff --git a/Sources/ComposableArchitecture/Internal/NavigationID.swift b/Sources/ComposableArchitecture/Internal/NavigationID.swift index 53d5c6fa5ac0..006729c81916 100644 --- a/Sources/ComposableArchitecture/Internal/NavigationID.swift +++ b/Sources/ComposableArchitecture/Internal/NavigationID.swift @@ -17,6 +17,7 @@ private enum NavigationIDPathKey: DependencyKey { struct NavigationIDPath: Hashable, Identifiable, Sendable { fileprivate var path: [NavigationID] + @usableFromInline init(path: [NavigationID] = []) { self.path = path } diff --git a/Sources/ComposableArchitecture/Reducer/Reducers/Presentation.swift b/Sources/ComposableArchitecture/Reducer/Reducers/Presentation.swift index 0f57e7adce41..df9559661628 100644 --- a/Sources/ComposableArchitecture/Reducer/Reducers/Presentation.swift +++ b/Sources/ComposableArchitecture/Reducer/Reducers/Presentation.swift @@ -8,7 +8,7 @@ import Combine @propertyWrapper public struct PresentationState { private var boxedValue: [State] - fileprivate var isPresented = false + @usableFromInline var isPresented = false public init(wrappedValue: State?) { self.boxedValue = wrappedValue.map { [$0] } ?? [] @@ -149,6 +149,7 @@ extension ReducerProtocol { /// state. /// - Returns: A reducer that combines the child reducer with the parent reducer. @warn_unqualified_access + @inlinable public func ifLet( _ toPresentationState: WritableKeyPath>, action toPresentationAction: CasePath>, @@ -172,6 +173,7 @@ extension ReducerProtocol { /// A special overload of ``ReducerProtocol/ifLet(_:action:then:file:fileID:line:)-23pza`` for /// alerts and confirmation dialogs that does not require a child reducer. @warn_unqualified_access + @inlinable public func ifLet( _ toPresentationState: WritableKeyPath>, action toPresentationAction: CasePath>, @@ -193,16 +195,36 @@ extension ReducerProtocol { public struct _PresentationReducer< Base: ReducerProtocol, Destination: ReducerProtocol >: ReducerProtocol { - let base: Base - let toPresentationState: WritableKeyPath> - let toPresentationAction: CasePath> - let destination: Destination - let file: StaticString - let fileID: StaticString - let line: UInt + @usableFromInline let base: Base + @usableFromInline let toPresentationState: WritableKeyPath> + @usableFromInline let toPresentationAction: CasePath> + @usableFromInline let destination: Destination + @usableFromInline let file: StaticString + @usableFromInline let fileID: StaticString + @usableFromInline let line: UInt + + @usableFromInline @Dependency(\.navigationIDPath) var navigationIDPath - @Dependency(\.navigationIDPath) var navigationIDPath + @usableFromInline + init( + base: Base, + toPresentationState: WritableKeyPath>, + toPresentationAction: CasePath>, + destination: Destination, + file: StaticString, + fileID: StaticString, + line: UInt + ) { + self.base = base + self.toPresentationState = toPresentationState + self.toPresentationAction = toPresentationAction + self.destination = destination + self.file = file + self.fileID = fileID + self.line = line + } + @inlinable public func reduce( into state: inout Base.State, action: Base.Action ) -> EffectTask { @@ -316,7 +338,8 @@ public struct _PresentationReducer< ) } - private func navigationIDPath(for state: Destination.State) -> NavigationIDPath { + @usableFromInline + func navigationIDPath(for state: Destination.State) -> NavigationIDPath { self.navigationIDPath.appending( NavigationID( base: state, @@ -326,8 +349,14 @@ public struct _PresentationReducer< } } -private struct DismissID: Hashable {} -struct OnFirstAppearID: Hashable {} +@usableFromInline +struct DismissID: Hashable { + @usableFromInline init() {} +} +@usableFromInline +struct OnFirstAppearID: Hashable { + @usableFromInline init() {} +} public struct _PresentedID: Hashable { @inlinable @@ -340,6 +369,7 @@ public struct _PresentedID: Hashable { } extension Task where Success == Never, Failure == Never { + @usableFromInline internal static func _cancel( id: AnyHashable, navigationID: NavigationIDPath diff --git a/Sources/swift-composable-architecture-benchmark/StoreSuite.swift b/Sources/swift-composable-architecture-benchmark/StoreSuite.swift index 11e790f9955a..80de19669935 100644 --- a/Sources/swift-composable-architecture-benchmark/StoreSuite.swift +++ b/Sources/swift-composable-architecture-benchmark/StoreSuite.swift @@ -17,7 +17,7 @@ let storeSuite = BenchmarkSuite(name: "Store") { ) } tearDown: { precondition(count(of: store.state.value, level: level) == 1) - precondition(_cancellationCancellables.count == 0) + _cancellationCancellables.removeAll() } } for level in 1...levels { @@ -30,18 +30,18 @@ let storeSuite = BenchmarkSuite(name: "Store") { ) } tearDown: { precondition(count(of: store.state.value, level: level) == 0) - precondition(_cancellationCancellables.count == 0) + _cancellationCancellables.removeAll() } } } private struct Feature: ReducerProtocol { struct State { - @Box var child: State? + @PresentationState var child: State? var count = 0 } enum Action { - indirect case child(Action) + indirect case child(PresentationAction) case tap case none } @@ -59,24 +59,12 @@ private struct Feature: ReducerProtocol { return .none } } - .ifLet(\.child, action: /Action.child) { + .ifLet(\.$child, action: /Action.child) { Feature() } } } -@propertyWrapper -private struct Box { - private var value: [Value] - var wrappedValue: Value? { - get { self.value.first } - set { self.value = newValue.map { [$0] } ?? [] } - } - init(wrappedValue: Value?) { - self.value = wrappedValue.map { [$0] } ?? [] - } -} - private func state(level: Int) -> Feature.State { Feature.State( child: level == 0 @@ -87,12 +75,12 @@ private func state(level: Int) -> Feature.State { private func tap(level: Int) -> Feature.Action { level == 0 ? .tap - : Feature.Action.child(tap(level: level - 1)) + : Feature.Action.child(.presented(tap(level: level - 1))) } private func none(level: Int) -> Feature.Action { level == 0 ? .none - : Feature.Action.child(none(level: level - 1)) + : Feature.Action.child(.presented(none(level: level - 1))) } private func count(of state: Feature.State?, level: Int) -> Int? { level == 0 diff --git a/Sources/swift-composable-architecture-benchmark/main.swift b/Sources/swift-composable-architecture-benchmark/main.swift index 7a5fb7c2fd76..ad2f2958c54c 100644 --- a/Sources/swift-composable-architecture-benchmark/main.swift +++ b/Sources/swift-composable-architecture-benchmark/main.swift @@ -2,10 +2,10 @@ import Benchmark import ComposableArchitecture Benchmark.main([ - defaultBenchmarkSuite, - dependenciesSuite, - effectSuite, - storeScopeSuite, +// defaultBenchmarkSuite, +// dependenciesSuite, +// effectSuite, +// storeScopeSuite, storeSuite, - viewStoreSuite, +// viewStoreSuite, ]) diff --git a/Tests/ComposableArchitectureTests/EffectCancellationTests.swift b/Tests/ComposableArchitectureTests/EffectCancellationTests.swift index 4bfc32852e40..7a82e6ad2fdb 100644 --- a/Tests/ComposableArchitectureTests/EffectCancellationTests.swift +++ b/Tests/ComposableArchitectureTests/EffectCancellationTests.swift @@ -108,18 +108,6 @@ final class EffectCancellationTests: BaseTCATestCase { XCTAssertEqual(value, nil) } - func testCancellablesCleanUp_OnComplete() { - let id = UUID() - - Just(1) - .eraseToEffect() - .cancellable(id: id) - .sink(receiveValue: { _ in }) - .store(in: &self.cancellables) - - XCTAssertEqual(_cancellationCancellables.exists(at: id), false) - } - func testCancellablesCleanUp_OnCancel() { let id = UUID() @@ -135,7 +123,7 @@ final class EffectCancellationTests: BaseTCATestCase { .sink(receiveValue: { _ in }) .store(in: &self.cancellables) - XCTAssertEqual(_cancellationCancellables.exists(at: id), false) + XCTAssertEqual(_cancellationCancellables.exists(at: id, path: NavigationIDPath()), false) } func testDoubleCancellation() { @@ -228,7 +216,7 @@ final class EffectCancellationTests: BaseTCATestCase { for id in ids { XCTAssertEqual( - _cancellationCancellables.exists(at: id), + _cancellationCancellables.exists(at: id, path: NavigationIDPath()), false, "cancellationCancellables should not contain id \(id)" ) @@ -252,7 +240,7 @@ final class EffectCancellationTests: BaseTCATestCase { cancellables.removeAll() - XCTAssertEqual(_cancellationCancellables.exists(at: id), false) + XCTAssertEqual(_cancellationCancellables.exists(at: id, path: NavigationIDPath()), false) } func testSharedId() { @@ -347,11 +335,31 @@ final class EffectCancellationTests: BaseTCATestCase { func testCancelIDHash() { struct CancelID1: Hashable {} struct CancelID2: Hashable {} - let id1 = _CancelID(id: CancelID1()) - let id2 = _CancelID(id: CancelID2()) + let id1 = _CancelID(id: CancelID1(), navigationIDPath: NavigationIDPath()) + let id2 = _CancelID(id: CancelID2(), navigationIDPath: NavigationIDPath()) XCTAssertNotEqual(id1, id2) // NB: We hash the type of the cancel ID to give more variance in the hash since all empty // structs in Swift have the same hash value. XCTAssertNotEqual(id1.hashValue, id2.hashValue) } } + +#if DEBUG +@testable import ComposableArchitecture + +final class Internal_EffectCancellationTests: BaseTCATestCase { + var cancellables: Set = [] + + func testCancellablesCleanUp_OnComplete() { + let id = UUID() + + Just(1) + .eraseToEffect() + .cancellable(id: id) + .sink(receiveValue: { _ in }) + .store(in: &self.cancellables) + + XCTAssertEqual(_cancellationCancellables.exists(at: id, path: NavigationIDPath()), false) + } +} +#endif diff --git a/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift b/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift index c10cc4e8e476..69cd64c0339d 100644 --- a/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift +++ b/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift @@ -4,6 +4,7 @@ import XCTest class BaseTCATestCase: XCTestCase { override func tearDown() { super.tearDown() - XCTAssertEqual(_cancellationCancellables.count, 0) + XCTAssertEqual(_cancellationCancellables.count, 0, "\(self)") + _cancellationCancellables.removeAll() } } diff --git a/Tests/ComposableArchitectureTests/PresentationReducerTests.swift b/Tests/ComposableArchitectureTests/PresentationReducerTests.swift index 7a5d816f25c6..aadd7a38cfb0 100644 --- a/Tests/ComposableArchitectureTests/PresentationReducerTests.swift +++ b/Tests/ComposableArchitectureTests/PresentationReducerTests.swift @@ -3,7 +3,7 @@ import XCTest #if swift(>=5.7) @MainActor - final class PresentationReducerTests: XCTestCase { + final class PresentationReducerTests: BaseTCATestCase { func testPresentation_parentDismissal() async { struct Child: ReducerProtocol { struct State: Equatable { @@ -1966,7 +1966,11 @@ import XCTest Reduce { state, action in switch action { case .destination(.presented(.alert(.showDialog))): - state.destination = .dialog(ConfirmationDialogState { TextState("Hello!") } actions: {}) + state.destination = .dialog( + ConfirmationDialogState { + TextState("Hello!") + } actions: { + }) return .none case .destination(.presented(.dialog(.showAlert))): state.destination = .alert(AlertState { TextState("Hello!") }) @@ -2009,7 +2013,11 @@ import XCTest $0.destination = .alert(Feature.alert) } await store.send(.destination(.presented(.alert(.showDialog)))) { - $0.destination = .dialog(ConfirmationDialogState { TextState("Hello!") } actions: {}) + $0.destination = .dialog( + ConfirmationDialogState { + TextState("Hello!") + } actions: { + }) } await store.send(.destination(.dismiss)) { $0.destination = nil @@ -2114,28 +2122,29 @@ import XCTest await store.send(.child(.presented(.tap))) XCTExpectFailure { - $0.sourceCodeContext.location?.lineNumber == line + 1 - && $0.compactDescription == """ - An effect returned for this action is still running. It must complete before the end \ - of the test. … - - To fix, inspect any effects the reducer returns for this action and ensure that all of \ - them complete by the end of the test. There are a few reasons why an effect may not \ - have completed: - - • If using async/await in your effect, it may need a little bit of time to properly \ - finish. To fix you can simply perform "await store.finish()" at the end of your test. - - • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", etc.), \ - make sure that you wait enough time for it to perform the effect. If you are using a \ - test clock/scheduler, advance it so that the effects may complete, or consider using an \ - immediate clock/scheduler to immediately perform the effect instead. - - • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ - then make sure those effects are torn down by marking the effect ".cancellable" and \ - returning a corresponding cancellation effect ("Effect.cancel") from another action, or, \ - if your effect is driven by a Combine subject, send it a completion. - """ + $0.sourceCodeContext.location?.fileURL.absoluteString.contains("BaseTCATestCase") == true + || $0.sourceCodeContext.location?.lineNumber == line + 1 + && $0.compactDescription == """ + An effect returned for this action is still running. It must complete before the end \ + of the test. … + + To fix, inspect any effects the reducer returns for this action and ensure that all \ + of them complete by the end of the test. There are a few reasons why an effect may \ + not have completed: + + • If using async/await in your effect, it may need a little bit of time to properly \ + finish. To fix you can simply perform "await store.finish()" at the end of your test. + + • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", \ + etc.), make sure that you wait enough time for it to perform the effect. If you are \ + using a test clock/scheduler, advance it so that the effects may complete, or \ + consider using an immediate clock/scheduler to immediately perform the effect instead. + + • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ + then make sure those effects are torn down by marking the effect ".cancellable" and \ + returning a corresponding cancellation effect ("Effect.cancel") from another action, \ + or, if your effect is driven by a Combine subject, send it a completion. + """ } } From 24abb3504a0e46ac43dea6f2e4e6fdc5278ae195 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 15 Mar 2023 17:09:58 -0700 Subject: [PATCH 6/7] wip --- .../xcshareddata/swiftpm/Package.resolved | 4 +- Package.swift | 1 + .../Effects/Cancellation.swift | 8 +- .../SwiftUI/NavigationDestination.swift | 164 +++++++++--------- 4 files changed, 91 insertions(+), 86 deletions(-) diff --git a/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved index a2709131a87d..c67c033a2962 100644 --- a/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -77,8 +77,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-docc-plugin", "state" : { - "revision" : "10bc670db657d11bdd561e07de30a9041311b2b1", - "version" : "1.1.0" + "revision" : "9b1258905c21fc1b97bf03d1b4ca12c4ec4e5fda", + "version" : "1.2.0" } }, { diff --git a/Package.swift b/Package.swift index 3e5a8f218295..05920f3c4863 100644 --- a/Package.swift +++ b/Package.swift @@ -39,6 +39,7 @@ let package = Package( .product(name: "IdentifiedCollections", package: "swift-identified-collections"), .product(name: "OrderedCollections", package: "swift-collections"), .product(name: "_SwiftUINavigationState", package: "swiftui-navigation"), + // TODO: should we depend on this or copy some stuff over? .product(name: "SwiftUINavigation", package: "swiftui-navigation"), .product(name: "XCTestDynamicOverlay", package: "xctest-dynamic-overlay"), ] diff --git a/Sources/ComposableArchitecture/Effects/Cancellation.swift b/Sources/ComposableArchitecture/Effects/Cancellation.swift index af9027411d47..9b6333741dee 100644 --- a/Sources/ComposableArchitecture/Effects/Cancellation.swift +++ b/Sources/ComposableArchitecture/Effects/Cancellation.swift @@ -231,18 +231,20 @@ extension EffectPublisher { cancelInFlight: Bool = false, operation: @Sendable @escaping () async throws -> T ) async rethrows -> T { + @Dependency(\.navigationIDPath) var navigationIDPath + let (cancellable, task) = _cancellablesLock.sync { () -> (AnyCancellable, Task) in if cancelInFlight { - _cancellationCancellables.cancel(id: id) + _cancellationCancellables.cancel(id: id, path: navigationIDPath) } let task = Task { try await operation() } let cancellable = AnyCancellable { task.cancel() } - _cancellationCancellables.insert(cancellable, at: id) + _cancellationCancellables.insert(cancellable, at: id, path: navigationIDPath) return (cancellable, task) } defer { _cancellablesLock.sync { - _cancellationCancellables.remove(cancellable, at: id) + _cancellationCancellables.remove(cancellable, at: id, path: navigationIDPath) } } do { diff --git a/Sources/ComposableArchitecture/SwiftUI/NavigationDestination.swift b/Sources/ComposableArchitecture/SwiftUI/NavigationDestination.swift index 9bd5d8b45a3e..079e581fe6c1 100644 --- a/Sources/ComposableArchitecture/SwiftUI/NavigationDestination.swift +++ b/Sources/ComposableArchitecture/SwiftUI/NavigationDestination.swift @@ -1,93 +1,95 @@ -import SwiftUI -import SwiftUINavigation +#if swift(>=5.7) + import SwiftUI + import SwiftUINavigation -extension View { - @available(iOS 16, macOS 13, tvOS 16, watchOS 9, *) - public func navigationDestination( - store: Store, PresentationAction>, - @ViewBuilder destination: @escaping (Store) -> Destination - ) -> some View { - self.navigationDestination( - store: store, state: { $0 }, action: { $0 }, destination: destination - ) - } + extension View { + @available(iOS 16, macOS 13, tvOS 16, watchOS 9, *) + public func navigationDestination( + store: Store, PresentationAction>, + @ViewBuilder destination: @escaping (Store) -> Destination + ) -> some View { + self.navigationDestination( + store: store, state: { $0 }, action: { $0 }, destination: destination + ) + } - @available(iOS 16, macOS 13, tvOS 16, watchOS 9, *) - public func navigationDestination< - State, Action, DestinationState, DestinationAction, Destination: View - >( - store: Store, PresentationAction>, - state toDestinationState: @escaping (State) -> DestinationState?, - action fromDestinationAction: @escaping (DestinationAction) -> Action, - @ViewBuilder destination: @escaping (Store) -> - Destination - ) -> some View { - self.modifier( - PresentationNavigationDestinationModifier( - store: store, - state: toDestinationState, - action: fromDestinationAction, - content: destination + @available(iOS 16, macOS 13, tvOS 16, watchOS 9, *) + public func navigationDestination< + State, Action, DestinationState, DestinationAction, Destination: View + >( + store: Store, PresentationAction>, + state toDestinationState: @escaping (State) -> DestinationState?, + action fromDestinationAction: @escaping (DestinationAction) -> Action, + @ViewBuilder destination: @escaping (Store) -> + Destination + ) -> some View { + self.modifier( + PresentationNavigationDestinationModifier( + store: store, + state: toDestinationState, + action: fromDestinationAction, + content: destination + ) ) - ) + } } -} -@available(iOS 16, macOS 13, tvOS 16, watchOS 9, *) -private struct PresentationNavigationDestinationModifier< - State, - Action, - DestinationState, - DestinationAction, - DestinationContent: View ->: ViewModifier { - let store: Store, PresentationAction> - @StateObject var viewStore: ViewStore> - let toDestinationState: (State) -> DestinationState? - let fromDestinationAction: (DestinationAction) -> Action - let destinationContent: (Store) -> DestinationContent + @available(iOS 16, macOS 13, tvOS 16, watchOS 9, *) + private struct PresentationNavigationDestinationModifier< + State, + Action, + DestinationState, + DestinationAction, + DestinationContent: View + >: ViewModifier { + let store: Store, PresentationAction> + @StateObject var viewStore: ViewStore> + let toDestinationState: (State) -> DestinationState? + let fromDestinationAction: (DestinationAction) -> Action + let destinationContent: (Store) -> DestinationContent - init( - store: Store, PresentationAction>, - state toDestinationState: @escaping (State) -> DestinationState?, - action fromDestinationAction: @escaping (DestinationAction) -> Action, - content destinationContent: - @escaping (Store) -> DestinationContent - ) { - self.store = store - self._viewStore = StateObject( - wrappedValue: ViewStore( - store - .filterSend { state, _ in state.wrappedValue != nil } - .scope(state: { $0.wrappedValue.flatMap(toDestinationState) != nil }) + init( + store: Store, PresentationAction>, + state toDestinationState: @escaping (State) -> DestinationState?, + action fromDestinationAction: @escaping (DestinationAction) -> Action, + content destinationContent: + @escaping (Store) -> DestinationContent + ) { + self.store = store + self._viewStore = StateObject( + wrappedValue: ViewStore( + store + .filterSend { state, _ in state.wrappedValue != nil } + .scope(state: { $0.wrappedValue.flatMap(toDestinationState) != nil }) + ) ) - ) - self.toDestinationState = toDestinationState - self.fromDestinationAction = fromDestinationAction - self.destinationContent = destinationContent - } + self.toDestinationState = toDestinationState + self.fromDestinationAction = fromDestinationAction + self.destinationContent = destinationContent + } - func body(content: Content) -> some View { - content.navigationDestination( - // TODO: do binding with ID check - unwrapping: self.viewStore.binding(send: .dismiss).presence - ) { _ in - IfLetStore( - self.store.scope( - state: returningLastNonNilValue { $0.wrappedValue.flatMap(self.toDestinationState) }, - action: { .presented(self.fromDestinationAction($0)) } - ), - then: self.destinationContent - ) + func body(content: Content) -> some View { + content.navigationDestination( + // TODO: do binding with ID check + unwrapping: self.viewStore.binding(send: .dismiss).presence + ) { _ in + IfLetStore( + self.store.scope( + state: returningLastNonNilValue { $0.wrappedValue.flatMap(self.toDestinationState) }, + action: { .presented(self.fromDestinationAction($0)) } + ), + then: self.destinationContent + ) + } } } -} -fileprivate extension Binding where Value == Bool { - var presence: Binding { - .init( - get: { self.wrappedValue ? () : nil }, - set: { self.transaction($1).wrappedValue = $0 != nil } - ) + extension Binding where Value == Bool { + fileprivate var presence: Binding { + .init( + get: { self.wrappedValue ? () : nil }, + set: { self.transaction($1).wrappedValue = $0 != nil } + ) + } } -} +#endif From 619cb405a2a193aefa23e13f8cb9fbe67c8b1b1c Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Tue, 21 Mar 2023 12:00:20 -0700 Subject: [PATCH 7/7] Escape dependencies to EffectTask.publisher (#1988) --- .../Effects/Publisher.swift | 11 ++++++++- .../EffectPublisherTests.swift | 24 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 Tests/ComposableArchitectureTests/EffectPublisherTests.swift diff --git a/Sources/ComposableArchitecture/Effects/Publisher.swift b/Sources/ComposableArchitecture/Effects/Publisher.swift index 564d130a0a60..e21102a32df6 100644 --- a/Sources/ComposableArchitecture/Effects/Publisher.swift +++ b/Sources/ComposableArchitecture/Effects/Publisher.swift @@ -8,7 +8,16 @@ extension EffectPublisher where Failure == Never { public static func publisher(_ createPublisher: @escaping () -> P) -> Self where P.Output == Action, P.Failure == Never { Self( - operation: .publisher(Deferred(createPublisher: createPublisher).eraseToAnyPublisher()) + operation: .publisher( + withEscapedDependencies { continuation in + Deferred { + continuation.yield { + createPublisher() + } + } + } + .eraseToAnyPublisher() + ) ) } } diff --git a/Tests/ComposableArchitectureTests/EffectPublisherTests.swift b/Tests/ComposableArchitectureTests/EffectPublisherTests.swift new file mode 100644 index 000000000000..0f146c2e4578 --- /dev/null +++ b/Tests/ComposableArchitectureTests/EffectPublisherTests.swift @@ -0,0 +1,24 @@ +import Combine +import ComposableArchitecture +import XCTest + +@MainActor +final class EffectPublisherTests: BaseTCATestCase { + var cancellables: Set = [] + + func testEscapedDependencies() { + @Dependency(\.date.now) var now + + let effect = withDependencies { + $0.date.now = Date(timeIntervalSince1970: 1234567890) + } operation: { + EffectTask.publisher { + Just(now) + } + } + + var value: Date? + effect.sink { value = $0 }.store(in: &self.cancellables) + XCTAssertEqual(value, Date(timeIntervalSince1970: 1234567890)) + } +}