Prevented reference retention in withPrevious(…)

Author: lukaskubanek

Sources/CombineExtensions/Operators/WithPrevious.swift

@@ -1,7 +1,5 @@
 import Combine
 
-// Inspired by https://stackoverflow.com/a/67133582/670119
-
 extension Publisher {
 
     /// Transforms elements from the upstream publisher into a tuple containing the current and
@@ -19,9 +17,19 @@ extension Publisher {
     ///
     /// - Returns: A publisher that emits a tuple of the previous and current elements from the
     ///   upstream publisher.
+    ///
+    /// - Note: Until version 0.3.0 of the library, this was implemented using `.scan` (see
+    ///   [this post](https://stackoverflow.com/a/67133582/670119)). However, that approach
+    ///   strongly retained previous values and could keep them in memory longer than expected
+    ///   when `Output` was a reference type. This version avoids that by using a local variable
+    ///   inside `map`, which does not retain previous values beyond each emission.
     public func withPrevious() -> AnyPublisher<(previous: Output?, current: Output), Failure> {
-        self.scan(Optional<(Output?, Output)>.none) { ($0?.1, $1) }
-            .compactMap { $0 }
+        var previous: Output?
+        return self
+            .map { current in
+                defer { previous = current }
+                return (previous: previous, current: current)
+            }
             .eraseToAnyPublisher()
     }
 
@@ -42,10 +50,21 @@ extension Publisher {
     ///   emission.
     /// - Returns: A publisher that emits a tuple of the previous and current elements from the
     ///   upstream publisher.
+    ///
+    /// - Note: Until version 0.3.0 of the library, this was implemented using `.scan` (see
+    ///   [this post](https://stackoverflow.com/a/67133582/670119)). However, that approach
+    ///   strongly retained previous values and could keep them in memory longer than expected
+    ///   when `Output` was a reference type. This version avoids that by using a local variable
+    ///   inside `map`, which does not retain previous values beyond each emission.
     public func withPrevious(
         initialValue: Output
     ) -> AnyPublisher<(previous: Output, current: Output), Failure> {
-        self.scan((initialValue, initialValue)) { ($0.1, $1) }
+        var previous = initialValue
+        return self
+            .map { current in
+                defer { previous = current }
+                return (previous: previous, current: current)
+            }
             .eraseToAnyPublisher()
     }
 

Tests/CombineExtensionsTests/Operators/WithPreviousTests.swift

@@ -13,7 +13,9 @@ class WithPreviousTests: XCTestCase {
 
         subject
             .withPrevious()
-            .sink { _ in completed = true } receiveValue: { pair in
+            .sink { _ in
+                completed = true
+            } receiveValue: { pair in
                 results.append([pair.previous, pair.current])
             }
             .store(in: &cancellables)
@@ -41,7 +43,9 @@ class WithPreviousTests: XCTestCase {
 
         subject
             .withPrevious(initialValue: 0)
-            .sink { _ in completed = true } receiveValue: { pair in
+            .sink { _ in
+                completed = true
+            } receiveValue: { pair in
                 results.append([pair.previous, pair.current])
             }
             .store(in: &cancellables)
@@ -96,4 +100,40 @@ class WithPreviousTests: XCTestCase {
         XCTAssertTrue(completed)
     }
 
+    func testWithPrevious_doesNotRetainObjects() {
+        var cancellables = Set<AnyCancellable>()
+        var deallocatedIDs = Set<Int>()
+        
+        let subject = PassthroughSubject<TestObject, Never>()
+        
+        subject
+            .withPrevious()
+            .sink { _ in }
+            .store(in: &cancellables)
+        
+        for id in 0...2 {
+            let object = TestObject(id: id) { deallocatedIDs.insert(id) }
+            subject.send(object)
+        }
+        
+        XCTAssertEqual(deallocatedIDs, [0, 1])
+        
+        subject.send(completion: .finished)
+        XCTAssertEqual(deallocatedIDs, [0, 1, 2])
+    }
+    
+}
+
+private final class TestObject {
+    let id: Int
+    let onDeinit: () -> Void
+    
+    init(id: Int, onDeinit: @escaping () -> Void) {
+        self.id = id
+        self.onDeinit = onDeinit
+    }
+    
+    deinit {
+        self.onDeinit()
+    }
 }