Merge pull request #1799 from mobxjs/smarter-keep-alive

Smarter keep alive

src/core/computedvalue.ts

@@ -91,7 +91,6 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
     private equals: IEqualsComparer<any>
     private requiresReaction: boolean
     private keepAlive: boolean
-    private firstGet: boolean = true
 
     /**
      * Create a new computed value based on a function expression.
@@ -134,12 +133,8 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
      * Will evaluate its computation first if needed.
      */
     public get(): T {
-        if (this.keepAlive && this.firstGet) {
-            this.firstGet = false
-            autorun(() => this.get())
-        }
         if (this.isComputing) fail(`Cycle detected in computation ${this.name}: ${this.derivation}`)
-        if (globalState.inBatch === 0 && this.observers.size === 0) {
+        if (globalState.inBatch === 0 && this.observers.size === 0 && !this.keepAlive) {
             if (shouldCompute(this)) {
                 this.warnAboutUntrackedRead()
                 startBatch() // See perf test 'computed memoization'
@@ -235,8 +230,10 @@ export class ComputedValue<T> implements IObservable, IComputedValue<T>, IDeriva
     }
 
     suspend() {
-        clearObserving(this)
-        this.value = undefined // don't hold on to computed value!
+        if (!this.keepAlive) {
+            clearObserving(this)
+            this.value = undefined // don't hold on to computed value!
+        }
     }
 
     observe(listener: (change: IValueDidChange<T>) => void, fireImmediately?: boolean): Lambda {

test/base/observables.js

@@ -1877,7 +1877,40 @@ test("can make non-extenible objects observable", () => {
     expect(mobx.isObservableProp(o, "x")).toBeTruthy()
 })
 
-test("keeping computed properties alive works", () => {
+test("keeping computed properties alive does not run before access", () => {
+    let calcs = 0
+    observable(
+        {
+            x: 1,
+            get y() {
+                calcs++
+                return this.x * 2
+            }
+        },
+        {
+            y: mobx.computed({ keepAlive: true })
+        }
+    )
+
+    expect(calcs).toBe(0) // initially there is no calculation done
+})
+
+test("(for objects) keeping computed properties alive does not run before access", () => {
+    let calcs = 0
+    class Foo {
+        @observable x = 1
+        @computed({ keepAlive: true })
+        get y() {
+            calcs++
+            return this.x * 2
+        }
+    }
+    new Foo()
+
+    expect(calcs).toBe(0) // initially there is no calculation done
+})
+
+test("keeping computed properties alive runs on first access", () => {
     let calcs = 0
     const x = observable(
         {
@@ -1893,17 +1926,76 @@ test("keeping computed properties alive works", () => {
     )
 
     expect(calcs).toBe(0)
-    expect(x.y).toBe(2)
+    expect(x.y).toBe(2) // perform calculation on access
     expect(calcs).toBe(1)
-    expect(x.y).toBe(2)
-    expect(calcs).toBe(1) // kept alive!
+})
+
+test("keeping computed properties alive caches values on subsequent accesses", () => {
+    let calcs = 0
+    const x = observable(
+        {
+            x: 1,
+            get y() {
+                calcs++
+                return this.x * 2
+            }
+        },
+        {
+            y: mobx.computed({ keepAlive: true })
+        }
+    )
 
-    x.x = 3
-    expect(calcs).toBe(2) // reactively updated
+    expect(x.y).toBe(2) // first access: do calculation
+    expect(x.y).toBe(2) // second access: use cached value, no calculation
+    expect(calcs).toBe(1) // only one calculation: cached!
+})
+
+test("keeping computed properties alive does not recalculate when dirty", () => {
+    let calcs = 0
+    const x = observable(
+        {
+            x: 1,
+            get y() {
+                calcs++
+                return this.x * 2
+            }
+        },
+        {
+            y: mobx.computed({ keepAlive: true })
+        }
+    )
+
+    expect(x.y).toBe(2) // first access: do calculation
+    expect(calcs).toBe(1)
+    x.x = 3 // mark as dirty: no calculation
+    expect(calcs).toBe(1)
     expect(x.y).toBe(6)
 })
 
-test("keeping computed properties alive works for objects", () => {
+test("keeping computed properties alive recalculates when accessing it dirty", () => {
+    let calcs = 0
+    const x = observable(
+        {
+            x: 1,
+            get y() {
+                calcs++
+                return this.x * 2
+            }
+        },
+        {
+            y: mobx.computed({ keepAlive: true })
+        }
+    )
+
+    expect(x.y).toBe(2) // first access: do calculation
+    expect(calcs).toBe(1)
+    x.x = 3 // mark as dirty: no calculation
+    expect(calcs).toBe(1)
+    expect(x.y).toBe(6) // second access: do calculation because it is dirty
+    expect(calcs).toBe(2)
+})
+
+test("(for objects) keeping computed properties alive recalculates when accessing it dirty", () => {
     let calcs = 0
     class Foo {
         @observable x = 1
@@ -1915,15 +2007,12 @@ test("keeping computed properties alive works for objects", () => {
     }
     const x = new Foo()
 
-    expect(calcs).toBe(0)
-    expect(x.y).toBe(2)
+    expect(x.y).toBe(2) // first access: do calculation
     expect(calcs).toBe(1)
-    expect(x.y).toBe(2)
-    expect(calcs).toBe(1) // kept alive!
-
-    x.x = 3
-    expect(calcs).toBe(2) // reactively updated
-    expect(x.y).toBe(6)
+    x.x = 3 // mark as dirty: no calculation
+    expect(calcs).toBe(1)
+    expect(x.y).toBe(6) // second access: do calculation because it is dirty
+    expect(calcs).toBe(2)
 })
 
 test("tuples", () => {