From 152f2833cbb4e49eda9569218447a874b24e5c97 Mon Sep 17 00:00:00 2001 From: Pavel Mayorov Date: Mon, 8 Jul 2019 16:07:20 +0500 Subject: [PATCH] use onBecomeUnobserved for ObservableMap._hasMap gc, fixes #2031 --- src/types/observablemap.ts | 31 ++++++++++++++++++------------- test/base/map.js | 23 ++++++++++++++++++++++- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/src/types/observablemap.ts b/src/types/observablemap.ts index 07aed5d36..f4b558310 100644 --- a/src/types/observablemap.ts +++ b/src/types/observablemap.ts @@ -31,6 +31,7 @@ import { stringifyKey, transaction, untracked, + onBecomeUnobserved, globalState } from "../internal" @@ -108,8 +109,22 @@ export class ObservableMap } has(key: K): boolean { - if (this._hasMap.has(key)) return this._hasMap.get(key)!.get() - return this._updateHasMapEntry(key, false).get() + if (!globalState.trackingDerivation) return this._has(key) + + let entry = this._hasMap.get(key) + if (!entry) { + // todo: replace with atom (breaking change) + const newEntry = (entry = new ObservableValue( + this._has(key), + referenceEnhancer, + `${this.name}.${stringifyKey(key)}?`, + false + )) + this._hasMap.set(key, newEntry) + onBecomeUnobserved(newEntry, () => this._hasMap.delete(key)) + } + + return entry.get() } set(key: K, value: V) { @@ -170,21 +185,11 @@ export class ObservableMap return false } - private _updateHasMapEntry(key: K, value: boolean): ObservableValue { - // optimization; don't fill the hasMap if we are not observing, or remove entry if there are no observers anymore + private _updateHasMapEntry(key: K, value: boolean) { let entry = this._hasMap.get(key) if (entry) { entry.setNewValue(value) - } else { - entry = new ObservableValue( - value, - referenceEnhancer, - `${this.name}.${stringifyKey(key)}?`, - false - ) - this._hasMap.set(key, entry) } - return entry } private _updateValue(key: K, newValue: V | undefined) { diff --git a/test/base/map.js b/test/base/map.js index 3b81a725c..45e653055 100644 --- a/test/base/map.js +++ b/test/base/map.js @@ -227,7 +227,28 @@ test("cleanup", function() { disposer() expect(aValue).toBe(2) expect(observable.observers.size).toBe(0) - expect(x._hasMap.get("a").observers.size).toBe(0) + expect(x._hasMap.has("a")).toBe(false) +}) + +test("getAtom encapsulation leak test", function() { + const x = map({}) + + let disposer = autorun(function() { + x.has("a") + }) + + let atom = mobx.getAtom(x, "a") + + disposer() + + expect(x._hasMap.get("a")).toBe(undefined) + + disposer = autorun(function() { + x.has("a") + atom && atom.reportObserved() + }) + + expect(x._hasMap.get("a")).not.toBe(atom) }) test("strict", function() {