Skip to content

Commit d9b6335

Browse files
gfreezyclaude
andcommitted
Fix external change handler to correctly handle nil values
When UserDefaults key is removed (set to nil), use _default_value_of_ as fallback instead of current cached value. This ensures removed keys correctly revert to default without triggering spurious mutations. Also distinguish between @DefaultsBacked (stored in UD) and @ObservableOnly (memory-only) properties - only apply equality check to backed properties that have _default_value_of_ storage. Add tests for nil/removed key scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ceffd41 commit d9b6335

2 files changed

Lines changed: 54 additions & 6 deletions

File tree

Sources/ObservableDefaultsMacros/Macros/ObservableDefaultsMacro.swift

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ extension ObservableDefaultsMacros: MemberMacro {
109109
suiteName,
110110
prefix,
111111
ignoreExternalChanges,
112-
_,
112+
observeFirst,
113113
limitToInstance,
114114
defaultIsolationIsMainActor,
115115
suiteNameExpression
@@ -149,7 +149,7 @@ extension ObservableDefaultsMacros: MemberMacro {
149149
}
150150

151151
// Build mapping between properties and their UserDefaults keys
152-
let metas: [(userDefaultsKey: String, propertyID: String)] = persistentProperties
152+
let metas: [(userDefaultsKey: String, propertyID: String, isBacked: Bool)] = persistentProperties
153153
.map { property in
154154
let key =
155155
property.attributes.extractValue(
@@ -159,7 +159,10 @@ extension ObservableDefaultsMacros: MemberMacro {
159159
forAttribute: DefaultsKeyMacro.name,
160160
argument: DefaultsKeyMacro.key) ?? property.identifier?.text ?? ""
161161
let propertyID = property.identifier?.text ?? ""
162-
return (key, propertyID)
162+
// In observeFirst mode, only explicitly @DefaultsBacked properties are backed;
163+
// otherwise all persistent properties are backed.
164+
let isBacked = !observeFirst || property.hasAttribute(named: DefaultsBackedMacro.name)
165+
return (key, propertyID, isBacked)
163166
}
164167

165168
// Generate keyPath mapping for external change handling
@@ -178,11 +181,26 @@ extension ObservableDefaultsMacros: MemberMacro {
178181
let caseCode = metas.enumerated().map { index, meta in
179182
let caseIndent = index == 0 ? "" : " "
180183
// swiftformat:disable all
181-
if hasMainActor {
184+
if !meta.isBacked {
185+
// ObservableOnly properties are not stored in UserDefaults,
186+
// so we cannot compare values — keep the original behavior.
187+
if hasMainActor {
188+
return """
189+
\(caseIndent)case prefix + "\(meta.userDefaultsKey)":
190+
\(caseIndent) MainActor.assumeIsolated {
191+
\(caseIndent) host._$observationRegistrar.withMutation(of: host, keyPath: \\.\(meta.propertyID)) {}
192+
\(caseIndent) }
193+
"""
194+
} else {
195+
return """
196+
\(caseIndent)case prefix + "\(meta.userDefaultsKey)": host._$observationRegistrar.withMutation(of: host, keyPath: \\.\(meta.propertyID)) {}
197+
"""
198+
}
199+
} else if hasMainActor {
182200
return """
183201
\(caseIndent)case prefix + "\(meta.userDefaultsKey)":
184202
\(caseIndent) MainActor.assumeIsolated {
185-
\(caseIndent) let newValue = UserDefaultsWrapper.getValue(fullKey, host._\(meta.propertyID), host._userDefaults)
203+
\(caseIndent) let newValue = UserDefaultsWrapper.getValue(fullKey, host._default_value_of_\(meta.propertyID), host._userDefaults)
186204
\(caseIndent) if host.shouldSetValue(newValue, host._\(meta.propertyID)) {
187205
\(caseIndent) host._\(meta.propertyID) = newValue
188206
\(caseIndent) host._$observationRegistrar.withMutation(of: host, keyPath: \\.\(meta.propertyID)) {}
@@ -192,7 +210,7 @@ extension ObservableDefaultsMacros: MemberMacro {
192210
} else {
193211
return """
194212
\(caseIndent)case prefix + "\(meta.userDefaultsKey)":
195-
\(caseIndent) let newValue = UserDefaultsWrapper.getValue(fullKey, host._\(meta.propertyID), host._userDefaults)
213+
\(caseIndent) let newValue = UserDefaultsWrapper.getValue(fullKey, host._default_value_of_\(meta.propertyID), host._userDefaults)
196214
\(caseIndent) if host.shouldSetValue(newValue, host._\(meta.propertyID)) {
197215
\(caseIndent) host._\(meta.propertyID) = newValue
198216
\(caseIndent) host._$observationRegistrar.withMutation(of: host, keyPath: \\.\(meta.propertyID)) {}

Tests/ObservableDefaultsTests/ExternalChangeEqualityTests.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,34 @@ struct ExternalChangeEqualityTests {
6464
try? await Task.sleep(nanoseconds: 100_000_000)
6565
#expect(model.name == "Test")
6666
}
67+
68+
// MARK: - Removing key (setting to nil) should trigger mutation
69+
70+
@Test("Removing key from UserDefaults triggers mutation back to default value")
71+
func removingKeyTriggersMutation() {
72+
let userDefaults = UserDefaults.getTestInstance(suiteName: #function)
73+
let model = MockModel(userDefaults: userDefaults)
74+
75+
// Set a non-default value first
76+
model.name = "Changed"
77+
78+
// Removing the key should trigger mutation (value reverts to default "Test")
79+
tracking(model, \.name, .userDefaults)
80+
userDefaults.removeObject(forKey: "name")
81+
#expect(model.name == "Test")
82+
}
83+
84+
@Test("Optional property: removing key triggers mutation back to nil")
85+
func optionalRemovingKeyTriggersMutation() {
86+
let userDefaults = UserDefaults.getTestInstance(suiteName: #function)
87+
let model = MockModelOptional(userDefaults: userDefaults)
88+
89+
// Set a non-nil value first
90+
model.optionalName = "Hello"
91+
92+
// Removing the key should trigger mutation (value reverts to nil)
93+
tracking(model, \.optionalName, .userDefaults)
94+
userDefaults.removeObject(forKey: "optionalName")
95+
#expect(model.optionalName == nil)
96+
}
6797
}

0 commit comments

Comments
 (0)