Use WeakRef for observers

.changeset/little-buckets-impress.md

@@ -0,0 +1,5 @@
+---
+"mobx": minor
+---
+
+Use WeakRefs for computed values so that computed values with the keepAlive option can be garbage collected

docs/computeds.md

@@ -121,7 +121,6 @@ setInterval(() => {
 ```
 
 It can be overridden by setting the annotation with the `keepAlive` option ([try it out yourself](https://codesandbox.io/s/computed-3cjo9?file=/src/index.tsx)) or by creating a no-op `autorun(() => { someObject.someComputed })`, which can be nicely cleaned up later if needed.
-Note that both solutions have the risk of creating memory leaks. Changing the default behavior here is an anti-pattern.
 
 MobX can also be configured with the [`computedRequiresReaction`](configuration.md#computedrequiresreaction-boolean) option, to report an error when computeds are accessed outside of a reactive context.
 
@@ -237,4 +236,4 @@ It is recommended to set this one to `true` on very expensive computed values. I
 
 ### `keepAlive`
 
-This avoids suspending computed values when they are not being observed by anything (see the above explanation). Can potentially create memory leaks, similar to the ones discussed for [reactions](reactions.md#always-dispose-of-reactions).
+This avoids suspending computed values when they are not being observed by anything (see the above explanation).

packages/mobx/src/core/atom.ts

@@ -29,7 +29,7 @@ export class Atom implements IAtom {
     private static readonly diffValueMask_ = 0b100
     private flags_ = 0b000
 
-    observers_ = new Set<IDerivation>()
+    observers_ = new Set<WeakRef<IDerivation>>()
 
     lastAccessedBy_ = 0
     lowestObserverState_ = IDerivationState_.NOT_TRACKING_

packages/mobx/src/core/computedvalue.ts

@@ -81,7 +81,7 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
     dependenciesState_ = IDerivationState_.NOT_TRACKING_
     observing_: IObservable[] = [] // nodes we are looking at. Our value depends on these nodes
     newObserving_ = null // during tracking it's an array with new observed observers
-    observers_ = new Set<IDerivation>()
+    observers_ = new Set<WeakRef<IDerivation>>()
     runId_ = 0
     lastAccessedBy_ = 0
     lowestObserverState_ = IDerivationState_.UP_TO_DATE_

packages/mobx/src/core/globalstate.ts

@@ -71,6 +71,12 @@ export class MobXGlobals {
      */
     pendingUnobservations: IObservable[] = []
 
+    /**
+     * Set of reactions that have not yet been disposed.
+     * Inclusion in this set prevents them from being garbage collected.
+     */
+    strongRefReactions = new Set<Reaction>()
+
     /**
      * List of scheduled, not yet executed, reactions.
      */

packages/mobx/src/core/observable.ts

@@ -10,6 +10,7 @@ import {
     runReactions,
     checkIfStateReadsAreAllowed
 } from "../internal"
+import { createWeakRef } from "../utils/weakRef"
 
 export interface IDepTreeNode {
     name_: string
@@ -29,7 +30,7 @@ export interface IObservable extends IDepTreeNode {
     lowestObserverState_: IDerivationState_ // Used to avoid redundant propagations
     isPendingUnobservation: boolean // Used to push itself to global.pendingUnobservations at most once per batch.
 
-    observers_: Set<IDerivation>
+    observers_: Set<WeakRef<IDerivation>>
 
     onBUO(): void
     onBO(): void
@@ -42,8 +43,22 @@ export function hasObservers(observable: IObservable): boolean {
     return observable.observers_ && observable.observers_.size > 0
 }
 
-export function getObservers(observable: IObservable): Set<IDerivation> {
+export function getObservers(observable: IObservable): IteratorObject<IDerivation> {
     return observable.observers_
+        .keys()
+        .map(ref => ref.deref())
+        .filter(observer => observer !== undefined)
+}
+
+export function forEachObserver(observable: IObservable, callback: (d: IDerivation) => void) {
+    observable.observers_.forEach(ref => {
+        const d = ref.deref()
+        if (d === undefined) {
+            observable.observers_.delete(ref)
+            return
+        }
+        callback(d)
+    })
 }
 
 // function invariantObservers(observable: IObservable) {
@@ -68,7 +83,7 @@ export function addObserver(observable: IObservable, node: IDerivation) {
     // invariant(observable._observers.indexOf(node) === -1, "INTERNAL ERROR add already added node");
     // invariantObservers(observable);
 
-    observable.observers_.add(node)
+    observable.observers_.add(createWeakRef(node))
     if (observable.lowestObserverState_ > node.dependenciesState_) {
         observable.lowestObserverState_ = node.dependenciesState_
     }
@@ -81,7 +96,12 @@ export function removeObserver(observable: IObservable, node: IDerivation) {
     // invariant(globalState.inBatch > 0, "INTERNAL ERROR, remove should be called only inside batch");
     // invariant(observable._observers.indexOf(node) !== -1, "INTERNAL ERROR remove already removed node");
     // invariantObservers(observable);
-    observable.observers_.delete(node)
+    observable.observers_.forEach(ref => {
+        const observer = ref.deref()
+        if (observer === undefined || observer === node) {
+            observable.observers_.delete(ref)
+        }
+    })
     if (observable.observers_.size === 0) {
         // deleting last observer
         queueForUnobservation(observable)
@@ -190,7 +210,7 @@ export function propagateChanged(observable: IObservable) {
     observable.lowestObserverState_ = IDerivationState_.STALE_
 
     // Ideally we use for..of here, but the downcompiled version is really slow...
-    observable.observers_.forEach(d => {
+    forEachObserver(observable, d => {
         if (d.dependenciesState_ === IDerivationState_.UP_TO_DATE_) {
             if (__DEV__ && d.isTracing_ !== TraceMode.NONE) {
                 logTraceInfo(d, observable)
@@ -210,7 +230,7 @@ export function propagateChangeConfirmed(observable: IObservable) {
     }
     observable.lowestObserverState_ = IDerivationState_.STALE_
 
-    observable.observers_.forEach(d => {
+    forEachObserver(observable, d => {
         if (d.dependenciesState_ === IDerivationState_.POSSIBLY_STALE_) {
             d.dependenciesState_ = IDerivationState_.STALE_
             if (__DEV__ && d.isTracing_ !== TraceMode.NONE) {
@@ -233,7 +253,7 @@ export function propagateMaybeChanged(observable: IObservable) {
     }
     observable.lowestObserverState_ = IDerivationState_.POSSIBLY_STALE_
 
-    observable.observers_.forEach(d => {
+    forEachObserver(observable, d => {
         if (d.dependenciesState_ === IDerivationState_.UP_TO_DATE_) {
             d.dependenciesState_ = IDerivationState_.POSSIBLY_STALE_
             d.onBecomeStale_()

packages/mobx/src/core/reaction.ts

@@ -74,7 +74,9 @@ export class Reaction implements IDerivation, IReactionPublic {
         private onInvalidate_: () => void,
         private errorHandler_?: (error: any, derivation: IDerivation) => void,
         public requiresObservable_?
-    ) {}
+    ) {
+        globalState.strongRefReactions.add(this)
+    }
 
     get isDisposed() {
         return getFlag(this.flags_, Reaction.isDisposedMask_)
@@ -223,6 +225,7 @@ export class Reaction implements IDerivation, IReactionPublic {
     dispose() {
         if (!this.isDisposed) {
             this.isDisposed = true
+            globalState.strongRefReactions.delete(this)
             if (!this.isRunning) {
                 // if disposed while running, clean up later. Maybe not optimal, but rare case
                 startBatch()

packages/mobx/src/utils/weakRef.ts

@@ -0,0 +1,12 @@
+import { getGlobal } from "../internal"
+
+export function createWeakRef<T extends WeakKey>(item: T): WeakRef<T> {
+    const global = getGlobal()
+    if (global.WeakRef) {
+        return new WeakRef(item)
+    }
+    return {
+        deref: () => item,
+        [Symbol.toStringTag]: "WeakRef"
+    }
+}