Skip to content

Commit

Permalink
Runtime warn when stack integration is missing from store's reducer (#…
Browse files Browse the repository at this point in the history
…3048)

* Runtime warn when reducer is missing stack integration

If SwiftUI writes to a binding and the state is invalid (it is not
inserting or removing elements from the stack), then the most likely
cause is a missing `forEach` in the reducer (or a missing integration in
some parent reducer).

This commit adds a runtime warning to detect this issue.

* wip
  • Loading branch information
stephencelis authored May 6, 2024
1 parent 096c554 commit c76914e
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,15 @@ import SwiftUI
public init<State, Action, Destination: View, R>(
path: Binding<Store<StackState<State>, StackAction<State, Action>>>,
root: () -> R,
@ViewBuilder destination: @escaping (Store<State, Action>) -> Destination
@ViewBuilder destination: @escaping (Store<State, Action>) -> Destination,
fileID: StaticString = #fileID,
line: UInt = #line
)
where
Data == StackState<State>.PathView,
Root == ModifiedContent<R, _NavigationDestinationViewModifier<State, Action, Destination>>
{
self.init(path: path[]) {
self.init(path: path[fileID: "\(fileID)", line: line]) {
root()
.modifier(
_NavigationDestinationViewModifier(store: path.wrappedValue, destination: destination)
Expand Down Expand Up @@ -285,11 +287,40 @@ import SwiftUI
}

extension Store {
fileprivate subscript<ElementState, ElementAction>() -> StackState<ElementState>.PathView
@_spi(Internals) public subscript<ElementState, ElementAction>(
fileID fileID: String,
line line: UInt
) -> StackState<ElementState>.PathView
where State == StackState<ElementState>, Action == StackAction<ElementState, ElementAction> {
get { self.currentState.path }
set {
let newCount = newValue.count
guard newCount != self.currentState.count else {
runtimeWarn(
"""
SwiftUI wrote to a "NavigationStack" binding at "\(fileID):\(line)" with a path that \
has the same number of elements that already exist in the store. SwiftUI should only \
write to this binding with a path that has pushed a new element onto the stack, or \
popped one or more elements from the stack.
This usually means the "forEach" has not been integrated with the reducer powering the \
store, and this reducer is responsible for handling stack actions.
To fix this, ensure that "forEach" is invoked from the reducer's "body":
Reduce { state, action in
// ...
}
.forEach(\\.path, action: \\.path) {
Path()
}
And ensure that every parent reducer is integrated into the root reducer that powers \
the store.
"""
)
return
}
if newCount > self.currentState.count, let component = newValue.last {
self.send(.push(id: component.id, state: component.element))
} else {
Expand Down
2 changes: 1 addition & 1 deletion Sources/ComposableArchitecture/SwiftUI/Binding.swift
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,7 @@ extension WithViewStore where ViewState: Equatable, Content: View {
customDump(self.value, to: &value, maxDepth: 0)
runtimeWarn(
"""
A binding action sent from a view store \
A binding action sent from a store \
\(self.context == .bindingState ? "for binding state defined " : "")at \
"\(self.fileID):\(self.line)" was not handled. …
Expand Down
1 change: 0 additions & 1 deletion Sources/ComposableArchitecture/TestStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,6 @@ public final class TestStore<State, Action> {
self.timeout = 1 * NSEC_PER_SEC
self.sharedChangeTracker = sharedChangeTracker
self.useMainSerialExecutor = true
let dismiss = self.reducer.dependencies.dismiss.dismiss
self.reducer.store = self
}

Expand Down
51 changes: 48 additions & 3 deletions Tests/ComposableArchitectureTests/RuntimeWarningTests.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#if DEBUG
import Combine
import ComposableArchitecture
@_spi(Internals) import ComposableArchitecture
import XCTest

final class RuntimeWarningTests: BaseTCATestCase {
Expand Down Expand Up @@ -193,7 +193,7 @@
ViewStore(store, observe: { $0 }).$value.wrappedValue = 42
} issueMatcher: {
$0.compactDescription == """
A binding action sent from a view store for binding state defined at \
A binding action sent from a store for binding state defined at \
"\(#fileID):\(line)" was not handled. …
Action:
Expand All @@ -219,7 +219,7 @@
ViewStore(store, observe: { $0 }).$value.wrappedValue = 42
} issueMatcher: {
$0.compactDescription == """
A binding action sent from a view store for binding state defined at \
A binding action sent from a store for binding state defined at \
"\(#fileID):\(line)" was not handled. …
Action:
Expand All @@ -229,5 +229,50 @@
"""
}
}

#if swift(>=5.9)
@Reducer
struct TestStorePath_NotIntegrated {
@ObservableState
struct State: Equatable {
var path = StackState<Int>()
}
enum Action {
case path(StackAction<Int, Void>)
}
}
@MainActor
func testStorePath_NotIntegrated() {
let store = Store(initialState: TestStorePath_NotIntegrated.State()) {
TestStorePath_NotIntegrated()
}

XCTExpectFailure {
store.scope(state: \.path, action: \.path)[fileID: "file.swift", line: 1] = .init()
} issueMatcher: {
$0.compactDescription == """
SwiftUI wrote to a "NavigationStack" binding at "file.swift:1" with a path that has \
the same number of elements that already exist in the store. SwiftUI should only write \
to this binding with a path that has pushed a new element onto the stack, or popped \
one or more elements from the stack.
This usually means the "forEach" has not been integrated with the reducer powering the \
store, and this reducer is responsible for handling stack actions.
To fix this, ensure that "forEach" is invoked from the reducer's "body":
Reduce { state, action in
// ...
}
.forEach(\\.path, action: \\.path) {
Path()
}
And ensure that every parent reducer is integrated into the root reducer that powers \
the store.
"""
}
}
#endif
}
#endif

0 comments on commit c76914e

Please sign in to comment.