From 7d42f47abef1caf2277a4c9502c1193eb4220a0b Mon Sep 17 00:00:00 2001 From: Wee Date: Fri, 7 Dec 2018 01:07:50 +0800 Subject: [PATCH] fix: earlier onBecome(Un)Observed disposer will not dispose later listener (fix #1537) --- src/api/become-observed.ts | 21 ++++++++++----- src/core/atom.ts | 26 ++++++++++++++----- src/core/computedvalue.ts | 15 +++++++++-- test/base/extras.js | 53 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 14 deletions(-) diff --git a/src/api/become-observed.ts b/src/api/become-observed.ts index 1646991dc..c9de42206 100644 --- a/src/api/become-observed.ts +++ b/src/api/become-observed.ts @@ -38,16 +38,25 @@ function interceptHook(hook: "onBecomeObserved" | "onBecomeUnobserved", thing, a const atom: IObservable = typeof arg2 === "string" ? getAtom(thing, arg2) : (getAtom(thing) as any) const cb = typeof arg2 === "string" ? arg3 : arg2 - const orig = atom[hook] + const listenersKey = `${hook}Listeners` + + if (!atom[listenersKey]) { + atom[listenersKey] = new Set([cb]) + } else { + ;(atom[listenersKey] as Set).add(cb) + } + const orig = atom[hook] 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) - } return function() { - atom[hook] = orig + const hookListeners = atom[listenersKey] as Set | undefined + if (hookListeners) { + hookListeners.delete(cb) + if (hookListeners.size === 0) { + delete atom[listenersKey] + } + } } } diff --git a/src/core/atom.ts b/src/core/atom.ts index 274ba2171..85a37f9e9 100644 --- a/src/core/atom.ts +++ b/src/core/atom.ts @@ -11,6 +11,7 @@ import { reportObserved, startBatch } from "../internal" +import { Lambda } from "../utils/utils" export const $mobx = Symbol("mobx administration") @@ -33,12 +34,19 @@ export class Atom implements IAtom { */ constructor(public name = "Atom@" + getNextId()) {} - public onBecomeUnobserved() { - // noop - } + public onBecomeObservedListeners: Set | undefined + public onBecomeUnobservedListeners: Set | undefined public onBecomeObserved() { - /* noop */ + if (this.onBecomeObservedListeners) { + this.onBecomeObservedListeners.forEach(listener => listener()) + } + } + + public onBecomeUnobserved() { + if (this.onBecomeUnobservedListeners) { + this.onBecomeUnobservedListeners.forEach(listener => listener()) + } } /** @@ -71,7 +79,13 @@ export function createAtom( onBecomeUnobservedHandler: () => void = noop ): IAtom { const atom = new Atom(name) - onBecomeObserved(atom, onBecomeObservedHandler) - onBecomeUnobserved(atom, onBecomeUnobservedHandler) + // default `noop` listener will not initialize the hook Set + if (onBecomeObservedHandler !== noop) { + onBecomeObserved(atom, onBecomeObservedHandler) + } + + if (onBecomeUnobservedHandler !== noop) { + onBecomeUnobserved(atom, onBecomeUnobservedHandler) + } return atom } diff --git a/src/core/computedvalue.ts b/src/core/computedvalue.ts index 2dbf56845..e9b2bd9d7 100644 --- a/src/core/computedvalue.ts +++ b/src/core/computedvalue.ts @@ -124,9 +124,20 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva propagateMaybeChanged(this) } - onBecomeUnobserved() {} + public onBecomeObservedListeners: Set | undefined + public onBecomeUnobservedListeners: Set | undefined - onBecomeObserved() {} + public onBecomeObserved() { + if (this.onBecomeObservedListeners) { + this.onBecomeObservedListeners.forEach(listener => listener()) + } + } + + public onBecomeUnobserved() { + if (this.onBecomeUnobservedListeners) { + this.onBecomeUnobservedListeners.forEach(listener => listener()) + } + } /** * Returns the current value of this computed value. diff --git a/test/base/extras.js b/test/base/extras.js index 4772156a1..e5c48b7e8 100644 --- a/test/base/extras.js +++ b/test/base/extras.js @@ -399,6 +399,59 @@ 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.onBecomeObserved(x, "a", () => { + events.push("a observed") + }) + const d2 = mobx.onBecomeObserved(x, "b", () => { + events.push("b observed") + }) + d1() + const d3 = mobx.reaction(() => x.get(), () => {}) + d3() + expect(events.length).toBe(1) + expect(events).toEqual(["b observed"]) + d2() + const d4 = mobx.reaction(() => x.get(), () => {}) + expect(events).toEqual(["b observed"]) +}) + +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"]) + d2() + const d4 = mobx.reaction(() => x.get(), () => {}) + 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")