IfLetStore: ignore view store binding writes to nil state (#1879)

* `IfLetStore`: ignore view store binding writes to `nil` state

* swift-format

* wip

* add test for filter

---------

Co-authored-by: Brandon Williams <[email protected]>

Author: stephencelis

Sources/ComposableArchitecture/Store.swift

@@ -307,10 +307,10 @@ public final class Store<State, Action> {
     self.threadCheck(status: .scope)
 
     #if swift(>=5.7)
-      return self.reducer.rescope(self, state: toChildState, action: fromChildAction)
+      return self.reducer.rescope(self, state: toChildState, action: { fromChildAction($1) })
     #else
       return (self.scope ?? StoreScope(root: self))
-        .rescope(self, state: toChildState, action: fromChildAction)
+        .rescope(self, state: toChildState, action: { fromChildAction($1) })
     #endif
   }
 
@@ -326,6 +326,19 @@ public final class Store<State, Action> {
     self.scope(state: toChildState, action: { $0 })
   }
 
+  @_spi(Internals) public func filter(
+    _ isSent: @escaping (State, Action) -> Bool
+  ) -> Store<State, Action> {
+    self.threadCheck(status: .scope)
+
+    #if swift(>=5.7)
+      return self.reducer.rescope(self, state: { $0 }, action: { isSent($0, $1) ? $1 : nil })
+    #else
+      return (self.scope ?? StoreScope(root: self))
+        .rescope(self, state: { $0 }, action: { isSent($0, $1) ? $1 : nil })
+    #endif
+  }
+
   @_spi(Internals) public func send(
     _ action: Action,
     originatingFrom originatingAction: Action? = nil
@@ -571,7 +584,7 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
     fileprivate func rescope<ChildState, ChildAction>(
       _ store: Store<State, Action>,
       state toChildState: @escaping (State) -> ChildState,
-      action fromChildAction: @escaping (ChildAction) -> Action
+      action fromChildAction: @escaping (ChildState, ChildAction) -> Action?
     ) -> Store<ChildState, ChildAction> {
       (self as? any AnyScopedReducer ?? ScopedReducer(rootStore: store))
         .rescope(store, state: toChildState, action: fromChildAction)
@@ -584,7 +597,7 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
     let rootStore: Store<RootState, RootAction>
     let toScopedState: (RootState) -> ScopedState
     private let parentStores: [Any]
-    let fromScopedAction: (ScopedAction) -> RootAction
+    let fromScopedAction: (ScopedState, ScopedAction) -> RootAction?
     private(set) var isSending = false
 
     @inlinable
@@ -593,14 +606,14 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
       self.rootStore = rootStore
       self.toScopedState = { $0 }
       self.parentStores = []
-      self.fromScopedAction = { $0 }
+      self.fromScopedAction = { $1 }
     }
 
     @inlinable
     init(
       rootStore: Store<RootState, RootAction>,
       state toScopedState: @escaping (RootState) -> ScopedState,
-      action fromScopedAction: @escaping (ScopedAction) -> RootAction,
+      action fromScopedAction: @escaping (ScopedState, ScopedAction) -> RootAction?,
       parentStores: [Any]
     ) {
       self.rootStore = rootStore
@@ -618,7 +631,7 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
         state = self.toScopedState(self.rootStore.state.value)
         self.isSending = false
       }
-      if let task = self.rootStore.send(self.fromScopedAction(action)) {
+      if let action = self.fromScopedAction(state, action), let task = self.rootStore.send(action) {
         return .fireAndForget { await task.cancellableValue }
       } else {
         return .none
@@ -630,7 +643,7 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
     func rescope<ScopedState, ScopedAction, RescopedState, RescopedAction>(
       _ store: Store<ScopedState, ScopedAction>,
       state toRescopedState: @escaping (ScopedState) -> RescopedState,
-      action fromRescopedAction: @escaping (RescopedAction) -> ScopedAction
+      action fromRescopedAction: @escaping (RescopedState, RescopedAction) -> ScopedAction?
     ) -> Store<RescopedState, RescopedAction>
   }
 
@@ -639,13 +652,13 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
     func rescope<ScopedState, ScopedAction, RescopedState, RescopedAction>(
       _ store: Store<ScopedState, ScopedAction>,
       state toRescopedState: @escaping (ScopedState) -> RescopedState,
-      action fromRescopedAction: @escaping (RescopedAction) -> ScopedAction
+      action fromRescopedAction: @escaping (RescopedState, RescopedAction) -> ScopedAction?
     ) -> Store<RescopedState, RescopedAction> {
-      let fromScopedAction = self.fromScopedAction as! (ScopedAction) -> RootAction
+      let fromScopedAction = self.fromScopedAction as! (ScopedState, ScopedAction) -> RootAction?
       let reducer = ScopedReducer<RootState, RootAction, RescopedState, RescopedAction>(
         rootStore: self.rootStore,
         state: { _ in toRescopedState(store.state.value) },
-        action: { fromScopedAction(fromRescopedAction($0)) },
+        action: { fromRescopedAction($0, $1).flatMap { fromScopedAction(store.state.value, $0) } },
         parentStores: self.parentStores + [store]
       )
       let childStore = Store<RescopedState, RescopedAction>(
@@ -666,7 +679,7 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
     func rescope<ScopedState, ScopedAction, RescopedState, RescopedAction>(
       _ store: Store<ScopedState, ScopedAction>,
       state toRescopedState: @escaping (ScopedState) -> RescopedState,
-      action fromRescopedAction: @escaping (RescopedAction) -> ScopedAction
+      action fromRescopedAction: @escaping (RescopedState, RescopedAction) -> ScopedAction?
     ) -> Store<RescopedState, RescopedAction>
   }
 
@@ -675,12 +688,15 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
     let fromScopedAction: Any
 
     init(root: Store<RootState, RootAction>) {
-      self.init(root: root, fromScopedAction: { $0 })
+      self.init(
+        root: root,
+        fromScopedAction: { (state: RootState, action: RootAction) -> RootAction? in action }
+      )
     }
 
-    private init<ScopedAction>(
+    private init<ScopedState, ScopedAction>(
       root: Store<RootState, RootAction>,
-      fromScopedAction: @escaping (ScopedAction) -> RootAction
+      fromScopedAction: @escaping (ScopedState, ScopedAction) -> RootAction?
     ) {
       self.root = root
       self.fromScopedAction = fromScopedAction
@@ -689,17 +705,21 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
     func rescope<ScopedState, ScopedAction, RescopedState, RescopedAction>(
       _ scopedStore: Store<ScopedState, ScopedAction>,
       state toRescopedState: @escaping (ScopedState) -> RescopedState,
-      action fromRescopedAction: @escaping (RescopedAction) -> ScopedAction
+      action fromRescopedAction: @escaping (RescopedState, RescopedAction) -> ScopedAction?
     ) -> Store<RescopedState, RescopedAction> {
-      let fromScopedAction = self.fromScopedAction as! (ScopedAction) -> RootAction
+      let fromScopedAction = self.fromScopedAction as! (ScopedState, ScopedAction) -> RootAction?
 
       var isSending = false
       let rescopedStore = Store<RescopedState, RescopedAction>(
         initialState: toRescopedState(scopedStore.state.value),
         reducer: .init { rescopedState, rescopedAction, _ in
           isSending = true
           defer { isSending = false }
-          let task = self.root.send(fromScopedAction(fromRescopedAction(rescopedAction)))
+          guard
+            let scopedAction = fromRescopedAction(rescopedState, rescopedAction),
+            let rootAction = fromScopedAction(scopedStore.state.value, scopedAction)
+          else { return .none }
+          let task = self.root.send(rootAction)
           rescopedState = toRescopedState(scopedStore.state.value)
           if let task = task {
             return .fireAndForget { await task.cancellableValue }
@@ -717,7 +737,9 @@ public typealias StoreOf<R: ReducerProtocol> = Store<R.State, R.Action>
         }
       rescopedStore.scope = StoreScope<RootState, RootAction>(
         root: self.root,
-        fromScopedAction: { fromScopedAction(fromRescopedAction($0)) }
+        fromScopedAction: {
+          fromRescopedAction($0, $1).flatMap { fromScopedAction(scopedStore.state.value, $0) }
+        }
       )
       return rescopedStore
     }

Sources/ComposableArchitecture/SwiftUI/IfLetStore.swift

@@ -57,10 +57,12 @@ public struct IfLetStore<State, Action, Content: View>: View {
       if var state = viewStore.state {
         return ViewBuilder.buildEither(
           first: ifContent(
-            store.scope {
-              state = $0 ?? state
-              return state
-            }
+            store
+              .filter { state, _ in state == nil ? !BindingLocal.isActive : true }
+              .scope {
+                state = $0 ?? state
+                return state
+              }
           )
         )
       } else {
@@ -84,10 +86,12 @@ public struct IfLetStore<State, Action, Content: View>: View {
     self.content = { viewStore in
       if var state = viewStore.state {
         return ifContent(
-          store.scope {
-            state = $0 ?? state
-            return state
-          }
+          store
+            .filter { state, _ in state == nil ? !BindingLocal.isActive : true }
+            .scope {
+              state = $0 ?? state
+              return state
+            }
         )
       } else {
         return nil

Sources/ComposableArchitecture/ViewStore.swift

@@ -447,6 +447,7 @@ public final class ViewStore<ViewState, ViewAction>: ObservableObject {
       }
     }
   }
+
   /// Derives a binding from the store that prevents direct writes to state and instead sends
   /// actions to the store.
   ///
@@ -577,7 +578,11 @@ public final class ViewStore<ViewState, ViewAction>: ObservableObject {
     send action: HashableWrapper<(Value) -> ViewAction>
   ) -> Value {
     get { state.rawValue(self.state) }
-    set { self.send(action.rawValue(newValue)) }
+    set {
+      BindingLocal.$isActive.withValue(true) {
+        self.send(action.rawValue(newValue))
+      }
+    }
   }
 }
 
@@ -767,3 +772,7 @@ private struct HashableWrapper<Value>: Hashable {
   static func == (lhs: Self, rhs: Self) -> Bool { false }
   func hash(into hasher: inout Hasher) {}
 }
+
+enum BindingLocal {
+  @TaskLocal static var isActive = false
+}

Tests/ComposableArchitectureTests/BindingLocalTests.swift

@@ -0,0 +1,40 @@
+#if DEBUG
+  import XCTest
+
+  @testable import ComposableArchitecture
+
+  @MainActor
+  final class BindingLocalTests: XCTestCase {
+    public func testBindingLocalIsActive() {
+      XCTAssertFalse(BindingLocal.isActive)
+
+      struct MyReducer: ReducerProtocol {
+        struct State: Equatable {
+          var text = ""
+        }
+
+        enum Action: Equatable {
+          case textChanged(String)
+        }
+
+        func reduce(into state: inout State, action: Action) -> EffectTask<Action> {
+          switch action {
+          case let .textChanged(text):
+            state.text = text
+            return .none
+          }
+        }
+      }
+
+      let store = Store(initialState: MyReducer.State(), reducer: MyReducer())
+      let viewStore = ViewStore(store, observe: { $0 })
+
+      let binding = viewStore.binding(get: \.text) { text in
+        XCTAssertTrue(BindingLocal.isActive)
+        return .textChanged(text)
+      }
+      binding.wrappedValue = "Hello!"
+      XCTAssertEqual(viewStore.text, "Hello!")
+    }
+  }
+#endif

Tests/ComposableArchitectureTests/StoreTests.swift

@@ -559,4 +559,21 @@ final class StoreTests: XCTestCase {
 
     XCTAssertEqual(viewStore.state, UUID(uuidString: "deadbeef-dead-beef-dead-beefdeadbeef")!)
   }
+
+  func testFilter() {
+    let store = Store<Int?, Void>(initialState: nil, reducer: EmptyReducer())
+      .filter { state, _ in state != nil }
+
+    let viewStore = ViewStore(store)
+    var count = 0
+    viewStore.publisher
+      .sink { _ in count += 1 }
+      .store(in: &self.cancellables)
+
+    XCTAssertEqual(count, 1)
+    viewStore.send(())
+    XCTAssertEqual(count, 1)
+    viewStore.send(())
+    XCTAssertEqual(count, 1)
+  }
 }