Skip to content

Commit

Permalink
IfLetStore: ignore view store binding writes to nil state (pointf…
Browse files Browse the repository at this point in the history 
…reeco#1879)

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

* swift-format

* wip

* add test for filter

---------

Co-authored-by: Brandon Williams <mbrandonw@hey.com>
  • Loading branch information
@stephencelis @mbrandonw
stephencelis and mbrandonw authored Jan 27, 2023
1 parent 1a168e2 commit 98af2ad
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 28 deletions.
60 changes: 41 additions & 19 deletions Sources/ComposableArchitecture/Store.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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>
}

Expand All @@ -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>(
Expand All @@ -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>
}

Expand All @@ -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
Expand All @@ -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 }
Expand All @@ -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
}
Expand Down
20 changes: 12 additions & 8 deletions Sources/ComposableArchitecture/SwiftUI/IfLetStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
11 changes: 10 additions & 1 deletion Sources/ComposableArchitecture/ViewStore.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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))
}
}
}
}

Expand Down Expand Up @@ -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
}
40 changes: 40 additions & 0 deletions Tests/ComposableArchitectureTests/BindingLocalTests.swift
View file
Original file line number Diff line number Diff line change
@@ -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
17 changes: 17 additions & 0 deletions Tests/ComposableArchitectureTests/StoreTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 98af2ad

Please sign in to comment.