Skip to content

Commit

Permalink
Add ifLet SwiftUI integration warning (#3089)
Browse files Browse the repository at this point in the history
* Add `ifLet` SwiftUI integration warning

We recently added a warning when we detect that `forEach` wasn't
integrated with a reducer when path elements are pushed or popped from
the stack, and I realized that we could do similarly for `ifLet` during
dismissal.

* docc
  • Loading branch information
stephencelis committed May 14, 2024
1 parent 57526d8 commit e37353e
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ struct InventoryView: View {
```

> Note: We use SwiftUI's `@Bindable` property wrapper to produce a binding to a store, which can be
> further scoped using ``SwiftUI/Binding/scope(state:action:)-4mj4d``.
> further scoped using ``SwiftUI/Binding/scope(state:action:fileID:line:)``.
With those few steps completed the domains and views of the parent and child features are now
integrated together, and when the `addItem` state flips to a non-`nil` value the sheet will be
Expand Down Expand Up @@ -269,8 +269,8 @@ struct InventoryView: View {
}
```

And then in the `body` of the view you can use the ``SwiftUI/Binding/scope(state:action:)-4mj4d``
operator to derive bindings from `$store`:
And then in the `body` of the view you can use the
``SwiftUI/Binding/scope(state:action:fileID:line:)`` operator to derive bindings from `$store`:

```swift
var body: some View {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

### Scoping store bindings

- ``SwiftUI/Binding/scope(state:action:)-4mj4d``
- ``SwiftUI/Binding/scope(state:action:fileID:line:)``
- ``SwiftUI/Binding/scope(state:action:)-35r82``

### Combine integration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ designed with SwiftUI in mind, and comes with many powerful tools to integrate i

### Presentation

- ``SwiftUI/Binding/scope(state:action:)-4mj4d``
- ``SwiftUI/Binding/scope(state:action:fileID:line:)``

### Navigation stacks and links

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@

Luckily the library comes with the tools necessary. Just as there is a scoping operation on
stores for focusing on sub-domains of a parent domain, there is also a scope on _bindings_ of
stores for doing the same: ``SwiftUI/Binding/scope(state:action:)-4mj4d``. This tool can be
used to derive a binding that is appropriate to pass to `sheet(item:)`.
stores for doing the same: ``SwiftUI/Binding/scope(state:action:fileID:line:)``. This tool can
be used to derive a binding that is appropriate to pass to `sheet(item:)`.

@Step {
Since we want to derive bindings from the store we need to decorate the property in the view
Expand All @@ -127,8 +127,8 @@
}

@Step {
Use the ``SwiftUI/Binding/scope(state:action:)-4mj4d`` operator on `$store` to focus the
binding to the presentation domain of the `SyncUpForm`. The `sheet(item:)` modifier will
Use the ``SwiftUI/Binding/scope(state:action:fileID:line:)`` operator on `$store` to focus
the binding to the presentation domain of the `SyncUpForm`. The `sheet(item:)` modifier will
hand the trailing closure a `StoreOf<SyncUpForm>`, and that is exactly what can be handed to
the `SyncUpFormView`.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@

@Step {
At the very bottom of the view use the `sheet(item:)` modifier by deriving a binding to the
`SyncUpForm` domain using ``SwiftUI/Binding/scope(state:action:)-4mj4d``.
`SyncUpForm` domain using ``SwiftUI/Binding/scope(state:action:fileID:line:)``.

@Code(name: "SyncUpDetail.swift", file: EditingAndDeletingSyncUp-01-code-0006.swift)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ import SwiftUI
extension SwiftUI.Bindable {
/// Derives a binding to a store focused on ``StackState`` and ``StackAction``.
///
/// See ``SwiftUI/Binding/scope(state:action:)-4mj4d`` defined on `Binding` for more
/// See ``SwiftUI/Binding/scope(state:action:fileID:line:)`` defined on `Binding` for more
/// information.
public func scope<State: ObservableState, Action, ElementState, ElementAction>(
state: KeyPath<State, StackState<ElementState>>,
Expand All @@ -88,7 +88,7 @@ import SwiftUI
extension Perception.Bindable {
/// Derives a binding to a store focused on ``StackState`` and ``StackAction``.
///
/// See ``SwiftUI/Binding/scope(state:action:)-4mj4d`` defined on `Binding` for more
/// See ``SwiftUI/Binding/scope(state:action:fileID:line:)`` defined on `Binding` for more
/// information.
public func scope<State: ObservableState, Action, ElementState, ElementAction>(
state: KeyPath<State, StackState<ElementState>>,
Expand Down
67 changes: 59 additions & 8 deletions Sources/ComposableArchitecture/Observation/Store+Observation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,18 @@
/// - Returns: A binding of an optional child store.
public func scope<State: ObservableState, Action, ChildState, ChildAction>(
state: KeyPath<State, ChildState?>,
action: CaseKeyPath<Action, PresentationAction<ChildAction>>
action: CaseKeyPath<Action, PresentationAction<ChildAction>>,
fileID: StaticString = #fileID,
line: UInt = #line
) -> Binding<Store<ChildState, ChildAction>?>
where Value == Store<State, Action> {
self[state: state, action: action]
self[
state: state,
action: action,
isInViewBody: _isInPerceptionTracking,
fileID: "\(fileID)",
line: line
]
}
}

Expand Down Expand Up @@ -209,10 +217,18 @@
/// - Returns: A binding of an optional child store.
public func scope<State: ObservableState, Action, ChildState, ChildAction>(
state: KeyPath<State, ChildState?>,
action: CaseKeyPath<Action, PresentationAction<ChildAction>>
action: CaseKeyPath<Action, PresentationAction<ChildAction>>,
fileID: StaticString = #fileID,
line: UInt = #line
) -> Binding<Store<ChildState, ChildAction>?>
where Value == Store<State, Action> {
self[state: state, action: action]
self[
state: state,
action: action,
isInViewBody: _isInPerceptionTracking,
fileID: "\(fileID)",
line: line
]
}
}

Expand Down Expand Up @@ -269,18 +285,29 @@
/// - Returns: A binding of an optional child store.
public func scope<State: ObservableState, Action, ChildState, ChildAction>(
state: KeyPath<State, ChildState?>,
action: CaseKeyPath<Action, PresentationAction<ChildAction>>
action: CaseKeyPath<Action, PresentationAction<ChildAction>>,
fileID: StaticString = #fileID,
line: UInt = #line
) -> Binding<Store<ChildState, ChildAction>?>
where Value == Store<State, Action> {
self[state: state, action: action]
self[
state: state,
action: action,
isInViewBody: _isInPerceptionTracking,
fileID: "\(fileID)",
line: line
]
}
}

extension Store where State: ObservableState {
fileprivate subscript<ChildState, ChildAction>(
@_spi(Internals)
public subscript<ChildState, ChildAction>(
state state: KeyPath<State, ChildState?>,
action action: CaseKeyPath<Action, PresentationAction<ChildAction>>,
isInViewBody isInViewBody: Bool = _isInPerceptionTracking
isInViewBody isInViewBody: Bool,
fileID fileID: String,
line line: UInt
) -> Store<ChildState, ChildAction>? {
get {
#if DEBUG && !os(visionOS)
Expand All @@ -294,6 +321,30 @@
set {
if newValue == nil, self.state[keyPath: state] != nil, !self._isInvalidated() {
self.send(action(.dismiss))
if self.state[keyPath: state] != nil {
runtimeWarn(
"""
SwiftUI dismissed a view through a binding at "\(fileID):\(line)", but the store \
destination wasn't set to "nil".
This usually means an "ifLet" has not been integrated with the reducer powering the \
store, and this reducer is responsible for handling presentation actions.
To fix this, ensure that "ifLet" is invoked from the reducer's "body":
Reduce { state, action in
// ...
}
.ifLet(\\.destination, action: \\.destination) {
Destination()
}
And ensure that every parent reducer is integrated into the root reducer that powers \
the store.
"""
)
return
}
}
}
}
Expand Down
51 changes: 51 additions & 0 deletions Tests/ComposableArchitectureTests/RuntimeWarningTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,57 @@
"""
}
}

@Reducer
struct TestStoreDestination_NotIntegrated {
@Reducer
struct Destination {}
@ObservableState
struct State: Equatable {
@Presents var destination: Destination.State?
}
enum Action {
case destination(PresentationAction<Destination.Action>)
}
}
@MainActor
func testStoreDestination_NotIntegrated() {
let store = Store(
initialState: TestStoreDestination_NotIntegrated.State(destination: .init())
) {
TestStoreDestination_NotIntegrated()
}

XCTExpectFailure {
store[
state: \.destination,
action: \.destination,
isInViewBody: false,
fileID: "file.swift",
line: 1
] = nil
} issueMatcher: {
$0.compactDescription == """
SwiftUI dismissed a view through a binding at "file.swift:1", but the store \
destination wasn't set to "nil".
This usually means an "ifLet" has not been integrated with the reducer powering the \
store, and this reducer is responsible for handling presentation actions.
To fix this, ensure that "ifLet" is invoked from the reducer's "body":
Reduce { state, action in
// ...
}
.ifLet(\\.destination, action: \\.destination) {
Destination()
}
And ensure that every parent reducer is integrated into the root reducer that powers \
the store.
"""
}
}
#endif
}
#endif

0 comments on commit e37353e

Please sign in to comment.