fix: Fix Set interceptors (#4528)

* fix(observableset): use the new value returned from the interceptor

* test(set): add a couple of interceptor tests

* chore(changeset): add patch entry

---------

Co-authored-by: k-g-a <[email protected]>

Author: k-g-a

.changeset/set-interceptors.md

@@ -0,0 +1,5 @@
+---
+"mobx": patch
+---
+
+Fix observable.set not respecting the new value from interceptors 

packages/mobx/__tests__/v5/base/set.js

@@ -474,3 +474,48 @@ describe("Observable Set methods are reactive", () => {
         expect(c).toBe(3)
     })
 })
+
+
+describe("Observable Set interceptors", () => {
+
+    let s = set()
+
+    beforeEach(() => {
+        s = set()
+    })
+
+    test("Add does not add value if interceptor returned no change", () => {
+        mobx.intercept(s, (change) => {
+            if(change.type === 'add' && change.newValue === 2) {
+                return undefined;
+            }
+
+            return change;
+        })
+
+        s.add(1);
+        s.add(2);
+
+        expect([...s]).toStrictEqual([1]);
+
+
+    })
+
+    test("Add respects newValue from interceptor", () => {
+
+        mobx.intercept(s, (change) => {
+            if(change.type === 'add' && change.newValue === 2) {
+                change.newValue = 10;
+            }
+
+            return change;
+        })
+
+        s.add(1);
+        s.add(2);
+
+        expect([...s]).toStrictEqual([1, 10])
+    })
+
+
+})

packages/mobx/src/types/observableset.ts

@@ -51,17 +51,20 @@ export type ISetDidChange<T = any> =
           oldValue: T
       }
 
+export type ISetWillDeleteChange<T = any> = {
+    type: "delete"
+    object: ObservableSet<T>
+    oldValue: T
+};
+export type ISetWillAddChange<T = any> = {
+    type: "add"
+    object: ObservableSet<T>
+    newValue: T
+};
+
 export type ISetWillChange<T = any> =
-    | {
-          type: "delete"
-          object: ObservableSet<T>
-          oldValue: T
-      }
-    | {
-          type: "add"
-          object: ObservableSet<T>
-          newValue: T
-      }
+    | ISetWillDeleteChange<T>
+    | ISetWillAddChange<T>
 
 export class ObservableSet<T = any> implements Set<T>, IInterceptable<ISetWillChange>, IListenable {
     [$mobx] = ObservableSetMarker
@@ -120,16 +123,18 @@ export class ObservableSet<T = any> implements Set<T>, IInterceptable<ISetWillCh
     add(value: T) {
         checkIfStateModificationsAreAllowed(this.atom_)
         if (hasInterceptors(this)) {
-            const change = interceptChange<ISetWillChange<T>>(this, {
+            const change = interceptChange<ISetWillAddChange<T>>(this, {
                 type: ADD,
                 object: this,
                 newValue: value
             })
             if (!change) {
                 return this
             }
-            // ideally, value = change.value would be done here, so that values can be
-            // changed by interceptor. Same applies for other Set and Map api's.
+
+            // implemented reassignment same as it's done for ObservableMap
+            value = change.newValue!;
+
         }
         if (!this.has(value)) {
             transaction(() => {
@@ -164,7 +169,7 @@ export class ObservableSet<T = any> implements Set<T>, IInterceptable<ISetWillCh
 
     delete(value: T) {
         if (hasInterceptors(this)) {
-            const change = interceptChange<ISetWillChange<T>>(this, {
+            const change = interceptChange<ISetWillDeleteChange<T>>(this, {
                 type: DELETE,
                 object: this,
                 oldValue: value