Mobx: Computed function invoked regardless of whether or not the underlying params have changed.

[pdavidow]

The last test below is failing. The computed function (tickCount) seems to be invoked regardless of whether or not the underlying params have changed.

import {computed, observable} from 'mobx';
import {lcm} from 'mathjs';
import test from 'tape';

class Beat {
    constructor (rh, lh) {
        this.observed = observable({
            rh: rh,
            lh: lh
        });

        this.count = 0;

        this.tickCount = computed(() => {
            this.count++;
            return lcm(this.observed.rh, this.observed.lh);
        });
    }
};

test('Beat tick count', (assert) => {
    const beat = new Beat(3, 1);

    let before = beat.count;
    beat.tickCount.get();
    let after = beat.count;
    assert.ok(before != after, 'tickCount should be called'); // pass

    before = beat.count;
    beat.tickCount.get();
    after = beat.count;
    assert.ok(before == after, 'tickCount should NOT be called'); // FAIL

    assert.end();
});

[mweststrate]

@pdavidow as long as a computed value is not used by a reaction, it is not memoized and so it just like a normal eager evaluating function. If you would would use the tickCount in an autorun this behavior will change and you won't see unnecessary computations.

E.g. simply adding:

var disposer = autorun(() => beat.tickCount.get())
.... tests
disposer()
assert.end()

[pdavidow]

I was really hoping the functionality would be

this.observed = memoizedObservable({
            rh: rh,
            lh: lh
        });

Would that be a possibility? (For now, please update doc to explain current functionality. )

[mweststrate]

@pdavidow the reason MobX works this way is that as long as a computed value is not in use by some reaction, it can simply be ignored. MobX doesn't recompute it all, and the computation doesn't keep any other computation alive.

To keep the computation alive you can simply do the following:

var c = computed(() => expr)
var suspend = c.observe(() => noop)
c.get() // will now memoize
suspend() // stops memoizing

you could even build a simple abstraction around that:

function memoize(expr) {
  var c = computed(expr)
  var v;
  var d = c.observe(newValue => v = newValue)
  return {
    get: () => return v
    stop: () => d()
  }
}

var m = memoize(() => expr)

m.get() // memoized value
m.stop() // clean up

I hope that helps!

[pdavidow]

But I don't want to to worry about cleanup. That's what garbage-collection is for.

[mweststrate]

You have to choose.

Suppose you have:

var p = { name: "Michel" }
var b;
function sync() {
   b = p.name
}

Now you have to choose:

1. either sync always runs "somehow" (e.g each second using setInterval), keeping b in sync, but disallowing p to be garbage collected (because sync cannot be garbage collected)
2. you don't run sync automatically, and p can be GC-ed as soon as sync run out of scope

MobX uses option 2 for computed values. Meaning you have to invoke sync at some point, and if you don't sync and p will be GC-ed. So what you are doing with tickCount.get is invoking sync, which is the thing you count.

However MobX switches to strategy 1 for a computed if it is in use by a react / autorun / being observed. But that give you the responsibility to stop syncing (similar to clearing your setInterval)

(Now memoizing above is not the correct term to use. Memoizing is about caching your result based on the arguments. But the point of computed is the opposite, even without arguments, the thing is always consistent with the backing data. Memoizing () => expr would be computing expr only once and then returning that always)

Does that clarify?

[pdavidow]

Yes I see, a choice needs to be made. Thanks for clarifying.
(By _memoized_, I meant _smartMemoized_ which resets and recalcs when underlying values change.)

[mweststrate]

Yep, cached would be the better word, but if you keep computations alive, they will cache until an underlying value change by actively tracking those. But indeed, that introduces some householding in the sense that you need to dispose. (well in some use cases that is not relevant at all, when the thing should live for the entire life of the app, but that depends on the case)

[jumika]

@mweststrate

as long as a computed value is not used by a reaction, it is not memoized and so it just like a normal eager evaluating function.

I think we should make it clear in the documentation also.
One of my colleague was struggling with this for pretty long time. He did not understand why a console.log in a computed getter made output to the console every time he used that computed getter. He did not observe the getter as it was just an initial experiment. Of course it was evaluated all the time because no observation.

Edit: I see it is in the pitfalls section, but we should mention it in the computed section also.

[mweststrate]

PR welcome

Op vr 12 jul. 2019 12:23 schreef jumika [e-mail]:

@mweststrate https://github.com/mweststrate

as long as a computed value is not used by a reaction, it is not memoized
and so it just like a normal eager evaluating function.

I think we should make it clear in the documentation also.
One of my colleague was struggling with this for pretty long time. He did
not understand why a console.log in a computed getter made output to the
console every time he used that computed getter. He did not observe the
getter as it was just an initial experiment. Of course it was evaluated all
the time because no observation.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/356?email_source=notifications&email_token=AAN4NBDBP7UWCKUS3CL2YRTP7BLTPA5CNFSM4CHRWCIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZZLYEY#issuecomment-510835731,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAN4NBB6W3K55HN6R4VAX6TP7BLTPANCNFSM4CHRWCIA
.