mobxjs / mobx-state-tree

Full-featured reactive state management without the boilerplate
https://mobx-state-tree.js.org/
MIT License
6.95k stars 642 forks source link

Reference get does not support async call #1338

Open rdewolff opened 5 years ago

rdewolff commented 5 years ago

Am currently working on some lazy loading examples, and I cannot find a way to use async call in custom references. This might be a bug or a lack of TS knowledge from my part.

Am wondering if you could have a look in this. Check the forked sandbox.

Bug report

Sandbox link or minimal reproduction code https://codesandbox.io/s/mobx-state-tree-lazy-loading-cache-transporter-async-rsgoe Interesting lines 152 to 164.

Describe the expected behavior It should be possible to use an async getter in PeopleReference.

Describe the observed behavior Currently, I cannot find a way to run the the getter with async.

mweststrate commented 5 years ago

References map identifiers to values directly. Since you try to load lazily, you don't have a value, so you can't "resolve" the identifier. Note that we can't really support async at that point either, as async is the red pill that affects all it's callees. E.g. const myPerson = myModel.someRefToAsyncPurpose would need an await to make myPerson meaningful. In other words someRefToAsyncPurpose would need to be able to return a promise, but that breaks the type contract.

What you can do instead however, is to create a .view that either returns a promise to the resolved object, or undefined / the value, depending on what semantics you want. For example:

const Cache = types.model({
   fetchedPeople: types.map(People)
}).views(self => {
  const inFlight = new Set() // Or store promises in here
  function fetchPerson(id) {
     if (inFlight.has(id) || self.fetchedPeople.has(id)) return 
     inFlight.set(id)
     fetchPersonSomehow(id).then((data) => {
        inFlight.remove(id)
        self.fetchedPeople.set(id, data)
     }).catch(() => {
       inFlight.remove(id)
     })
  }
  return {
    getPerson(id): Instance<typeof Person> | undefined {
      fetchPerson(id)
      return self.fetchedPeople(id) // if we have it, return, otherwise return undefined
      // since views are reactive, this expression will be re-evaluated if observed, once the person arrives in the fetchedPeople map.
    }
}

It is pseudo code, but hope that helps!

If you have this pattern a lot, it should be easy enough to extract a utility for it

rdewolff commented 5 years ago

Thanks for your detailed answer @mweststrate. I have been trying since then to make this work on CodePen without success. Here is the closed I can get :

https://codesandbox.io/s/mobx-state-tree-lazy-loading-cache-transporter-promises-9tcxq

The cache fetchedPeople is not getting filled. I have tried by using volatile, I have tried to use an action in the callback, I have tried all the variants I could think of but no success so far. I noticed some issues when trying to use the map storying fetchedPeople. But logging error in try catch in CodeSandBox does not show any error message ?

rdewolff commented 5 years ago

Am making progress by extracting the codesandbox project in a native react app on my computer. The error messages are now displayed and am making progress. Gimme some time to dig in it and I'll get back, hopefully with a solution and an example for the community :)

rdewolff commented 5 years ago

Here is fixed and working version 🎉

https://codesandbox.io/s/mobx-state-tree-lazy-loading-cache-transporter-promises-9tcxq

rdewolff commented 5 years ago

What would be the best way to store this as a reference to new users? Any other way to improve this example? We could clean this up in an example and include this proper lazy loading in the docs ?

sergioisidoro commented 1 year ago

Given this discussion, I think it would be nice to update the documentation, which points to custom references as the "way to go" when it comes to lazy loading. I was just attempting to make the same mistake, of making an async call in a reference resolution.

The default implementation uses the identifier cache to resolve references (See resolveIdentifier). However, it is also possible to override the resolve logic and provide your own custom resolve logic. This also makes it possible to, for example, trigger a data fetch when trying to resolve the reference (example).

(my highlight) https://mobx-state-tree.js.org/concepts/references

Also, as the example suggests, having a side effect in a view sounds problematic, and there's really no decent and clean way to reference an object that might not yet be in the tree. This is especially relevant when we get references to objects in API calls -- See the discussion https://github.com/mobxjs/mobx-state-tree/discussions/1995

coolsoftwaretyler commented 3 months ago

@sergioisidoro - sorry it took so long for anyone to get to this. I think you're correct that we should update the docs. I've just added a label and assigned myself. I'll try to get around to it soon, but if anyone else wants to write it up and send a quick PR, I'm happy to review.