Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ifLet SwiftUI integration warning #3089

Merged
merged 2 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Comment on lines +155 to +157
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's probably some DocC references to fix after this...

) -> 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