diff --git a/Sources/ComposableArchitecture/Store.swift b/Sources/ComposableArchitecture/Store.swift index 0ad7ad3678e3..6e3fdde3ab54 100644 --- a/Sources/ComposableArchitecture/Store.swift +++ b/Sources/ComposableArchitecture/Store.swift @@ -485,6 +485,10 @@ public final class Store { for id in self.children.keys { self.invalidateChild(id: id) } + self.effectCancellables.values.forEach { cancellable in + cancellable.cancel() + } + self.effectCancellables.removeAll() } fileprivate func invalidateChild(id: ScopeID) { @@ -526,6 +530,7 @@ public final class Store { defer { index += 1 } let action = self.bufferedActions[index] let effect = self.reducer.reduce(into: ¤tState, action: action) + let uuid = UUID() switch effect.operation { case .none: @@ -533,7 +538,6 @@ public final class Store { case let .publisher(publisher): var didComplete = false let boxedTask = Box?>(wrappedValue: nil) - let uuid = UUID() let effectCancellable = withEscapedDependencies { continuation in publisher .handleEvents( @@ -571,45 +575,48 @@ public final class Store { } case let .run(priority, operation): withEscapedDependencies { continuation in - tasks.wrappedValue.append( - Task(priority: priority) { @MainActor in - #if DEBUG - let isCompleted = LockIsolated(false) - defer { isCompleted.setValue(true) } - #endif - await operation( - Send { effectAction in - #if DEBUG - if isCompleted.value { - 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 'Effect.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) + let task = Task(priority: priority) { @MainActor [weak self] in + #if DEBUG + let isCompleted = LockIsolated(false) + defer { isCompleted.setValue(true) } + #endif + await operation( + Send { effectAction in + #if DEBUG + if isCompleted.value { + 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 'Effect.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) } - ) - } - ) + } + ) + self?.effectCancellables[uuid] = nil + } + tasks.wrappedValue.append(task) + self.effectCancellables[uuid] = AnyCancellable { + task.cancel() + } } } } diff --git a/Tests/ComposableArchitectureMacrosTests/ReducerMacroTests.swift b/Tests/ComposableArchitectureMacrosTests/ReducerMacroTests.swift index 243de8c3895e..5d17e4bff2f4 100644 --- a/Tests/ComposableArchitectureMacrosTests/ReducerMacroTests.swift +++ b/Tests/ComposableArchitectureMacrosTests/ReducerMacroTests.swift @@ -1,168 +1,170 @@ -import ComposableArchitectureMacros -import MacroTesting -import XCTest +#if canImport(ComposableArchitectureMacros) + import ComposableArchitectureMacros + import MacroTesting + import XCTest -final class ReducerMacroTests: XCTestCase { - override func invokeTest() { - withMacroTesting( - // isRecording: true, - macros: [ReducerMacro.self] - ) { - super.invokeTest() + final class ReducerMacroTests: XCTestCase { + override func invokeTest() { + withMacroTesting( + // isRecording: true, + macros: [ReducerMacro.self] + ) { + super.invokeTest() + } } - } - func testBasics() { - assertMacro { - """ - @Reducer - struct Feature { - struct State { - } - enum Action { - } - var body: some ReducerOf { - EmptyReducer() - } - } - """ - } expansion: { - """ - struct Feature { - struct State { - } - @CasePathable - enum Action { - } - @ComposableArchitecture.ReducerBuilder - var body: some ReducerOf { - EmptyReducer() + func testBasics() { + assertMacro { + """ + @Reducer + struct Feature { + struct State { + } + enum Action { + } + var body: some ReducerOf { + EmptyReducer() + } + } + """ + } expansion: { + """ + struct Feature { + struct State { + } + @CasePathable + enum Action { + } + @ComposableArchitecture.ReducerBuilder + var body: some ReducerOf { + EmptyReducer() + } } - } - extension Feature: ComposableArchitecture.Reducer { + extension Feature: ComposableArchitecture.Reducer { + } + """ } - """ } - } - func testEnumState() { - assertMacro { - """ - @Reducer - struct Feature { - enum State { - } - enum Action { - } - var body: some ReducerOf { - EmptyReducer() - } - } - """ - } expansion: { - """ - struct Feature { - @CasePathable @dynamicMemberLookup - enum State { + func testEnumState() { + assertMacro { + """ + @Reducer + struct Feature { + enum State { + } + enum Action { + } + var body: some ReducerOf { + EmptyReducer() + } + } + """ + } expansion: { + """ + struct Feature { + @CasePathable @dynamicMemberLookup + enum State { + } + @CasePathable + enum Action { + } + @ComposableArchitecture.ReducerBuilder + var body: some ReducerOf { + EmptyReducer() + } } - @CasePathable - enum Action { - } - @ComposableArchitecture.ReducerBuilder - var body: some ReducerOf { - EmptyReducer() - } - } - extension Feature: ComposableArchitecture.Reducer { + extension Feature: ComposableArchitecture.Reducer { + } + """ } - """ } - } - func testAlreadyApplied() { - assertMacro { - """ - @Reducer - struct Feature: Reducer, Sendable { - @CasePathable - @dynamicMemberLookup - enum State { - } - @CasePathable - enum Action { - } - @ReducerBuilder - var body: some ReducerOf { - EmptyReducer() - } + func testAlreadyApplied() { + assertMacro { + """ + @Reducer + struct Feature: Reducer, Sendable { + @CasePathable + @dynamicMemberLookup + enum State { + } + @CasePathable + enum Action { + } + @ReducerBuilder + var body: some ReducerOf { + EmptyReducer() + } + } + """ + } expansion: { + """ + struct Feature: Reducer, Sendable { + @CasePathable + @dynamicMemberLookup + enum State { + } + @CasePathable + enum Action { + } + @ReducerBuilder + var body: some ReducerOf { + EmptyReducer() + } + } + """ } - """ - } expansion: { - """ - struct Feature: Reducer, Sendable { - @CasePathable - @dynamicMemberLookup - enum State { - } - @CasePathable - enum Action { - } - @ReducerBuilder - var body: some ReducerOf { - EmptyReducer() - } - } - """ } - } - func testReduceMethodDiagnostic() { - assertMacro { - """ - @Reducer - struct Feature { - struct State { - } - enum Action { - } - func reduce(into state: inout State, action: Action) -> EffectOf { - .none - } - var body: some ReducerOf { - Reduce(reduce) - Reduce(reduce(into:action:)) - Reduce(self.reduce) - Reduce(self.reduce(into:action:)) - Reduce(AnotherReducer().reduce) - Reduce(AnotherReducer().reduce(into:action:)) - } - } - """ - } diagnostics: { - """ - @Reducer - struct Feature { - struct State { - } - enum Action { - } - func reduce(into state: inout State, action: Action) -> EffectOf { - ┬───── - ╰─ 🛑 A 'reduce' method should not be defined in a reducer with a 'body'; it takes precedence and 'body' will never be invoked - .none - } - var body: some ReducerOf { - Reduce(reduce) - Reduce(reduce(into:action:)) - Reduce(self.reduce) - Reduce(self.reduce(into:action:)) - Reduce(AnotherReducer().reduce) - Reduce(AnotherReducer().reduce(into:action:)) - } + func testReduceMethodDiagnostic() { + assertMacro { + """ + @Reducer + struct Feature { + struct State { + } + enum Action { + } + func reduce(into state: inout State, action: Action) -> EffectOf { + .none + } + var body: some ReducerOf { + Reduce(reduce) + Reduce(reduce(into:action:)) + Reduce(self.reduce) + Reduce(self.reduce(into:action:)) + Reduce(AnotherReducer().reduce) + Reduce(AnotherReducer().reduce(into:action:)) + } + } + """ + } diagnostics: { + """ + @Reducer + struct Feature { + struct State { + } + enum Action { + } + func reduce(into state: inout State, action: Action) -> EffectOf { + ┬───── + ╰─ 🛑 A 'reduce' method should not be defined in a reducer with a 'body'; it takes precedence and 'body' will never be invoked + .none + } + var body: some ReducerOf { + Reduce(reduce) + Reduce(reduce(into:action:)) + Reduce(self.reduce) + Reduce(self.reduce(into:action:)) + Reduce(AnotherReducer().reduce) + Reduce(AnotherReducer().reduce(into:action:)) + } + } + """ } - """ } } -} +#endif diff --git a/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift b/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift index 69cd64c0339d..2a01e1c5efc4 100644 --- a/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift +++ b/Tests/ComposableArchitectureTests/Internal/BaseTCATestCase.swift @@ -1,4 +1,4 @@ -@_spi(Internals) import ComposableArchitecture +@_spi(Internals) @_spi(Logging) import ComposableArchitecture import XCTest class BaseTCATestCase: XCTestCase { @@ -6,5 +6,7 @@ class BaseTCATestCase: XCTestCase { super.tearDown() XCTAssertEqual(_cancellationCancellables.count, 0, "\(self)") _cancellationCancellables.removeAll() + Logger.shared.isEnabled = false + Logger.shared.clear() } } diff --git a/Tests/ComposableArchitectureTests/Reducers/PresentationReducerTests.swift b/Tests/ComposableArchitectureTests/Reducers/PresentationReducerTests.swift index 0e548347358d..39df2faef70e 100644 --- a/Tests/ComposableArchitectureTests/Reducers/PresentationReducerTests.swift +++ b/Tests/ComposableArchitectureTests/Reducers/PresentationReducerTests.swift @@ -2221,77 +2221,79 @@ final class PresentationReducerTests: BaseTCATestCase { } } - func testPresentation_leaveChildPresented_WithLongLivingEffect() async { - struct Child: Reducer { - struct State: Equatable {} - enum Action: Equatable { case tap } - var body: some Reducer { - Reduce { state, action in - .run { _ in try await Task.never() } + #if DEBUG + func testPresentation_leaveChildPresented_WithLongLivingEffect() async { + struct Child: Reducer { + struct State: Equatable {} + enum Action: Equatable { case tap } + var body: some Reducer { + Reduce { state, action in + .run { _ in try await Task.never() } + } } } - } - struct Parent: Reducer { - struct State: Equatable { - @PresentationState var child: Child.State? - } - enum Action: Equatable { - case child(PresentationAction) - case presentChild - } - var body: some ReducerOf { - Reduce { state, action in - switch action { - case .child: - return .none - case .presentChild: - state.child = Child.State() - return .none - } + struct Parent: Reducer { + struct State: Equatable { + @PresentationState var child: Child.State? } - .ifLet(\.$child, action: /Action.child) { - Child() + enum Action: Equatable { + case child(PresentationAction) + case presentChild + } + var body: some ReducerOf { + Reduce { state, action in + switch action { + case .child: + return .none + case .presentChild: + state.child = Child.State() + return .none + } + } + .ifLet(\.$child, action: /Action.child) { + Child() + } } } - } - let store = TestStore(initialState: Parent.State()) { - Parent() - } + let store = TestStore(initialState: Parent.State()) { + Parent() + } - await store.send(.presentChild) { - $0.child = Child.State() - } - let line = #line - await store.send(.child(.presented(.tap))) + await store.send(.presentChild) { + $0.child = Child.State() + } + let line = #line + await store.send(.child(.presented(.tap))) - XCTExpectFailure { - $0.sourceCodeContext.location?.fileURL.absoluteString.contains("BaseTCATestCase") == true - || $0.sourceCodeContext.location?.lineNumber == line + 1 - && $0.compactDescription == """ - An effect returned for this action is still running. It must complete before the end \ - of the test. … - - To fix, inspect any effects the reducer returns for this action and ensure that all \ - of them complete by the end of the test. There are a few reasons why an effect may \ - not have completed: - - • If using async/await in your effect, it may need a little bit of time to properly \ - finish. To fix you can simply perform "await store.finish()" at the end of your test. - - • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", \ - etc.), make sure that you wait enough time for it to perform the effect. If you are \ - using a test clock/scheduler, advance it so that the effects may complete, or \ - consider using an immediate clock/scheduler to immediately perform the effect instead. - - • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ - then make sure those effects are torn down by marking the effect ".cancellable" and \ - returning a corresponding cancellation effect ("Effect.cancel") from another action, \ - or, if your effect is driven by a Combine subject, send it a completion. - """ + XCTExpectFailure { + $0.sourceCodeContext.location?.fileURL.absoluteString.contains("BaseTCATestCase") == true + || $0.sourceCodeContext.location?.lineNumber == line + 1 + && $0.compactDescription == """ + An effect returned for this action is still running. It must complete before the end \ + of the test. … + + To fix, inspect any effects the reducer returns for this action and ensure that all \ + of them complete by the end of the test. There are a few reasons why an effect may \ + not have completed: + + • If using async/await in your effect, it may need a little bit of time to properly \ + finish. To fix you can simply perform "await store.finish()" at the end of your test. + + • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", \ + etc.), make sure that you wait enough time for it to perform the effect. If you are \ + using a test clock/scheduler, advance it so that the effects may complete, or \ + consider using an immediate clock/scheduler to immediately perform the effect instead. + + • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ + then make sure those effects are torn down by marking the effect ".cancellable" and \ + returning a corresponding cancellation effect ("Effect.cancel") from another action, \ + or, if your effect is driven by a Combine subject, send it a completion. + """ + } } - } + #endif func testCancelInFlightEffects() async { struct Child: Reducer { diff --git a/Tests/ComposableArchitectureTests/Reducers/StackReducerTests.swift b/Tests/ComposableArchitectureTests/Reducers/StackReducerTests.swift index fd4bd4f52ab7..5440f111a897 100644 --- a/Tests/ComposableArchitectureTests/Reducers/StackReducerTests.swift +++ b/Tests/ComposableArchitectureTests/Reducers/StackReducerTests.swift @@ -846,63 +846,65 @@ final class StackReducerTests: BaseTCATestCase { } #endif - func testChildWithInFlightEffect() async { - struct Child: Reducer { - struct State: Equatable {} - enum Action { case tap } - var body: some Reducer { - Reduce { state, action in - .run { _ in try await Task.never() } + #if DEBUG + func testChildWithInFlightEffect() async { + struct Child: Reducer { + struct State: Equatable {} + enum Action { case tap } + var body: some Reducer { + Reduce { state, action in + .run { _ in try await Task.never() } + } } } - } - struct Parent: Reducer { - struct State: Equatable { - var path = StackState() - } - enum Action { - case path(StackAction) - } - var body: some ReducerOf { - EmptyReducer() - .forEach(\.path, action: /Action.path) { Child() } + struct Parent: Reducer { + struct State: Equatable { + var path = StackState() + } + enum Action { + case path(StackAction) + } + var body: some ReducerOf { + EmptyReducer() + .forEach(\.path, action: /Action.path) { Child() } + } } - } - var path = StackState() - path.append(Child.State()) - let store = TestStore(initialState: Parent.State(path: path)) { - Parent() - } - let line = #line - await store.send(.path(.element(id: 0, action: .tap))) + var path = StackState() + path.append(Child.State()) + let store = TestStore(initialState: Parent.State(path: path)) { + Parent() + } + let line = #line + await store.send(.path(.element(id: 0, action: .tap))) - XCTExpectFailure { - $0.sourceCodeContext.location?.fileURL.absoluteString.contains("BaseTCATestCase") == true - || $0.sourceCodeContext.location?.lineNumber == line + 1 - && $0.compactDescription == """ - An effect returned for this action is still running. It must complete before the end \ - of the test. … + XCTExpectFailure { + $0.sourceCodeContext.location?.fileURL.absoluteString.contains("BaseTCATestCase") == true + || $0.sourceCodeContext.location?.lineNumber == line + 1 + && $0.compactDescription == """ + An effect returned for this action is still running. It must complete before the end \ + of the test. … - To fix, inspect any effects the reducer returns for this action and ensure that all \ - of them complete by the end of the test. There are a few reasons why an effect may \ - not have completed: + To fix, inspect any effects the reducer returns for this action and ensure that all \ + of them complete by the end of the test. There are a few reasons why an effect may \ + not have completed: - • If using async/await in your effect, it may need a little bit of time to properly \ - finish. To fix you can simply perform "await store.finish()" at the end of your test. + • If using async/await in your effect, it may need a little bit of time to properly \ + finish. To fix you can simply perform "await store.finish()" at the end of your test. - • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", \ - etc.), make sure that you wait enough time for it to perform the effect. If you are \ - using a test clock/scheduler, advance it so that the effects may complete, or \ - consider using an immediate clock/scheduler to immediately perform the effect instead. + • If an effect uses a clock/scheduler (via "receive(on:)", "delay", "debounce", \ + etc.), make sure that you wait enough time for it to perform the effect. If you are \ + using a test clock/scheduler, advance it so that the effects may complete, or \ + consider using an immediate clock/scheduler to immediately perform the effect instead. - • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ - then make sure those effects are torn down by marking the effect ".cancellable" and \ - returning a corresponding cancellation effect ("Effect.cancel") from another action, \ - or, if your effect is driven by a Combine subject, send it a completion. - """ + • If you are returning a long-living effect (timers, notifications, subjects, etc.), \ + then make sure those effects are torn down by marking the effect ".cancellable" and \ + returning a corresponding cancellation effect ("Effect.cancel") from another action, \ + or, if your effect is driven by a Combine subject, send it a completion. + """ + } } - } + #endif func testMultipleChildEffects() async { struct Child: Reducer { diff --git a/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift b/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift index 335249d63792..15cc45ac77f3 100644 --- a/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift +++ b/Tests/ComposableArchitectureTests/StoreLifetimeTests.swift @@ -1,4 +1,5 @@ -import ComposableArchitecture +import Combine +@_spi(Logging) import ComposableArchitecture import XCTest @MainActor @@ -44,6 +45,78 @@ final class StoreLifetimeTests: BaseTCATestCase { XCTAssertEqual(4, grandparentStore.withState(\.child.child.count)) XCTAssertEqual(4, childStore.withState(\.count)) } + + #if DEBUG + func testStoreDeinit() { + Logger.shared.isEnabled = true + do { + let store = Store(initialState: ()) {} + _ = store + } + + XCTAssertEqual( + Logger.shared.logs, + [ + "Store<(), ()>.init", + "Store<(), ()>.deinit", + ] + ) + } + + func testStoreDeinit_RunningEffect() async { + Logger.shared.isEnabled = true + let effectFinished = self.expectation(description: "Effect finished") + do { + let store = Store(initialState: ()) { + Reduce { state, _ in + .run { _ in + try? await Task.never() + effectFinished.fulfill() + } + } + } + store.send(()) + _ = store + } + + XCTAssertEqual( + Logger.shared.logs, + [ + "Store<(), ()>.init", + "Store<(), ()>.deinit", + ] + ) + await self.fulfillment(of: [effectFinished], timeout: 0.5) + } + + func testStoreDeinit_RunningCombineEffect() async { + Logger.shared.isEnabled = true + let effectFinished = self.expectation(description: "Effect finished") + do { + let store = Store(initialState: ()) { + Reduce { state, _ in + .publisher { + Empty(completeImmediately: false) + .handleEvents(receiveCancel: { + effectFinished.fulfill() + }) + } + } + } + store.send(()) + _ = store + } + + XCTAssertEqual( + Logger.shared.logs, + [ + "Store<(), ()>.init", + "Store<(), ()>.deinit", + ] + ) + await self.fulfillment(of: [effectFinished], timeout: 0.5) + } + #endif } @Reducer