move shouldNotifyObservers logic into ObservationState

rcarver · rcarver · commit c88f90db0fab · 2025-08-12T08:30:35.000-07:00
diff --git a/Sources/ComposableArchitecture/Observation/ObservationStateRegistrar.swift b/Sources/ComposableArchitecture/Observation/ObservationStateRegistrar.swift
@@ -51,12 +51,17 @@ extension ObservationStateRegistrar: Equatable, Hashable, Codable {
       keyPath: KeyPath<Subject, Member>,
       _ value: inout Value,
       _ newValue: Value,
-      _ isIdentityEqual: (Value, Value) -> Bool
+      _ isIdentityEqual: (Value, Value) -> Bool,
+      _ shouldNotifyObservers: (Value, Value) -> Bool
     ) {
       if isIdentityEqual(value, newValue) {
         value = newValue
       } else {
-        self.registrar.withMutation(of: subject, keyPath: keyPath) {
+        if shouldNotifyObservers(value, newValue) {
+          self.registrar.withMutation(of: subject, keyPath: keyPath) {
+            value = newValue
+          }
+        } else {
           value = newValue
         }
       }
@@ -101,12 +106,13 @@ extension ObservationStateRegistrar: Equatable, Hashable, Codable {
       keyPath: KeyPath<Subject, Member>,
       _ member: inout Member,
       _ oldValue: Member,
-      _ isIdentityEqual: (Member, Member) -> Bool
+      _ isIdentityEqual: (Member, Member) -> Bool,
+      _ shouldNotifyObservers: (Member, Member) -> Bool
     ) {
       if !isIdentityEqual(oldValue, member) {
         let newValue = member
         member = oldValue
-        self.mutate(subject, keyPath: keyPath, &member, newValue, isIdentityEqual)
+        self.mutate(subject, keyPath: keyPath, &member, newValue, isIdentityEqual, shouldNotifyObservers)
       }
     }
   }
diff --git a/Sources/ComposableArchitectureMacros/ObservableStateMacro.swift b/Sources/ComposableArchitectureMacros/ObservableStateMacro.swift
@@ -647,20 +647,16 @@ public struct ObservationStateTrackedMacro: AccessorMacro {
     let setAccessor: AccessorDeclSyntax =
       """
       set {
-        guard shouldNotifyObservers(_\(identifier), newValue) else {
-          return
-        }
-        \(raw: ObservableStateMacro.registrarVariableName).mutate(self, keyPath: \\.\(identifier), &_\(identifier), newValue, _$isIdentityEqual)
+      \(raw: ObservableStateMacro.registrarVariableName).mutate(self, keyPath: \\.\(identifier), &_\(identifier), newValue, _$isIdentityEqual, shouldNotifyObservers)
       }
       """
     let modifyAccessor: AccessorDeclSyntax = """
       _modify {
         let oldValue = _$observationRegistrar.willModify(self, keyPath: \\.\(identifier), &_\(identifier))
-        yield &_\(identifier)
-        guard shouldNotifyObservers(oldValue, \(identifier)) else {
-          return
+        defer {
+          _$observationRegistrar.didModify(self, keyPath: \\.\(identifier), &_\(identifier), oldValue, _$isIdentityEqual, shouldNotifyObservers)
         }
-        _$observationRegistrar.didModify(self, keyPath: \\.\(identifier), &_\(identifier), oldValue, _$isIdentityEqual)
+        yield &_\(identifier)
       }
       """
 
diff --git a/Tests/ComposableArchitectureMacrosTests/ObservableStateMacroTests.swift b/Tests/ComposableArchitectureMacrosTests/ObservableStateMacroTests.swift
@@ -37,18 +37,14 @@
               return _count
             }
             set {
-              guard shouldNotifyObservers(_count, newValue) else {
-                return
-              }
-              _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual)
+              _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual, shouldNotifyObservers)
             }
             _modify {
               let oldValue = _$observationRegistrar.willModify(self, keyPath: \.count, &_count)
-              yield &_count
-              guard shouldNotifyObservers(oldValue, count) else {
-                return
+              defer {
+                _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual, shouldNotifyObservers)
               }
-              _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual)
+              yield &_count
             }
           }
 
@@ -103,18 +99,14 @@
               return _count
             }
             set {
-              guard shouldNotifyObservers(_count, newValue) else {
-                return
-              }
-              _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual)
+              _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual, shouldNotifyObservers)
             }
             _modify {
               let oldValue = _$observationRegistrar.willModify(self, keyPath: \.count, &_count)
-              yield &_count
-              guard shouldNotifyObservers(oldValue, count) else {
-                return
+              defer {
+                _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual, shouldNotifyObservers)
               }
-              _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual)
+              yield &_count
             }
           }
 
@@ -169,18 +161,14 @@
               return _count
             }
             set {
-              guard shouldNotifyObservers(_count, newValue) else {
-                return
-              }
-              _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual)
+              _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual, shouldNotifyObservers)
             }
             _modify {
               let oldValue = _$observationRegistrar.willModify(self, keyPath: \.count, &_count)
-              yield &_count
-              guard shouldNotifyObservers(oldValue, count) else {
-                return
+              defer {
+                _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual, shouldNotifyObservers)
               }
-              _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual)
+              yield &_count
             }
           }
 
