* Remove use of deferred where not required. (#4)

* Refactor Effect Cancellation logic
* Remove unnecessary holding on to Disposables

Author: mluisbrown

Examples/CaseStudies/UIKitCaseStudies/CounterViewController.swift

@@ -27,7 +27,6 @@ let counterReducer = Reducer<CounterState, CounterAction, CounterEnvironment> {
 
 final class CounterViewController: UIViewController {
   let viewStore: ViewStore<CounterState, CounterAction>
-  var cancellables: Set<AnyCancellable> = []
 
   init(store: Store<CounterState, CounterAction>) {
     self.viewStore = ViewStore(store)

Examples/CaseStudies/UIKitCaseStudies/Internal/IfLetStoreController.swift

@@ -7,7 +7,6 @@ final class IfLetStoreController<State, Action>: UIViewController {
   let ifDestination: (Store<State, Action>) -> UIViewController
   let elseDestination: () -> UIViewController
 
-  private var cancellables: Set<AnyCancellable> = []
   private var viewController = UIViewController() {
     willSet {
       self.viewController.willMove(toParent: nil)

Examples/CaseStudies/UIKitCaseStudies/ListsOfState.swift

@@ -25,7 +25,6 @@ let cellIdentifier = "Cell"
 final class CountersTableViewController: UITableViewController {
   let store: Store<CounterListState, CounterListAction>
   let viewStore: ViewStore<CounterListState, CounterListAction>
-  var cancellables: Set<AnyCancellable> = []
 
   var dataSource: [CounterState] = [] {
     didSet { self.tableView.reloadData() }

Examples/TicTacToe/Sources/Views-UIKit/AppViewController.swift

@@ -21,7 +21,6 @@ struct UIKitAppView: UIViewControllerRepresentable {
 
 class AppViewController: UINavigationController {
   let store: Store<AppState, AppAction>
-  private var cancellables: Set<AnyCancellable> = []
 
   init(store: Store<AppState, AppAction>) {
     self.store = store

Examples/TicTacToe/Sources/Views-UIKit/NewGameViewController.swift

@@ -22,8 +22,6 @@ class NewGameViewController: UIViewController {
   let store: Store<NewGameState, NewGameAction>
   let viewStore: ViewStore<ViewState, ViewAction>
 
-  private var cancellables: Set<AnyCancellable> = []
-
   init(store: Store<NewGameState, NewGameAction>) {
     self.store = store
     self.viewStore = ViewStore(store.scope(state: { $0.view }, action: NewGameAction.view))

Sources/ComposableArchitecture/Effect.swift

@@ -17,7 +17,7 @@ extension Effect {
   /// An effect that does nothing and completes immediately. Useful for situations where you must
   /// return an effect, but you don't need to do anything.
   public static var none: Effect {
-    return .empty
+    .empty
   }
 
   /// Creates an effect that executes some work in the real world that doesn't need to feed data
@@ -65,7 +65,7 @@ extension Effect {
   public static func deferred(_ createProducer: @escaping () -> SignalProducer<Value, Error>)
     -> SignalProducer<Value, Error>
   {
-    return Effect<Void, Error>(value: ())
+    Effect<Void, Error>(value: ())
       .flatMap(.merge, createProducer)
   }
 
@@ -97,16 +97,14 @@ extension Effect {
   public static func future(
     _ attemptToFulfill: @escaping (@escaping (Result<Value, Error>) -> Void) -> Void
   ) -> Effect {
-    return deferred { () -> SignalProducer<Value, Error> in
-      return SignalProducer { observer, _ in
-        attemptToFulfill { result in
-          switch result {
-          case let .success(value):
-            observer.send(value: value)
-            observer.sendCompleted()
-          case let .failure(error):
-            observer.send(error: error)
-          }
+    SignalProducer { observer, _ in
+      attemptToFulfill { result in
+        switch result {
+        case let .success(value):
+          observer.send(value: value)
+          observer.sendCompleted()
+        case let .failure(error):
+          observer.send(error: error)
         }
       }
     }

Sources/ComposableArchitecture/Effects/Cancellation.swift

@@ -1,31 +1,12 @@
 import Foundation
 import ReactiveSwift
 
-extension AnyDisposable: Hashable {
-  public static func == (lhs: AnyDisposable, rhs: AnyDisposable) -> Bool {
-    return ObjectIdentifier(lhs) == ObjectIdentifier(rhs)
-  }
-
-  public func hash(into hasher: inout Hasher) {
-    hasher.combine(ObjectIdentifier(self))
-  }
-}
-
-extension AnyDisposable {
-  /// Stores this AnyDisposable in the specified collection.
+extension Disposable {
+  /// Adds this Disposable to the specified CompositeDisposable
   /// Parameters:
-  ///    - collection: The collection to store this AnyCancellable.
-  public func store<Disposables: RangeReplaceableCollection>(
-    in collection: inout Disposables
-  ) where Disposables.Element == AnyDisposable {
-    collection.append(self)
-  }
-
-  /// Stores this AnyCancellable in the specified set.
-  /// Parameters:
-  ///    - set: The set to store this AnyCancellable.
-  public func store(in set: inout Set<AnyDisposable>) {
-    set.insert(self)
+  ///    - composite: The CompositeDisposable to which to add this Disposable.
+  internal func add(to composite: inout CompositeDisposable) {
+    composite.add(self)
   }
 }
 
@@ -58,37 +39,32 @@ extension Effect {
     return .deferred { () -> SignalProducer<Value, Error> in
       let subject = Signal<Value, Error>.pipe()
 
-      var disposable: Disposable?
       var values: [Value] = []
       var isCaching = true
 
       cancellablesLock.sync {
         if cancelInFlight {
-          cancellationCancellables[id]?.forEach { cancellable in cancellable.dispose() }
+          cancellationCancellables[id]?.dispose()
           cancellationCancellables[id] = nil
         }
 
-        disposable =
-          self
+        let disposable = self
           .on {
             guard isCaching else { return }
             values.append($0)
           }
           .start(subject.input)
 
-        AnyDisposable {
-          disposable?.dispose()
-          subject.input.sendCompleted()
-        }
-        .store(in: &cancellationCancellables[id, default: []])
+          disposable.add(to: &cancellationCancellables[id, default: CompositeDisposable()])
       }
 
       func cleanUp() {
-        disposable?.dispose()
         cancellablesLock.sync {
           guard !isCancelling.contains(id) else { return }
           isCancelling.insert(id)
           defer { isCancelling.remove(id) }
+
+          cancellationCancellables[id]?.dispose()
           cancellationCancellables[id] = nil
         }
       }
@@ -115,13 +91,13 @@ extension Effect {
   public static func cancel(id: AnyHashable) -> Effect {
     .fireAndForget {
       cancellablesLock.sync {
-        cancellationCancellables[id]?.forEach { cancellable in cancellable.dispose() }
+        cancellationCancellables[id]?.dispose()
         cancellationCancellables[id] = nil
       }
     }
   }
 }
 
-var cancellationCancellables: [AnyHashable: Set<AnyDisposable>] = [:]
+var cancellationCancellables: [AnyHashable: CompositeDisposable] = [:]
 let cancellablesLock = NSRecursiveLock()
 var isCancelling: Set<AnyHashable> = []

Sources/ComposableArchitecture/Store.swift

@@ -8,9 +8,7 @@ import ReactiveSwift
 /// the `scope` method to derive more focused stores that can be passed to subviews.
 public final class Store<State, Action> {
   @MutableProperty private(set) var state: State
-  var effectCancellables: [UUID: Disposable] = [:]
   private var isSending = false
-  private var parentCancellable: Disposable?
   private let reducer: (inout State, Action) -> Effect<Action, Never>
   private var synchronousActionsToSend: [Action] = []
 
@@ -67,7 +65,7 @@ public final class Store<State, Action> {
         return .none
       }
     )
-    localStore.parentCancellable = self.$state.producer
+    self.$state.producer
       .startWithValues { [weak localStore] newValue in localStore?.state = toLocalState(newValue) }
     return localStore
   }
@@ -111,7 +109,7 @@ public final class Store<State, Action> {
             return .none
           })
 
-        localStore.parentCancellable = self.$state.producer
+        self.$state.producer
           .startWithValues { [weak localStore] state in
             guard let localStore = localStore else { return }
             localStore.state = extractLocalState(state) ?? localStore.state
@@ -147,30 +145,16 @@ public final class Store<State, Action> {
     let effect = self.reducer(&self.state, action)
     self.isSending = false
 
-    var didComplete = false
-    let uuid = UUID()
-
     var isProcessingEffects = true
-    let effectCancellable = effect.start { [weak self] event in
-      switch event {
-      case .completed, .interrupted:
-        didComplete = true
-        self?.effectCancellables[uuid] = nil
-
-      case let .value(action):
-        if isProcessingEffects {
-          self?.synchronousActionsToSend.append(action)
-        } else {
-          self?.send(action)
-        }
+    effect.startWithValues { [weak self] action in
+      if isProcessingEffects {
+        self?.synchronousActionsToSend.append(action)
+      } else {
+        self?.send(action)
       }
     }
     isProcessingEffects = false
 
-    if !didComplete {
-      self.effectCancellables[uuid] = effectCancellable
-    }
-
     while !self.synchronousActionsToSend.isEmpty {
       let action = self.synchronousActionsToSend.removeFirst()
       self.send(action)

Sources/ComposableArchitecture/UIKit/IfLetUIKit.swift

@@ -12,7 +12,6 @@ extension Store {
   ///
   ///     class MasterViewController: UIViewController {
   ///       let store: Store<MasterState, MasterAction>
-  ///       var cancellables: Set<AnyCancellable> = []
   ///       ...
   ///       func viewDidLoad() {
   ///         ...
@@ -30,7 +29,6 @@ extension Store {
   ///               self.navigationController?.popToViewController(self, animated: true)
   ///             }
   ///           )
-  ///           .store(in: &self.cancellables)
   ///       }
   ///     }
   ///

Tests/ComposableArchitectureTests/EffectCancellationTests.swift

@@ -217,24 +217,22 @@ final class EffectCancellationTests: XCTestCase {
     }
     .cancellable(id: 1)
 
-    for _ in 1 ... .random(in: 1...1_000) {
+    for _ in 1 ... 500 {
       effect = effect.cancellable(id: 1)
     }
 
-    let composite = CompositeDisposable()
-    composite.add(effect.start())
+    let disposable = effect.start()
+    disposable.dispose()
 
-    composite.dispose()
-
-    XCTAssertEqual([:], cancellationCancellables)
+    XCTAssertTrue(cancellationCancellables.isEmpty)
     XCTAssertEqual([], isCancelling)
   }
 }
 
 func resetCancellables() {
   cancellablesLock.sync {
     for (id, _) in cancellationCancellables {
-      cancellationCancellables[id] = []
+      cancellationCancellables.removeValue(forKey: id)
     }
     cancellationCancellables = [:]
   }