Runtime warn for nested observe calls (#2996)

* Runtime warning when we detect nested observe. * finesse * wip * add some tests --------- Co-authored-by: Brandon Williams <[email protected]>
pointfreeco · Apr 17, 2024 · 899a68c · 899a68c
1 parent ca28c7b
commit 899a68c
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 2 deletions.
diff --git a/Sources/ComposableArchitecture/UIKit/NSObject+Observation.swift b/Sources/ComposableArchitecture/UIKit/NSObject+Observation.swift
@@ -139,6 +139,16 @@
     /// ```
     @discardableResult
     public func observe(_ apply: @escaping () -> Void) -> ObservationToken {
+      if ObserveLocals.isApplying {
+        runtimeWarn(
+          """
+          An "observe" was called from another "observe" closure, which can lead to \
+          over-observation and unintended side effects.
+
+          Avoid nested closures by moving child observation into their own lifecycle methods.
+          """
+        )
+      }
       let token = ObservationToken()
       self.tokens.insert(token)
       @Sendable func onChange() {
@@ -149,7 +159,9 @@
           Task { @MainActor in
             guard !token.isCancelled
             else { return }
-            onChange()
+            ObserveLocals.$isApplying.withValue(true) {
+              onChange()
+            }
           }
         }
       }
@@ -191,3 +203,7 @@
     }
   }
 #endif
+
+private enum ObserveLocals {
+  @TaskLocal static var isApplying = false
+}
diff --git a/Tests/ComposableArchitectureTests/ObserveTests.swift b/Tests/ComposableArchitectureTests/ObserveTests.swift
@@ -14,9 +14,13 @@
       model.count += 1
       try await Task.sleep(nanoseconds: 1_000_000)
       XCTAssertEqual(counts, [0, 1])
+
+      model.otherCount += 1
+      try await Task.sleep(nanoseconds: 1_000_000)
+      XCTAssertEqual(counts, [0, 1])
+
       _ = observation
     }
-
     func testCancellation() async throws {
       let model = Model()
       var counts: [Int] = []
@@ -30,10 +34,48 @@
       XCTAssertEqual(counts, [0])
       _ = observation
     }
+
+    @MainActor
+    func testNestedObservation() async throws {
+      XCTExpectFailure {
+        $0.compactDescription == """
+          An "observe" was called from another "observe" closure, which can lead to \
+          over-observation and unintended side effects.
+
+          Avoid nested closures by moving child observation into their own lifecycle methods.
+          """
+      }
+
+      let model = Model()
+      var counts: [Int] = []
+      var innerObservation: Any!
+      let observation = observe { [weak self] in
+        guard let self else { return }
+        counts.append(model.count)
+        innerObservation = observe {
+          _ = model.otherCount
+        }
+      }
+      defer {
+        _ = observation
+        _ = innerObservation
+      }
+
+      XCTAssertEqual(counts, [0])
+
+      model.count += 1
+      try await Task.sleep(nanoseconds: 1_000_000)
+      XCTAssertEqual(counts, [0, 1])
+
+      model.otherCount += 1
+      try await Task.sleep(nanoseconds: 1_000_000)
+      XCTAssertEqual(counts, [0, 1, 1])
+    }
   }
 
   @Perceptible
   class Model {
     var count = 0
+    var otherCount = 0
   }
 #endif