Skip to content

Equatable sinks prototype #254

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

Draft
wants to merge 3 commits into
base: tomb/swiftui-testbed
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion Samples/SwiftUITestbed/Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
window = UIWindow(frame: UIScreen.main.bounds)
window?.rootViewController = WorkflowHostingController(
workflow: RootWorkflow(close: nil)
workflow: RootWorkflow()
.mapRendering(MarketRootScreen.init)
.mapRendering(ModalHostContainer.init)
)
Expand Down
38 changes: 25 additions & 13 deletions Samples/SwiftUITestbed/Sources/MainScreen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,19 @@ import MarketUI
import MarketWorkflowUI
import ViewEnvironment

struct MainScreen: MarketScreen {
struct MainScreen: MarketScreen, Equatable {
enum Field: Hashable {
case title
}

@FocusState var focusedField: Field?

let title: String
let didChangeTitle: (String) -> Void

let canClose: Bool
let allCapsToggleIsOn: Bool
let allCapsToggleIsEnabled: Bool
let didChangeAllCapsToggle: (Bool) -> Void

let didTapPushScreen: () -> Void
let didTapPresentScreen: () -> Void

let didTapClose: (() -> Void)?
let sink: StableSink<Action>

func element(
in context: MarketWorkflowUI.MarketScreenContext,
Expand All @@ -56,7 +51,7 @@ struct MainScreen: MarketScreen {
style: styles.fields.textField,
label: "Text",
text: title,
onChange: didChangeTitle,
onChange: { sink.send(.changeTitle($0)) },
onReturn: { _ in focusedField = nil }
)
.focused(when: $focusedField, equals: .title)
Expand All @@ -77,7 +72,7 @@ struct MainScreen: MarketScreen {
isOn: allCapsToggleIsOn,
isEnabled: allCapsToggleIsEnabled,
accessibilityLabel: "is all caps",
onChange: didChangeAllCapsToggle
onChange: { sink.send(.changeAllCaps($0)) }
)
}

Expand All @@ -91,13 +86,13 @@ struct MainScreen: MarketScreen {
MarketButton(
style: styles.button(rank: .secondary),
text: "Push Screen",
onTap: didTapPushScreen
onTap: sink.closure(.pushScreen)
)

MarketButton(
style: styles.button(rank: .secondary),
text: "Present Screen",
onTap: didTapPresentScreen
onTap: sink.closure(.presentScreen)
)
}
}
Expand All @@ -108,9 +103,26 @@ extension MainScreen: MarketBackStackContentScreen {
func backStackItem(in environment: ViewEnvironment) -> MarketUI.MarketNavigationItem {
MarketNavigationItem(
title: .text(.init(regular: title)),
backButton: didTapClose.map { .close(onTap: $0) } ?? .automatic()
backButton: canClose ? .close(onTap: sink.closure(.close)) : .automatic()
)
}

var backStackIdentifier: AnyHashable? { nil }
}

extension MainScreen {
enum Action {
case pushScreen
case presentScreen
case changeTitle(String)
case changeAllCaps(Bool)
case close
}
}

// I guess this could be upstreamed to Blueprint
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah, I think I've seen this go by in POS

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just now noticing that SwiftUI's FocusState is not Equatable. That makes it harder for Views to be Equatable, but I presume SwiftUI's internal comparison of non-Equatable View types handles it well.

extension FocusState: Equatable where Value: Equatable {
public static func == (lhs: FocusState<Value>, rhs: FocusState<Value>) -> Bool {
lhs.wrappedValue == rhs.wrappedValue
}
}
109 changes: 77 additions & 32 deletions Samples/SwiftUITestbed/Sources/MainWorkflow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,18 @@ import MarketWorkflowUI
import Workflow

struct MainWorkflow: Workflow {
let didClose: (() -> Void)?
let canClose: Bool

enum Output {
case pushScreen
case presentScreen
case close
}

struct State {
var title: String
var isAllCaps: Bool
let trampoline = SinkTrampoline()
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like the best/only way to create a persistent identity that we can use to implement StableSink.== 👍


init(title: String) {
self.title = title
Expand All @@ -39,53 +41,96 @@ struct MainWorkflow: Workflow {
State(title: "New item")
}

enum Action: WorkflowAction {
typealias WorkflowType = MainWorkflow

case pushScreen
case presentScreen
case changeTitle(String)
case changeAllCaps(Bool)

func apply(toState state: inout WorkflowType.State) -> WorkflowType.Output? {
switch self {
case .pushScreen:
return .pushScreen
case .presentScreen:
return .presentScreen
case .changeTitle(let newValue):
state.title = newValue
state.isAllCaps = newValue.isAllCaps
case .changeAllCaps(let isAllCaps):
state.isAllCaps = isAllCaps
state.title = isAllCaps ? state.title.uppercased() : state.title.lowercased()
}
return nil
}
}

typealias Rendering = MainScreen
typealias Action = MainScreen.Action

func render(state: State, context: RenderContext<Self>) -> Rendering {
let sink = context.makeSink(of: Action.self)
let sink = state.trampoline.makeSink(of: Action.self, with: context)

return MainScreen(
title: state.title,
didChangeTitle: { sink.send(.changeTitle($0)) },
canClose: canClose,
allCapsToggleIsOn: state.isAllCaps,
allCapsToggleIsEnabled: !state.title.isEmpty,
didChangeAllCapsToggle: { sink.send(.changeAllCaps($0)) },
didTapPushScreen: { sink.send(.pushScreen) },
didTapPresentScreen: { sink.send(.presentScreen) },
didTapClose: didClose
sink: sink
)
}
}

extension MainScreen.Action: WorkflowAction {
typealias WorkflowType = MainWorkflow

func apply(toState state: inout WorkflowType.State) -> WorkflowType.Output? {
switch self {
case .pushScreen:
return .pushScreen
case .presentScreen:
return .presentScreen
case .changeTitle(let newValue):
state.title = newValue
state.isAllCaps = newValue.isAllCaps
case .changeAllCaps(let isAllCaps):
state.isAllCaps = isAllCaps
state.title = isAllCaps ? state.title.uppercased() : state.title.lowercased()
case .close:
return .close
}
return nil
}
}

private extension String {
var isAllCaps: Bool {
allSatisfy { character in
character.isUppercase || !character.isCased
}
}
}

class SinkTrampoline: Equatable {
private var sinks: [ObjectIdentifier: Any] = [:]

func makeSink<Action, WorkflowType>(
of actionType: Action.Type,
with context: RenderContext<WorkflowType>
) -> StableSink<Action> where Action: WorkflowAction, Action.WorkflowType == WorkflowType {
let sink = context.makeSink(of: actionType)

sinks[ObjectIdentifier(actionType)] = sink

return StableSink(trampoline: self)
Comment on lines +97 to +101
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we could pass sink into StableSink.init here and implement StableSink.send using that reference, allowing us to delete sinks, bounce, and destination. That would also avoid retaining any Workflow.Sink that is no longer referenced by any StableSink

Maintaining sinks does add some safety (against this, I think?) by ensuring we use the latest Workflow.Sink that the RenderContext has given us for a given Action type. But not total safety, because we're not evicting from sinks every sink that wasn't remade during the last render.

Is the need for that safety greater when the Rendering holds StableSinks rather than functions closing over Workflow.Sinks?

}

func bounce<Action>(action: Action) {
let sink = destination(for: Action.self)
sink.send(action)
}

private func destination<Action>(for actionType: Action.Type) -> Sink<Action> {
if let pipe = sinks[ObjectIdentifier(actionType)] {
return pipe as! Sink<Action>
}
fatalError("bad plumbing")
}

static func == (lhs: SinkTrampoline, rhs: SinkTrampoline) -> Bool {
lhs === rhs
}
}

struct StableSink<Action>: Equatable {
private var trampoline: SinkTrampoline

init(trampoline: SinkTrampoline) {
self.trampoline = trampoline
}

func send(_ action: Action) {
trampoline.bounce(action: action)
}

// sugar instead of writing { sink.send($0) }
func closure(_ action: Action) -> () -> Void {
{ send(action) }
}
}
18 changes: 13 additions & 5 deletions Samples/SwiftUITestbed/Sources/RootWorkflow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import Workflow
import WorkflowUI

struct RootWorkflow: Workflow {
let close: (() -> Void)?

typealias Output = Never
enum Output {
case close
}
Comment on lines -22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

We could upstream deed364 to #240. Seems cleaner and more conventional than what I was doing there!


struct State {
var backStack: BackStack
Expand Down Expand Up @@ -57,6 +57,8 @@ struct RootWorkflow: Workflow {
state.backStack.other.append(.main())
case .main(.presentScreen):
state.isPresentingModal = true
case .main(.close):
return .close
case .popScreen:
state.backStack.other.removeLast()
case .dismissScreen:
Expand All @@ -75,7 +77,7 @@ struct RootWorkflow: Workflow {
func rendering(_ screen: State.Screen, isRoot: Bool) -> AnyMarketBackStackContentScreen {
switch screen {
case .main(let id):
return MainWorkflow(didClose: isRoot ? close : nil)
return MainWorkflow(canClose: isRoot)
.mapOutput(Action.main)
.mapRendering(AnyMarketBackStackContentScreen.init)
.rendered(in: context, key: id.uuidString)
Expand All @@ -96,7 +98,13 @@ struct RootWorkflow: Workflow {
base: backStack,
modals: {
guard state.isPresentingModal else { return [] }
let screen = RootWorkflow(close: { sink.send(.dismissScreen) })
let screen = RootWorkflow()
.mapOutput { output in
switch output {
case .close:
return Action.dismissScreen
}
}
.rendered(in: context)
.asAnyScreen()
let modal = Modal(
Expand Down