Skip to content

Do not propagate dependencies to effect actions. #1954

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Mar 5, 2023
Merged

Do not propagate dependencies to effect actions. #1954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cbfea41
wip
mbrandonw Mar 1, 2023
b17c986
wip
mbrandonw Mar 1, 2023
d463ac8
wip
mbrandonw Mar 1, 2023
a62e555
wip
mbrandonw Mar 1, 2023
b9fab89
wip
mbrandonw Mar 1, 2023
c20e654
wip
mbrandonw Mar 1, 2023
f50ac53
wip
mbrandonw Mar 1, 2023
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
4 changes: 2 additions & 2 deletions ComposableArchitecture.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

119 changes: 63 additions & 56 deletions Sources/ComposableArchitecture/Store.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public final class Store<State, Action> {
private let reducer: (inout State, Action) -> EffectTask<Action>
fileprivate var scope: AnyStoreScope?
#endif
var state: CurrentValueSubject<State, Never>
@_spi(Internals) public var state: CurrentValueSubject<State, Never>
Copy link
Member Author

Choose a reason for hiding this comment

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

added this so that I could more fully test the Store in complete isolation (no need to interface with ViewStore). I just wanted to be 100% we were asserting on the full behavior inside the store without outside factors.

#if DEBUG
private let mainThreadChecksEnabled: Bool
#endif
Expand Down Expand Up @@ -385,28 +385,31 @@ public final class Store<State, Action> {
var didComplete = false
let boxedTask = Box<Task<Void, Never>?>(wrappedValue: nil)
let uuid = UUID()
let effectCancellable =
let effectCancellable = withEscapedDependencies { continuation in
Copy link
Member Author

Choose a reason for hiding this comment

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

easier to review this diff with whitespace turned off.

publisher
.handleEvents(
receiveCancel: { [weak self] in
self?.threadCheck(status: .effectCompletion(action))
self?.effectCancellables[uuid] = nil
}
)
.sink(
receiveCompletion: { [weak self] _ in
self?.threadCheck(status: .effectCompletion(action))
boxedTask.wrappedValue?.cancel()
didComplete = true
self?.effectCancellables[uuid] = nil
},
receiveValue: { [weak self] effectAction in
guard let self = self else { return }
if let task = self.send(effectAction, originatingFrom: action) {
tasks.wrappedValue.append(task)
.handleEvents(
receiveCancel: { [weak self] in
self?.threadCheck(status: .effectCompletion(action))
self?.effectCancellables[uuid] = nil
}
}
)
)
.sink(
receiveCompletion: { [weak self] _ in
self?.threadCheck(status: .effectCompletion(action))
boxedTask.wrappedValue?.cancel()
didComplete = true
self?.effectCancellables[uuid] = nil
},
receiveValue: { [weak self] effectAction in
guard let self = self else { return }
if let task = continuation.yield({
self.send(effectAction, originatingFrom: action)
}) {
tasks.wrappedValue.append(task)
}
}
)
}

if !didComplete {
let task = Task<Void, Never> { @MainActor in
Expand All @@ -418,43 +421,47 @@ public final class Store<State, Action> {
self.effectCancellables[uuid] = effectCancellable
}
case let .run(priority, operation):
tasks.wrappedValue.append(
Task(priority: priority) { @MainActor in
#if DEBUG
var isCompleted = false
defer { isCompleted = true }
#endif
await operation(
EffectTask<Action>.Send {
#if DEBUG
if isCompleted {
runtimeWarn(
"""
An action was sent from a completed effect:

Action:
\(debugCaseOutput($0))

Effect returned from:
\(debugCaseOutput(action))

Avoid sending actions using the 'send' argument from 'EffectTask.run' after \
the effect has completed. This can happen if you escape the 'send' argument in \
an unstructured context.

To fix this, make sure that your 'run' closure does not return until you're \
done calling 'send'.
"""
)
withEscapedDependencies { continuation in
tasks.wrappedValue.append(
Task(priority: priority) { @MainActor in
#if DEBUG
var isCompleted = false
defer { isCompleted = true }
#endif
await operation(
EffectTask<Action>.Send { effectAction in
#if DEBUG
if isCompleted {
runtimeWarn(
"""
An action was sent from a completed effect:

Action:
\(debugCaseOutput(effectAction))

Effect returned from:
\(debugCaseOutput(action))

Avoid sending actions using the 'send' argument from 'EffectTask.run' after \
the effect has completed. This can happen if you escape the 'send' argument in \
an unstructured context.

To fix this, make sure that your 'run' closure does not return until you're \
done calling 'send'.
"""
)
}
#endif
if let task = continuation.yield({
self.send(effectAction, originatingFrom: action)
}) {
tasks.wrappedValue.append(task)
}
#endif
if let task = self.send($0, originatingFrom: action) {
tasks.wrappedValue.append(task)
}
}
)
}
)
)
}
)
}
}
}

Expand Down
135 changes: 135 additions & 0 deletions Tests/ComposableArchitectureTests/StoreTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,4 +559,139 @@ final class StoreTests: XCTestCase {

XCTAssertEqual(viewStore.state, UUID(uuidString: "deadbeef-dead-beef-dead-beefdeadbeef")!)
}

func testStoreVsTestStore() async {
struct Feature: ReducerProtocol {
struct State: Equatable {
var count = 0
}
enum Action: Equatable {
case tap
case response1(Int)
case response2(Int)
case response3(Int)
}
@Dependency(\.count) var count
func reduce(into state: inout State, action: Action) -> EffectTask<Action> {
switch action {
case .tap:
return withDependencies {
$0.count.value += 1
} operation: {
.task { .response1(self.count.value) }
}
case let .response1(count):
state.count = count
return withDependencies {
$0.count.value += 1
} operation: {
.task { .response2(self.count.value) }
}
case let .response2(count):
state.count = count
return withDependencies {
$0.count.value += 1
} operation: {
.task { .response3(self.count.value) }
}
case let .response3(count):
state.count = count
return .none
}
}
}

let testStore = TestStore(
initialState: Feature.State(),
reducer: Feature()
)
await testStore.send(.tap)
await testStore.receive(.response1(1)) {
$0.count = 1
}
await testStore.receive(.response2(1))
await testStore.receive(.response3(1))

let store = Store(
initialState: Feature.State(),
reducer: Feature()
)
await store.send(.tap)?.value
XCTAssertEqual(store.state.value.count, testStore.state.count)
}

func testStoreVsTestStore_Publisher() async {
struct Feature: ReducerProtocol {
struct State: Equatable {
var count = 0
}
enum Action: Equatable {
case tap
case response1(Int)
case response2(Int)
case response3(Int)
}
@Dependency(\.count) var count
func reduce(into state: inout State, action: Action) -> EffectTask<Action> {
switch action {
case .tap:
return withDependencies {
$0.count.value += 1
} operation: {
EffectTask.task { .response1(self.count.value) }
}
.eraseToEffect()
case let .response1(count):
state.count = count
return withDependencies {
$0.count.value += 1
} operation: {
EffectTask.task { .response2(self.count.value) }
}
.eraseToEffect()
case let .response2(count):
state.count = count
return withDependencies {
$0.count.value += 1
} operation: {
EffectTask.task { .response3(self.count.value) }
}
.eraseToEffect()
case let .response3(count):
state.count = count
return .none
}
}
}

let testStore = TestStore(
initialState: Feature.State(),
reducer: Feature()
)
await testStore.send(.tap)
await testStore.receive(.response1(1)) {
$0.count = 1
}
await testStore.receive(.response2(1))
await testStore.receive(.response3(1))

let store = Store(
initialState: Feature.State(),
reducer: Feature()
)
await store.send(.tap)?.value
XCTAssertEqual(store.state.value.count, testStore.state.count)
}
}

private struct Count: TestDependencyKey {
var value: Int
static let liveValue = Count(value: 0)
static let testValue = Count(value: 0)
}
extension DependencyValues {
fileprivate var count: Count {
get { self[Count.self] }
set { self[Count.self] = newValue }
}
}