mobxjs / mobx

Simple, scalable state management.
http://mobx.js.org
MIT License
27.55k stars 1.78k forks source link

`computed` backed by an `atom` retains old value in `action` #3680

Closed emereum closed 1 year ago

emereum commented 1 year ago

Intended outcome:

When:

then subsequent accesses of the computed within the same action should return the new value.

Actual outcome:

The computed value returns the old value on the 2nd read inside an action.

I'm not certain if this is a bug or a known subtlety with atoms, however, it appears inconsistent with a plain observable.box as shown below.

How to reproduce the issue:

import { computed, createAtom, runInAction, observable } from "mobx";

{
  console.log("Atom");

  let apples = 0;
  const hasMoreApples = new EventTarget();
  const moreApples = () => {
    apples++;
    hasMoreApples.dispatchEvent(new Event("customEvent"));
  };
  const atm = createAtom("applesAtom", () => {
    console.log("onBO");
    hasMoreApples.addEventListener("customEvent", () => atm.reportChanged());
  });
  const c1 = computed(() => {
    atm.reportObserved();
    return apples;
  });

  runInAction(() => {
    console.log(c1.get()); // Expect 0 ✓
    moreApples();
    console.log(c1.get()); // Expect 1 ✗
  });
}

{
  console.log("Box");

  const bananas = observable.box(0);

  const c2 = computed(() => {
    return bananas.get();
  });

  runInAction(() => {
    console.log(c2.get()); // Expect 0 ✓
    bananas.set(1);
    console.log(c2.get()); // Expect 1 ✓
  });
}

See Codesandbox example

Versions

Tested on MobX 6.8.0 but this behaviour can be observed in other releases, too.

mweststrate commented 1 year ago

onBecomeOserverd registers when someone subscribes to an atom (that is, someone is interested in future values), not when someone reads an atom, so that it doesn't fire is correct. reportObserved() will actually return true / false to indicate whether it was a no-op or not.

Because you don't have a reactive context (e.g. not calling the computed from autorun / reaction / observer), there is no one listening for future values. And since MobX assumes computing computed values is an idempotent, side effect free operation, it assumes it is always safe to skip the onbecome(Un)Observed cycle like it does here, without altering the output.

In other, this is behaving as intended :).

On Thu, Apr 6, 2023 at 2:26 PM Mike Emery @.***> wrote:

Intended outcome:

When:

  • running an action, and
  • accessing a computed, and
  • the computed is backed by an atom, and
  • the data tracked by the atom changes during the action,

then the atom should becomeObserved and reportChanged so the computed will re-evaluate on the subsequent accesses.

I'm not certain if this is a bug or a known subtlety with atoms, however, it appears inconsistent with a plain observable.box as shown below.

Actual outcome:

The atom does not becomeObserved, so the computed value is stale on the 2nd read inside an action.

How to reproduce the issue:

import { computed, createAtom, runInAction, observable } from "mobx";

{ console.log("Atom");

let apples = 0; const hasMoreApples = new EventTarget(); const moreApples = () => { apples++; hasMoreApples.dispatchEvent(new Event("customEvent")); }; const atm = createAtom("applesAtom", () => { console.log("onBO"); hasMoreApples.addEventListener("customEvent", () => atm.reportChanged()); }); const c1 = computed(() => { atm.reportObserved(); return apples; });

runInAction(async () => { console.log(c1.get()); // Expect 0 ✓ moreApples(); console.log(c1.get()); // Expect 1 ✗ }); }

{ console.log("Box");

const bananas = observable.box(0);

const c2 = computed(() => { return bananas.get(); });

runInAction(() => { console.log(c2.get()); // Expect 0 ✓ bananas.set(1); console.log(c2.get()); // Expect 1 ✓ }); }

See Codesandbox example https://codesandbox.io/s/crazy-yonath-mg45l4?file=/src/index.ts

Versions

Tested on MobX 6.8.0 but this behaviour can be observed in other releases, too.

— Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx/issues/3680, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBEV4HFC2ODWZSKQESDW73AB7ANCNFSM6AAAAAAWVNGLVQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>