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

[RFC] [DNM] SideEffect as Type #1182

Closed
wants to merge 4 commits into from
Closed
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
32 changes: 9 additions & 23 deletions swift/Samples/SampleApp/Sources/DemoWorkflow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ struct DemoWorkflow: Workflow {

extension DemoWorkflow {
struct State {
fileprivate var signal: TimerSignal
var colorState: ColorState
var loadingState: LoadingState
var subscriptionState: SubscriptionState
Expand All @@ -54,7 +53,6 @@ extension DemoWorkflow {

func makeInitialState() -> DemoWorkflow.State {
return State(
signal: TimerSignal(),
colorState: .red,
loadingState: .idle(title: "Not Loaded"),
subscriptionState: .not
Expand Down Expand Up @@ -172,10 +170,16 @@ extension DemoWorkflow {
case .not:
subscribeTitle = "Subscribe"
case .subscribing:
// Subscribe to the timer signal, simulating the title being tapped.
context.awaitResult(for: state.signal.signal.asWorker(key: "Timer")) { _ -> Action in
.titleButtonTapped
context.run(SideEffectPerformer<Action>(key: "Timer") { sink, lifetime in
let timer = Timer.scheduledTimer(withTimeInterval: 1, repeats: true) { timer in
sink.send(.titleButtonTapped)
}

lifetime.onEnded {
timer.invalidate()
}
}
)
subscribeTitle = "Stop"
}

Expand All @@ -200,21 +204,3 @@ extension DemoWorkflow {
)
}
}

private class TimerSignal {
let signal: Signal<Void, Never>
let observer: Signal<Void, Never>.Observer
let timer: Timer

init() {
let (signal, observer) = Signal<Void, Never>.pipe()

let timer = Timer.scheduledTimer(withTimeInterval: 1.0, repeats: true) { [weak observer] _ in
observer?.send(value: ())
}

self.signal = signal
self.observer = observer
self.timer = timer
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -218,15 +218,17 @@ extension AuthenticationWorkflow {
authenticationService: authenticationService,
intermediateToken: intermediateSession,
twoFactorCode: twoFactorCode
))
)
)

backStackItems.append(twoFactorScreen(error: nil, intermediateSession: intermediateSession, sink: sink))
modals.append(ModalContainerScreenModal(screen: AnyScreen(LoadingScreen()), style: .fullScreen, key: "", animated: false))
}
return AlertContainerScreen(
baseScreen: ModalContainerScreen(
baseScreen: BackStackScreen(
items: backStackItems),
items: backStackItems
),
modals: modals
),
alert: alert
Expand Down Expand Up @@ -259,7 +261,8 @@ extension AuthenticationWorkflow {
handler: {
sink.send(.back)
}
))))
))
))
)
}
}
4 changes: 4 additions & 0 deletions swift/Workflow/Sources/Lifetime.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
import Foundation

