Fix infinite recursion in @ObservableDefault with mutating methods (#218)

Fixes #217

Author: sindresorhus

Sources/DefaultsMacrosDeclarations/ObservableDefaultMacro.swift

@@ -27,13 +27,19 @@ extension ObservableDefaultMacro: AccessorMacro {
 		// The get accessor also sets up an observation to update the value when the UserDefaults
 		// changes from elsewhere. Doing so requires attaching it as an Objective-C associated
 		// object due to limitations with current macro capabilities and Swift concurrency.
+		//
+		// To prevent infinite recursion, we use Defaults.withoutPropagation in the observation
+		// callback. This ensures that when the callback updates the property, it doesn't trigger
+		// observers again, while still allowing normal writes to propagate to other observers.
 		return [
 			#"""
 			get {
 				if objc_getAssociatedObject(self, &Self.\#(associatedKey)) == nil {
 					let cancellable = Defaults.publisher(\#(expression))
-						.sink { [weak self] in
-							self?.\#(property) = $0.newValue
+						.sink { [weak self] change in
+							Defaults.withoutPropagation {
+								self?.\#(property) = change.newValue
+							}
 						}
 					objc_setAssociatedObject(self, &Self.\#(associatedKey), cancellable, .OBJC_ASSOCIATION_RETAIN)
 				}

Tests/DefaultsMacrosDeclarationsTests/ObservableDefaultMacroTests.swift

@@ -88,8 +88,10 @@ final class ObservableDefaultMacroTests: XCTestCase {
 				get {
 					if objc_getAssociatedObject(self, &Self._objcAssociatedKey_name) == nil {
 						let cancellable = Defaults.publisher(\#(keyExpression))
-							.sink { [weak self] in
-								self?.name = $0.newValue
+							.sink { [weak self] change in
+								Defaults.withoutPropagation {
+									self?.name = change.newValue
+								}
 							}
 						objc_setAssociatedObject(self, &Self._objcAssociatedKey_name, cancellable, .OBJC_ASSOCIATION_RETAIN)
 					}

Tests/DefaultsMacrosTests/ObservableDefaultTests.swift

@@ -12,9 +12,12 @@ private let colorKey = "colorKey"
 private let defaultColor = "blue"
 private let newColor = "purple"
 
+private let testSetKey = "testSetKey"
+
 extension Defaults.Keys {
 	static let animal = Defaults.Key(animalKey, default: defaultAnimal)
 	static let color = Defaults.Key(colorKey, default: defaultColor)
+	static let testSet = Defaults.Key(testSetKey, default: Set<Int>())
 }
 
 func getKey() -> Defaults.Key<String> {
@@ -67,12 +70,21 @@ private final class TestModelWithMultipleValues {
 	var color: String
 }
 
+@available(macOS 14, iOS 17, tvOS 17, watchOS 10, visionOS 1, *)
+@Observable
+private final class TestModelWithSet {
+	@ObservableDefault(.testSet)
+	@ObservationIgnored
+	var testSet: Set<Int>
+}
+
 @Suite(.serialized)
 final class ObservableDefaultTests {
 	init() {
 		Defaults.removeAll()
 		Defaults[.animal] = defaultAnimal
 		Defaults[.color] = defaultColor
+		Defaults[.testSet] = []
 	}
 
 	deinit {
@@ -194,4 +206,41 @@ final class ObservableDefaultTests {
 		#expect(model.animal == newAnimal)
 		#expect(model.color == newColor)
 	}
+
+	@available(macOS 14, iOS 17, tvOS 17, watchOS 10, visionOS 1, *)
+	@Test
+	func testMacroWithSetNoInfiniteRecursion() async {
+		let model = TestModelWithSet()
+		#expect(model.testSet.isEmpty)
+
+		// This should not cause infinite recursion
+		model.testSet.formUnion(1...10)
+
+		#expect(model.testSet == Set(1...10))
+		#expect(Defaults[.testSet] == Set(1...10))
+	}
+
+	@available(macOS 14, iOS 17, tvOS 17, watchOS 10, visionOS 1, *)
+	@Test
+	func testMacroObserversPropagateAcrossModels() async {
+		let model1 = TestModelWithSet()
+		let model2 = TestModelWithSet()
+
+		#expect(model1.testSet.isEmpty)
+		#expect(model2.testSet.isEmpty)
+
+		await confirmation { confirmation in
+			_ = withObservationTracking {
+				model2.testSet
+			} onChange: {
+				confirmation()
+			}
+
+			// Write through model1
+			model1.testSet = [1, 2, 3]
+		}
+
+		// model2 should have observed the change
+		#expect(model2.testSet == [1, 2, 3])
+	}
 }