@@ -232,18 +220,14 @@
               return _count
             }
             set {
-              guard shouldNotifyObservers(_count, newValue) else {
-                return
-              }
-              _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual)
+              _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual, shouldNotifyObservers)
             }
             _modify {
               let oldValue = _$observationRegistrar.willModify(self, keyPath: \.count, &_count)
-              yield &_count
-              guard shouldNotifyObservers(oldValue, count) else {
-                return
+              defer {
+                _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual, shouldNotifyObservers)
               }
-              _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual)
+              yield &_count
             }
           }
 
diff --git a/Tests/ComposableArchitectureTests/ObservableTests.swift b/Tests/ComposableArchitectureTests/ObservableTests.swift
@@ -18,6 +18,19 @@ final class ObservableTests: BaseTCATestCase {
     XCTAssertEqual(state.count, 1)
   }
 
+  func testAssignEqualValue() async {
+    var state = ChildState()
+
+    withPerceptionTracking {
+      _ = state.count
+    } onChange: {
+      XCTFail("state.count should not change.")
+    }
+
+    state.count = state.count
+    XCTAssertEqual(state.count, 0)
+  }
+
   func testCopyMutation() async {
     XCTTODO(
       """

Original file line number	Diff line number	Diff line change
`@@ -37,18 +37,14 @@`
`37`	`37`	`return _count`
`38`	`38`	`}`
`39`	`39`	`set {`
`40`		`- guard shouldNotifyObservers(_count, newValue) else {`
`41`		`- return`
`42`		`- }`
`43`		`- _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual)`
	`40`	`+ _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual, shouldNotifyObservers)`
`44`	`41`	`}`
`45`	`42`	`_modify {`
`46`	`43`	`let oldValue = _$observationRegistrar.willModify(self, keyPath: \.count, &_count)`
`47`		`- yield &_count`
`48`		`- guard shouldNotifyObservers(oldValue, count) else {`
`49`		`- return`
	`44`	`+ defer {`
	`45`	`+ _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual, shouldNotifyObservers)`
`50`	`46`	`}`
`51`		`- _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual)`
	`47`	`+ yield &_count`
`52`	`48`	`}`
`53`	`49`	`}`
`54`	`50`
`@@ -103,18 +99,14 @@`
`103`	`99`	`return _count`
`104`	`100`	`}`
`105`	`101`	`set {`
`106`		`- guard shouldNotifyObservers(_count, newValue) else {`
`107`		`- return`
`108`		`- }`
`109`		`- _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual)`
	`102`	`+ _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual, shouldNotifyObservers)`
`110`	`103`	`}`
`111`	`104`	`_modify {`
`112`	`105`	`let oldValue = _$observationRegistrar.willModify(self, keyPath: \.count, &_count)`
`113`		`- yield &_count`
`114`		`- guard shouldNotifyObservers(oldValue, count) else {`
`115`		`- return`
	`106`	`+ defer {`
	`107`	`+ _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual, shouldNotifyObservers)`
`116`	`108`	`}`
`117`		`- _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual)`
	`109`	`+ yield &_count`
`118`	`110`	`}`
`119`	`111`	`}`
`120`	`112`
`@@ -169,18 +161,14 @@`
`169`	`161`	`return _count`
`170`	`162`	`}`
`171`	`163`	`set {`
`172`		`- guard shouldNotifyObservers(_count, newValue) else {`
`173`		`- return`
`174`		`- }`
`175`		`- _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual)`
	`164`	`+ _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual, shouldNotifyObservers)`
`176`	`165`	`}`
`177`	`166`	`_modify {`
`178`	`167`	`let oldValue = _$observationRegistrar.willModify(self, keyPath: \.count, &_count)`
`179`		`- yield &_count`
`180`		`- guard shouldNotifyObservers(oldValue, count) else {`
`181`		`- return`
	`168`	`+ defer {`
	`169`	`+ _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual, shouldNotifyObservers)`
`182`	`170`	`}`
`183`		`- _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual)`
	`171`	`+ yield &_count`
`184`	`172`	`}`
`185`	`173`	`}`
`186`	`174`
`@@ -232,18 +220,14 @@`
`232`	`220`	`return _count`
`233`	`221`	`}`
`234`	`222`	`set {`
`235`		`- guard shouldNotifyObservers(_count, newValue) else {`
`236`		`- return`
`237`		`- }`
`238`		`- _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual)`
	`223`	`+ _$observationRegistrar.mutate(self, keyPath: \.count, &_count, newValue, _$isIdentityEqual, shouldNotifyObservers)`
`239`	`224`	`}`
`240`	`225`	`_modify {`
`241`	`226`	`let oldValue = _$observationRegistrar.willModify(self, keyPath: \.count, &_count)`
`242`		`- yield &_count`
`243`		`- guard shouldNotifyObservers(oldValue, count) else {`
`244`		`- return`
	`227`	`+ defer {`
	`228`	`+ _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual, shouldNotifyObservers)`
`245`	`229`	`}`
`246`		`- _$observationRegistrar.didModify(self, keyPath: \.count, &_count, oldValue, _$isIdentityEqual)`
	`230`	`+ yield &_count`
`247`	`231`	`}`
`248`	`232`	`}`
`249`	`233`