Mobx: Question: side effects for caching in @computed values with existing observers

[aeroheim]

Problem

hi! I'm new to mobx and am trying to implement caching for my application's state using @computed values but am running into an error. I have a contrived example that hopefully demonstrates my intent:

class Cache {
  @observable _cachedValue;
  @computed get value() {
    if (!this._cachedValue) {
     // for context, this is a call into a native node module's synchronous API
     // to retrieve some data - no fetches or promises here!
      this._cachedValue = someApiCallToGetValue();
    }
    return this._cachedValue;
  }

  @action invalidateCache = () => {
    // value lazy-loads again next time computed value is observed
    this._cachedValue = undefined;
  }
}

On the initial observe or when the cache is invalidated via invalidateCache, the computed value should refresh the @observable with someApiCallToGetValue() as a side effect before returning it.

I get the following error when I use this approach however:
Error: [mobx] Computed values are not allowed to cause side effects by changing observables that are already being observed.

Question

Is it possible to accomplish my desired side-effect in a computed value with existing observers? If not, is there a recommended alternative approach I should be taking?

Alternatives considered

• It's been mentioned in #307 that a lazy-loading pattern with intentional side effects _is_ useful and can be achieved with onBecome(Un)Observed events added in #323, but it doesn't seem to solve this issue as I need to have my computed value able to achieve a side effect _while_ being observed.
• Another alternative after looking at #1534 and #1799 was to use @computed({keepAlive: true}) so that the computed value _itself_ becomes the cached value. However it seems like this would require the usage of a dummy @observable referenced by my computed value so that invalidateCache can update my computed value, which is _very_ hacky and doesn't seem like an intended use case.
• I've tried using ES6 proxies which were also mentioned in #307. I ended up with a proxy that handled lazy-loading and cache invalidation around the original cache which now only contained the @observables and nothing else. but I was seeing the following error: Error: [mobx] Side effects like changing state are not allowed at this point. Are you trying to modify state from, for example, the render function of a React component? when attempting to interact with my proxy in simple use cases such as onClick={() => this.props.injectedCache.value = 'test'} in a React render() function. It seemed like adding dumb @computed get/set wrappers around my @observables seemed to solve the issue, although I haven't attempted to debug this any further as to see why.

[urugator]

Why do you use @computed in the first place? If I follow correctly someApiCallToGetValue() doesn't read anything observable, the computed is absolutely useless, use plain getter.

[aeroheim]

hey @urugator , thanks for helping me out! You're absolutely right about the @computed being useless - it wasn't clear to me whether a plain getter would still be observed by mobx (hence why I added the @computed), but I removed it and my React components were still re-rendering as expected. Rookie mistake on my end :-)

I am running into a new error though when my plain getter sets my observable: [mobx] Side effects like changing state are not allowed at this point. Are you trying to modify state from, for example, the render function of a React component?

I've setup a live example of this in a codesandbox. FYI I couldn't get the error to display properly in Chrome, you may need to use another browser (e.g Firefox).

It seems like mobx is still unhappy about any kind of side-effects on observables when they're being observed, so I'm unsure if my approach is actually compatible with mobx or not.

[urugator]

Ah right, that was naive, the update still happens during derivation (render) ... have to think about it a bit more ...

[urugator]

The reason why the cached value is observable is that the invalidation should cause a component update?

[mweststrate]

This pattern is not unusual in itself, where stuff is lazy loaded upon
first usage. What makes it unique is that it happens _in sync_, so it
becomes part of the computation (if you would kick off the cache fetch in a
timeout there would be no problem). Could you elaborate a bit more on this?
E.g, why you can't do return someApiCallToGetValue()? Why do you want to
store it's output in an observable? If _cachedValue was not observable,
nothing would really change correct? Since the only reason for the thing to
be changed is calling the computed value? Probably missing something, but
some elaboration on the bigger picture would be nice :)

On Thu, Feb 21, 2019 at 7:20 PM urugator [e-mail] wrote:

Ah right, that was naive, the update still happens during derivation
(render) ... have to think about it a bit more ...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1910#issuecomment-466108884, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhFsPkWpvfw1tLD2AECqZMjNbZXHFks5vPuN2gaJpZM4bGbUk
.

[aeroheim]

@urugator Yes. If any components are already observing it, they will update (and the first one that does will cause the value to be cached). If there are no observers, the value will be cached the first time a component renders with it.

[urugator]

Ok here is the idea:

  _cachedValue;
  @observable _getCachedValue = this.makeGetter();

  get value() {
    return this._getCachedValue();
  }

  @action invalidateCache = () => {        
    this._getCachedValue = this.makeGetter();
  }

  _makeGetter() {
    return () => {
      if (!this._cachedValue) {
        return this._cachedValue = someApiCallToGetValue();
      }
      return this._cachedValue;
    }
  }

or possibly (same idea, but we use computed as cache)

  @observable _getCachedValue = this.makeGetter();

  @computed({ keepAlive: true }) get value() {
    return this._getCachedValue();
  }

  @action invalidateCache = () => {
    this._getCachedValue = this.makeGetter()
  }

  _makeGetter() {
    return () => someApiCallToGetValue();
  }

hopefully I got it right :)

[urugator]

It's still quite hacky though, simple timestamp/counter would be probably easier to follow...

[aeroheim]

@mweststrate thanks for also taking a look!

Preface: I'm new to mobx so it's very likely that I may be misunderstanding it - don't hesitate to correct me on anything that may be wrong :-)

Could you elaborate a bit more on this? E.g, why you can't do return someApiCallToGetValue()? Why do you want to store it's output in an observable? If _cachedValue was not observable, nothing would really change correct? Since the only reason for the thing to be changed is calling the computed value?