public final class Lifetime {
public init(action: @escaping () -> Void) {
onEnded(action)
}

public func onEnded(_ action: @escaping () -> Void) {
assert(!hasEnded, "Lifetime used after being ended.")
onEndedActions.append(action)
Expand Down
8 changes: 4 additions & 4 deletions swift/Workflow/Sources/RenderContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public class RenderContext<WorkflowType: Workflow>: RenderContextType {
fatalError()
}

public func runSideEffect(key: AnyHashable, action: (Lifetime) -> Void) {
public func run<S: SideEffect>(_ sideEffect: S) {
fatalError()
}

Expand Down Expand Up @@ -107,9 +107,9 @@ public class RenderContext<WorkflowType: Workflow>: RenderContextType {
return implementation.makeSink(of: actionType)
}

override func runSideEffect(key: AnyHashable, action: (_ lifetime: Lifetime) -> Void) {
override func run<S: SideEffect>(_ sideEffect: S) where S.Action.WorkflowType == WorkflowType {
assertStillValid()
implementation.runSideEffect(key: key, action: action)
implementation.run(sideEffect)
}

override func awaitResult<W, Action>(for worker: W, outputMap: @escaping (W.Output) -> Action) where W: Worker, Action: WorkflowAction, WorkflowType == Action.WorkflowType {
Expand All @@ -132,7 +132,7 @@ internal protocol RenderContextType: AnyObject {

func awaitResult<W, Action>(for worker: W, outputMap: @escaping (W.Output) -> Action) where W: Worker, Action: WorkflowAction, Action.WorkflowType == WorkflowType

func runSideEffect(key: AnyHashable, action: (_ lifetime: Lifetime) -> Void)
func run<S: SideEffect>(_ sideEffect: S) where S.Action.WorkflowType == WorkflowType
}

extension RenderContext {
Expand Down
21 changes: 21 additions & 0 deletions swift/Workflow/Sources/SideEffect.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright 2020 Square Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

public protocol SideEffect: Hashable {
associatedtype Action: WorkflowAction

func run(sink: Sink<Action>) -> Lifetime
}
Comment on lines +17 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

Squinting, this looks a lot like Worker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It pretty much is - except we're moving the ReactiveSwift specific stuff out of Worker. So instead of it returning SignalProducer<Output>, we'll be passing in a Sink and use an internal concept for Lifetime instead.

Oh, we're also adding a hash value to it.

41 changes: 41 additions & 0 deletions swift/Workflow/Sources/SideEffectPerformer.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2020 Square Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import Foundation

public struct SideEffectPerformer<Action: WorkflowAction>: SideEffect {
let key: AnyHashable
let action: (Sink<Action>, Lifetime) -> Void

public init(key: AnyHashable, action: @escaping (Sink<Action>, Lifetime) -> Void) {
self.key = key
self.action = action
}

public func run(sink: Sink<Action>) -> Lifetime {
let lifetime = Lifetime {}
action(sink, lifetime)
return lifetime
}

public static func == (lhs: SideEffectPerformer, rhs: SideEffectPerformer) -> Bool {
lhs.key == rhs.key
}

public func hash(into hasher: inout Hasher) {
hasher.combine(key)
}
}
18 changes: 11 additions & 7 deletions swift/Workflow/Sources/SubtreeManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,13 @@ extension WorkflowNode.SubtreeManager {
}
}

func runSideEffect(key: AnyHashable, action: (Lifetime) -> Void) {
if let existingSideEffect = originalSideEffectLifetimes[key] {
usedSideEffectLifetimes[key] = existingSideEffect
func run<S: SideEffect>(_ sideEffect: S) where S.Action.WorkflowType == WorkflowType {
let sink = makeSink(of: S.Action.self)
if let existingSideEffect = originalSideEffectLifetimes[sideEffect] {
usedSideEffectLifetimes[sideEffect] = existingSideEffect
} else {
let sideEffectLifetime = SideEffectLifetime()
action(sideEffectLifetime.lifetime)
usedSideEffectLifetimes[key] = sideEffectLifetime
let lifetime = sideEffect.run(sink: sink)
usedSideEffectLifetimes[sideEffect] = SideEffectLifetime(lifetime: lifetime)
}
}
}
Expand Down Expand Up @@ -553,7 +553,11 @@ extension WorkflowNode.SubtreeManager {
fileprivate let lifetime: Lifetime

fileprivate init() {
self.lifetime = Lifetime()
self.lifetime = Lifetime {}
}

fileprivate init(lifetime: Lifetime) {
self.lifetime = lifetime
}

deinit {
Expand Down
31 changes: 23 additions & 8 deletions swift/Workflow/Tests/SubtreeManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import ReactiveSwift
import class Workflow.Lifetime
import XCTest
@testable import Workflow

Expand Down Expand Up @@ -304,7 +305,7 @@ final class SubtreeManagerTests: XCTestCase {
XCTAssertTrue(manager.sideEffectLifetimes.isEmpty)

_ = manager.render { context -> TestViewModel in
context.runSideEffect(key: "helloWorld") { _ in }
context.run(TestSideEffect(lifetime: Lifetime()))
return context.render(
workflow: TestWorkflow(),
key: "",
Expand All @@ -316,7 +317,7 @@ final class SubtreeManagerTests: XCTestCase {
let sideEffectKey = manager.sideEffectLifetimes.values.first!

_ = manager.render { context -> TestViewModel in
context.runSideEffect(key: "helloWorld") { _ in }
context.run(TestSideEffect(lifetime: Lifetime()))
return context.render(
workflow: TestWorkflow(),
key: "",
Expand All @@ -334,13 +335,13 @@ final class SubtreeManagerTests: XCTestCase {

let lifetimeEndedExpectation = expectation(description: "Lifetime Ended Expectations")
_ = manager.render { context -> TestViewModel in
context.runSideEffect(key: "helloWorld") { lifetime in
lifetime.onEnded {
// Capturing `lifetime` to make sure a retain-cycle will still trigger the `onEnded` block
print("\(lifetime)")
lifetimeEndedExpectation.fulfill()
}
let lifetime = Lifetime()
lifetime.onEnded {
// Capturing `lifetime` to make sure a retain-cycle will still trigger the `onEnded` block
print("\(lifetime)")
lifetimeEndedExpectation.fulfill()
}
context.run(TestSideEffect(lifetime: lifetime))
return context.render(
workflow: TestWorkflow(),
key: "",
Expand All @@ -363,6 +364,20 @@ final class SubtreeManagerTests: XCTestCase {
}
}

private struct TestSideEffect: SideEffect, Hashable {
let key: String = "Test Side Effect"
let lifetime: Lifetime
func run() -> Lifetime {
lifetime
}

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

func hash(into hasher: inout Hasher) {}
}

private struct TestViewModel {
var onTap: () -> Void
var onToggle: () -> Void
Expand Down
8 changes: 4 additions & 4 deletions swift/WorkflowTesting/Sources/RenderExpectations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,11 @@ public struct ExpectedWorker {
}
}

public struct ExpectedSideEffect {
let key: AnyHashable
public struct ExpectedSideEffect: Equatable {
private let key: AnyHashable

public init(key: AnyHashable) {
self.key = key
public init<S: SideEffect>(sideEffect: S) {
self.key = sideEffect
}
}

Expand Down
11 changes: 5 additions & 6 deletions swift/WorkflowTesting/Sources/WorkflowRenderTester.swift
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,10 @@
}
}

func runSideEffect(key: AnyHashable, action: (Lifetime) -> Void) {
guard let sideEffectIndex = expectations.expectedSideEffects.firstIndex(where: { (expectedSideEffect) -> Bool in
expectedSideEffect.key == key
}) else {
XCTFail("Unexpected side-effect during render \(key)", file: file, line: line)
func run<S>(_ sideEffect: S) where S: SideEffect {
guard let sideEffectIndex = expectations.expectedSideEffects.firstIndex(of: ExpectedSideEffect(sideEffect: sideEffect))
else {
XCTFail("Unexpected side-effect during render \(sideEffect)", file: file, line: line)
return
}

Expand Down Expand Up @@ -344,7 +343,7 @@

if !expectations.expectedSideEffects.isEmpty {
for expectedSideEffect in expectations.expectedSideEffects {
XCTFail("Expected side-effect with key: \(expectedSideEffect.key)", file: file, line: line)
XCTFail("Expected side-effect with key: \(expectedSideEffect)", file: file, line: line)
}
}
}
Expand Down
10 changes: 7 additions & 3 deletions swift/WorkflowTesting/Tests/WorkflowRenderTesterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import ReactiveSwift
import Workflow
import class Workflow.Lifetime
import WorkflowTesting
import XCTest

Expand Down Expand Up @@ -85,7 +86,7 @@ final class WorkflowRenderTesterTests: XCTestCase {

renderTester.render(
with: RenderExpectations(expectedSideEffects: [
ExpectedSideEffect(key: TestSideEffectKey()),
ExpectedSideEffect(sideEffect: TestSideEffect()),
]),
assertions: { _ in }
)
Expand Down Expand Up @@ -324,8 +325,11 @@ private struct OutputWorkflow: Workflow {
}
}

private struct TestSideEffectKey: Hashable {
struct TestSideEffect: SideEffect {
let key: String = "Test Side Effect"
func run() -> Lifetime {
Lifetime {}
}
}

private struct SideEffectWorkflow: Workflow {
Expand All @@ -334,7 +338,7 @@ private struct SideEffectWorkflow: Workflow {
typealias Rendering = TestScreen

func render(state: State, context: RenderContext<SideEffectWorkflow>) -> TestScreen {
context.runSideEffect(key: TestSideEffectKey()) { _ in }
context.run(sideEffect: TestSideEffect())

return TestScreen(text: "value", tapped: {})
}
Expand Down