-
Notifications
You must be signed in to change notification settings - Fork 1.6k
expose navigation stack helper behind SPI #3657
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
expose navigation stack helper behind SPI #3657
Conversation
|
Hi @Alex293, can you provide more information of what exactly broke? We would rather than expand the surface area of |
|
We have a type that have exactly the api of SwiftUI NavigationStack and we’d like to provide the same initializer that you provide. I went with the helper in order to limit the spi to the minimum. I’ll provide a complete example soon. |
|
Here is how we use it : import SwiftUI
import NavigationStackBackport
@_spi(Internals) import ComposableArchitecture
@available(iOS 15, *)
extension NavigationStackBackport.NavigationStack { // (1)
public init<State, Action, Destination: View, R>(
path: Binding<Store<StackState<State>, StackAction<State, Action>>>,
@ViewBuilder root: () -> R,
@ViewBuilder destination: @escaping (Store<State, Action>) -> Destination,
fileID: StaticString = #fileID,
filePath: StaticString = #filePath,
line: UInt = #line,
column: UInt = #column
)
where
Data == StackState<State>.PathView,
Root == ModifiedContent<R, _NavigationDestinationViewModifier2<State, Action, Destination>>
{
self.init(
path: path[
fileID: _HashableStaticString(rawValue: fileID),
filePath: _HashableStaticString(rawValue: filePath),
line: line,
column: column
]
) {
root()
.modifier(
_NavigationDestinationViewModifier2(
store: path.wrappedValue,
destination: destination,
fileID: fileID,
filePath: filePath,
line: line,
column: column
)
)
}
}
}
public struct _NavigationDestinationViewModifier2<
State: ObservableState, Action, Destination: View
>:
ViewModifier
{
@SwiftUI.State var store: Store<StackState<State>, StackAction<State, Action>>
fileprivate let destination: (Store<State, Action>) -> Destination
fileprivate let fileID: StaticString
fileprivate let filePath: StaticString
fileprivate let line: UInt
fileprivate let column: UInt
public func body(content: Content) -> some View {
content
.environment(\.navigationDestinationType, State.self)
.backport.navigationDestination(for: StackState<State>.Component.self) { component in // (2)
destination(store.scope(component: component))
.environment(\.navigationDestinationType, State.self)
}
}
}We basically have the same code than TCA but for another pair of NavigationStack type (1) / navigationDestination modifier (2) |
|
Also I didn't want to imply that you broke your promise, I'm merely trying to figure a path forward while keeping support for iOS 15 for now, sorry for that. |
|
Oh, sorry, I didn't take what you said as an implication that we broke a promise. I'm just describing why we typically shy away from adding more But I'm still struggling to see what exactly about the 1.19 release broke your code. Here is the full diff of 1.18 to 1.19: What about that diff broke your code? |
|
Our previous version looked exactly as your previous version @available(iOS 15, *)
public struct _NavigationDestinationViewModifier2<
State: ObservableState, Action, Destination: View
>:
ViewModifier
{
@SwiftUI.State var store: Store<StackState<State>, StackAction<State, Action>>
fileprivate let destination: (Store<State, Action>) -> Destination
fileprivate let fileID: StaticString
fileprivate let filePath: StaticString
fileprivate let line: UInt
fileprivate let column: UInt
public func body(content: Content) -> some View {
content
.environment(\.navigationDestinationType, State.self)
.backport.navigationDestination(for: StackState<State>.Component.self) { component in
var element = component.element
self
.destination(
self.store.scope(
id: self.store.id(
state:
\.[
id:component.id,
fileID:_HashableStaticString(rawValue: fileID),
filePath:_HashableStaticString(rawValue: filePath),
line:line,
column:column
],
action: \.[id:component.id]
),
state: ToState {
element = $0[id: component.id] ?? element
return element
},
action: { .element(id: component.id, action: $0) },
isInvalid: { !$0.ids.contains(component.id) }
)
)
.environment(\.navigationDestinationType, State.self)
}
}
}
extension Store where State: ObservableState {
fileprivate subscript<ElementState, ElementAction>(
state state: KeyPath<State, StackState<ElementState>>,
action action: CaseKeyPath<Action, StackAction<ElementState, ElementAction>>,
isInViewBody isInViewBody: Bool = _isInPerceptionTracking
) -> Store<StackState<ElementState>, StackAction<ElementState, ElementAction>> {
get {
#if DEBUG && !os(visionOS)
_PerceptionLocals.$isInPerceptionTracking.withValue(isInViewBody) {
self.scope(state: state, action: action)
}
#else
self.scope(state: state, action: action)
#endif
}
set {}
}
}but since the core refactor the |
stephencelis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the context! We discussed our options here and this is probably the minimal way to provide support via SPI. When it goes green we can get things merged.
As always we don't guarantee we won't break internal SPI APIs at any time (and we almost certainly will again in the future). Hopefully it won't be often, though!
|
Yay thanks a lot to both of you ! Hopefully those api shouldn’t change anytime soon and we will upgrade our minimal version support. |
Hi,
The latest release with Core refactoring broke our back port of the NavigationStack.init(path:) you provide. At first I tried to expose enough to make the same thing but it's far easier and maybe better to only expose a helper method that get the job done. What do you think ?