Skip to content

Commit 899a68c

Browse files
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]>
1 parent ca28c7b commit 899a68c

File tree

2 files changed

+60
-2
lines changed

2 files changed

+60
-2
lines changed

Sources/ComposableArchitecture/UIKit/NSObject+Observation.swift

+17-1
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,16 @@
139139
/// ```
140140
@discardableResult
141141
public func observe(_ apply: @escaping () -> Void) -> ObservationToken {
142+
if ObserveLocals.isApplying {
143+
runtimeWarn(
144+
"""
145+
An "observe" was called from another "observe" closure, which can lead to \
146+
over-observation and unintended side effects.
147+
148+
Avoid nested closures by moving child observation into their own lifecycle methods.
149+
"""
150+
)
151+
}
142152
let token = ObservationToken()
143153
self.tokens.insert(token)
144154
@Sendable func onChange() {
@@ -149,7 +159,9 @@
149159
Task { @MainActor in
150160
guard !token.isCancelled
151161
else { return }
152-
onChange()
162+
ObserveLocals.$isApplying.withValue(true) {
163+
onChange()
164+
}
153165
}
154166
}
155167
}
@@ -191,3 +203,7 @@
191203
}
192204
}
193205
#endif
206+
207+
private enum ObserveLocals {
208+
@TaskLocal static var isApplying = false
209+
}

Tests/ComposableArchitectureTests/ObserveTests.swift

+43-1
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,13 @@
1414
model.count += 1
1515
try await Task.sleep(nanoseconds: 1_000_000)
1616
XCTAssertEqual(counts, [0, 1])
17+
18+
model.otherCount += 1
19+
try await Task.sleep(nanoseconds: 1_000_000)
20+
XCTAssertEqual(counts, [0, 1])
21+
1722
_ = observation
1823
}
19-
2024
func testCancellation() async throws {
2125
let model = Model()
2226
var counts: [Int] = []
@@ -30,10 +34,48 @@
3034
XCTAssertEqual(counts, [0])
3135
_ = observation
3236
}
37+
38+
@MainActor
39+
func testNestedObservation() async throws {
40+
XCTExpectFailure {
41+
$0.compactDescription == """
42+
An "observe" was called from another "observe" closure, which can lead to \
43+
over-observation and unintended side effects.
44+
45+
Avoid nested closures by moving child observation into their own lifecycle methods.
46+
"""
47+
}
48+
49+
let model = Model()
50+
var counts: [Int] = []
51+
var innerObservation: Any!
52+
let observation = observe { [weak self] in
53+
guard let self else { return }
54+
counts.append(model.count)
55+
innerObservation = observe {
56+
_ = model.otherCount
57+
}
58+
}
59+
defer {
60+
_ = observation
61+
_ = innerObservation
62+
}
63+
64+
XCTAssertEqual(counts, [0])
65+
66+
model.count += 1
67+
try await Task.sleep(nanoseconds: 1_000_000)
68+
XCTAssertEqual(counts, [0, 1])
69+
70+
model.otherCount += 1
71+
try await Task.sleep(nanoseconds: 1_000_000)
72+
XCTAssertEqual(counts, [0, 1, 1])
73+
}
3374
}
3475

3576
@Perceptible
3677
class Model {
3778
var count = 0
79+
var otherCount = 0
3880
}
3981
#endif

0 commit comments

Comments
 (0)