someApiCallToGetValue() can be expensive, hence why I'm trying to cache the value it returns. My goal was to let my React components be able to reference these values as if they were just regular observables without having to deal with or know about any of the caching going on behind the scenes. I didn't see any support for being able to lazy-load _and_ programmatically invalidate observables, so I decided that I would implement that logic myself by wrapping my observable with a getter. My understanding (which I now believe is incorrect) was that the @computed was necessary in order for mobx to correctly track my getter so that my React components would update as expected.

Doing the synchronous someApiCallToGetValue() in a timeout is feasible, although it means I need to have my React components accept placeholder values while they wait for another render with the actual value. It may be the ideal solution in the end though as having an expensive synchronous API call block the rendering may be bad (at least on initial render or after invalidation).

@urugator this took me a bit to wrap my head around - it's quite a clever solution :-)
It does solve my issue in the end, and ultimately I'm fine with using that approach. Thanks for helping out!

I'm leaving this open in case there are any more thoughts on this - if not, feel free to close this.

[urugator]

Not sure if there is a simple way around that. The problem basically is that it violates one way data flow.
If you would want to do that in react alone, you would have to "hydrate" the cache in componentDidMount or componentDidUpdate or use an imperative code in render with extra state just for invalidation (similary to extra observable field).

[mweststrate]

I'm probably missing something, but if the _first_ reader initializes the field, then there is never a case where other components are already observing? So any form of caching would do, e.g.:

class Cache {
  _cachedValue;
  get value() {
    if (!this._cachedValue) {
     // for context, this is a call into a native node module's synchronous API
     // to retrieve some data - no fetches or promises here!
      this._cachedValue = someApiCallToGetValue();
    }
    return this._cachedValue;
  }
}

But then again, I didn't grok what you meant with " // value lazy-loads again next time computed value is observed"

That being said, you can force to skip that warning at own risk by using _allowStateChanges(true, () => { this.observableValue = someValue } inside your computed. It is used for example by https://github.com/mobxjs/mobx-utils#fromresource and https://github.com/mobxjs/mobx-utils#lazyobservable to so similar lazy initializations. (altough they don't have cleanup on onBecomeObserved connected to them, but I think that is easy to add).

[urugator]

@mweststrate The problem isn't the cache, but it's invalidation. I am imagining there is a "refresh" button, which calls invalidateCache - it has to notify observers to retrieve new value...

[mweststrate]

Ahhh thanks. I had the feeling I was missing something 😊.

Op do 21 feb. 2019 21:29 schreef urugator [e-mail]:

@mweststrate https://github.com/mweststrate The problem isn't the
cache, but it's invalidation. I am imagining there is a "refresh" button,
which calls invalidateCache - it has to notify observers to retrieve new
value...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1910#issuecomment-466154192, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhB7cQSskP5IdFV0rV_YI-F4v7Wleks5vPwGMgaJpZM4bGbUk
.

[mweststrate]

TL,DR:

import { observable, computed, action, _allowStateChanges } from "mobx"

class Cache {
  @observable _cachedValue;
  @computed get value() {
    if (!this._cachedValue) {
     // for context, this is a call into a native node module's synchronous API
     // to retrieve some data - no fetches or promises here!
     _allowStateChanges(true, () => {
        this._cachedValue = someApiCallToGetValue();
     })
    }
    return this._cachedValue;
  }

  @action invalidateCache = () => {
    // value lazy-loads again next time computed value is observed
    this._cachedValue = undefined;
  }
}

[mweststrate]

@aeroheim for any future readers: did it solve the issue?

[aeroheim]

Using _allowStateChanges still seems to result in the original error: [mobx] Computed values are not allowed to cause side effects by changing observables that are already being observed.

codesandbox.io for demonstration.

[mweststrate]

Ha MobX it's even more protective about keeping it's mental model of a pure flow of state -> derivation -> effect. I tried tricking it by simply dropping the @computed decorator (since it is just caching a property lookup anyway, it is not adding much). And then React started to throw, reminding me why it is good to stay to the pure model :).

So if this warning isn't throw, React will start to generate warnings for exactly the same reason. Because the data flow becomes render -> read value -> change cache -> MobX detects change in cache -> MobX schedules a render -> React prints a warning: A render caused a render, so there are side effects!.

Going down that path made me realize we are looking in the wrong direction. We don't want to cause a change by loading, instead, we just want to abstract away whole the loading part and build our own observable concept; the cache. The right building block for creating our own custom observable data structures is createAtom; all observable datastructures are based on that, see: https://mobx.js.org/refguide/extending.html

Anyway, realizing that it become immediately quite simple, as demonstrated here:

https://codesandbox.io/s/pwl87n09kq

A neat benefit of createAtom, is that it allows you actually to hook into when an observable is first, or no longer used, and we can use that to potentially automatically discard the cache, as demonstrated here:

https://codesandbox.io/s/2olv8v8090

Sorry for realizing this so late, was for too long just staring at how to solve your code, rather than how to solve your problem :).

The clue is that our model is now still pure: reading state only _observes_ things. We report that our cache has been used. Writing the cache, reports a that the concept of our cache has _changed_.

[mweststrate]

TL,DR:

import { action, createAtom } from "mobx";

class Cache {
  atom = createAtom("cache");
  _cachedValue;

  get value() {
    this.atom.reportObserved();
    if (!this._cachedValue) {
      this._cachedValue = "cached " + Math.random();
    }
    return this._cachedValue;
  }

  @action
  invalidateCache = () => {
    this._cachedValue = undefined;
    this.atom.reportChanged(true);
  };
}

[aeroheim]

Sorry about the slow response - I tested out atoms as suggested in my application and it seems like a perfect fit for this use case. It's great to see that mobx has this flexibility! Thanks for the help!

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.