fix: earlier onBecome(Un)Observed disposer will not dispose later lis…

…tener (fix #1537)
mobxjs · Dec 6, 2018 · 25eb682 · 25eb682
1 parent d55c50a
commit 25eb682
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 5 deletions.
diff --git a/src/api/become-observed.ts b/src/api/become-observed.ts
@@ -5,9 +5,19 @@ import {
     Lambda,
     ObservableMap,
     fail,
-    getAtom
+    getAtom,
+    once
 } from "../internal"
 
+export interface atomListeners {
+    [propName: string]: {
+        observedListeners?: Set<Lambda>
+        unobservedListeners?: Set<Lambda>
+    }
+}
+
+const atomsObservedListeners: atomListeners = {}
+
 export function onBecomeObserved(
     value: IObservable | IComputedValue<any> | IObservableArray<any> | ObservableMap<any, any>,
     listener: Lambda
@@ -43,11 +53,27 @@ function interceptHook(hook: "onBecomeObserved" | "onBecomeUnobserved", thing, a
     if (typeof orig !== "function")
         return fail(process.env.NODE_ENV !== "production" && "Not an atom that can be (un)observed")
 
-    atom[hook] = function() {
-        orig.call(this)
-        cb.call(this)
+    const initHooks = () => {
+        atom[hook] = function() {
+            ;(atomsObservedListeners[atom.name][hook] as Set<Lambda>).forEach(listener =>
+                listener()
+            )
+        }
     }
+
+    if (atomsObservedListeners[atom.name] && atomsObservedListeners[atom.name][hook]) {
+        ;(atomsObservedListeners[atom.name][hook] as Set<Lambda>).add(cb)
+    } else {
+        if (atomsObservedListeners[atom.name]) {
+            atomsObservedListeners[atom.name][hook] = new Set([cb])
+            initHooks()
+        } else {
+            atomsObservedListeners[atom.name] = { [hook]: new Set([cb]) }
+            initHooks()
+        }
+    }
+
     return function() {
-        atom[hook] = orig
+        ;(atomsObservedListeners[atom.name][hook] as Set<Lambda>).delete(cb)
     }
 }
diff --git a/test/base/extras.js b/test/base/extras.js
@@ -399,6 +399,53 @@ test("onBecome(Un)Observed - less simple", () => {
     expect(events.length).toBe(0)
 })
 
+test("onBecomeObserved correctly disposes second listener #1537", () => {
+    const x = mobx.observable.box(3)
+    const events = []
+    const d1 = mobx.onBecomeObserved(x, "a", () => {
+        events.push("a observed")
+    })
+    const d2 = mobx.onBecomeObserved(x, "b", () => {
+        events.push("b observed")
+    })
+    d1()
+    mobx.reaction(() => x.get(), () => {})
+    expect(events.length).toBe(1)
+    expect(events).toEqual(["b observed"])
+})
+
+test("onBecomeObserved correctly disposes second listener #1537", () => {
+    const x = mobx.observable.box(3)
+    const events = []
+    const d1 = mobx.onBecomeUnobserved(x, "a", () => {
+        events.push("a unobserved")
+    })
+    const d2 = mobx.onBecomeUnobserved(x, "b", () => {
+        events.push("b unobserved")
+    })
+    d1()
+    const d3 = mobx.reaction(() => x.get(), () => {})
+    d3()
+    expect(events.length).toBe(1)
+    expect(events).toEqual(["b unobserved"])
+})
+
+test("onBecomeUnobserved correctly disposes second listener #1537", () => {
+    const x = mobx.observable.box(3)
+    const events = []
+    const d1 = mobx.onBecomeUnobserved(x, "a", () => {
+        events.push("a unobserved")
+    })
+    const d2 = mobx.onBecomeUnobserved(x, "b", () => {
+        events.push("b unobserved")
+    })
+    d1()
+    const d3 = mobx.reaction(() => x.get(), () => {})
+    d3()
+    expect(events.length).toBe(1)
+    expect(events).toEqual(["b unobserved"])
+})
+
 test("deepEquals should yield correct results for complex objects #1118 - 1", () => {
     const d2016jan1 = new Date("2016-01-01")
     const d2016jan1_2 = new Date("2016-01-01")