diff --git a/src/core/computedvalue.ts b/src/core/computedvalue.ts index 21451ea9d..2dbf56845 100644 --- a/src/core/computedvalue.ts +++ b/src/core/computedvalue.ts @@ -91,7 +91,6 @@ export class ComputedValue implements IObservable, IComputedValue, IDeriva private equals: IEqualsComparer 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 implements IObservable, IComputedValue, 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 implements IObservable, IComputedValue, 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) => void, fireImmediately?: boolean): Lambda { diff --git a/test/base/observables.js b/test/base/observables.js index cedab1bba..e2f78d4af 100644 --- a/test/base/observables.js +++ b/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", () => {