Idea: built-in support for promises

[mweststrate]

This proposal aims to making working with promises easier.

Some ideas:

1. Expose observable.promise(Promise) than returns { state, value, promise }. In other words, fromPromise of the mobx-utils package
2. observable(Promise) and @observable x = Promise will automatically convert to a promise observable (e.g. call observable.promise)
   3 Allow @computed to return Promises, and if so, convert this again in a promise structure.

Example code:

class User {
    @observable name

    @computed
    get profile() {
      // return a promise
      return fetch(`/endpoint/${name}`)
    }

    @computed
    get profileImageUrl() {
       // pluck the image from the profile promise
       return this.profile.state === "fullfilled"
          ? this.profile.value.image
          : "/defaultavatar.png"
    }

    @computed({ initialValue: [] })
    get tags() {
       // chain another promise onto the profile promise
       return this.profile.then(() =>
          fetch(`/endpoint/${name}/tags`)
       })
    }

    @computed
    get isLoading() {
       // are we fetching data?
       return this.profile.state === "pending" || this.tags.state === "pending"
    }

    @computed
    get isLoadingTakingLong() {
      // provide spinner if loading takes long!
      return Promise.race([
         new Promise(resolve => setTimeout(() => resolve(true), 1000),
         new Promise(resolve => this.isLoading.then((done) => { if (done) resolve(false) })
      ])
    }
}

const UserView = observer(({ user }) => (
   <div>
       <h1>{user.name}<h1/>
       { user.isLoadingTakingLong.value && <Spinner /> }
       <img src={user.profileImageUrl} />
       { user.tags.value.join(" - ") }
   </div>
))

I checked, this will even work correct with debug tools like trace.

Problems

The problem with the decorators is that their return type signature is incorrect; as the type Promise<T> would be converted into Promise<T> & { state: "pending" | "fullfilled" | "error", value: T | undefined | error }

Possible solutions:

@computed
 get profile() {
      // upcast
      return fetch(`/endpoint/${name}`) as IObservablePromise
}

@computed
get profile() {
      // wrapper fn (needing a better name)
      return observablify(`/endpoint/${name}`) 
}

// ... but still:
@computed
 get profile() {
      // upcast
      const p fetch(`/endpoint/${name}`) as IObservablePromise
      p.state // undefined!
      return p
}

Caveat: only the first part of a computed will be tracked. But that is explainable and makes it somewhere easy as well.

cc @spion @urugator @danielearwicker

[mweststrate]

cc @d4rky-pl

[urugator]

I don't mind "enhancing" Promise with observable fields. What seems like a bigger problem is the inability to control conversion of the value itself.
On @observable you can get the control back into your hands with ref modifier (btw should shallow convert promise, but not the value?), however that's not the case with computed.
Also this is the only case where computed converts the returned value right? That may be unexpected...
So I think it's quite reasonable on @observable, but in case of @computed I would leave the conversion to user.

Btw shouldn't it be @computed.options({ initialValue: [] }) in your example? :)

EDIT:
Also change fromPromise/observable.promise signature to match other structures: observable.promise(initial, { deep:false }) (so it's configurable and has the same defaults)

And some more errors(?) in migration quide I have noticed:
@computed({ requiresReaction: true }) should be @computed.options({ requiresReaction: true })
@computed({ equals: compareFn }) should be @computed.options({ equals: compareFn })
observable.map(values, {}, { deep: false) should be observable.object(values, {}, { deep: false)

[d4rky-pl]

Also this is the only case where computed converts the returned value right? That may be unexpected...
So I think it's quite reasonable on @observable, but in case of @computed I would leave the conversion to user.

This is why I proposed the @computed.promise syntax on Twitter. I'd be vary of automatically converting the returned value every time as this is not something you'd expect @computed to do.

[mweststrate]

Agreed on the @computed.promise to make things explicit

Also @observable.ref should leave all promises as is (likewise, shallow on collections of promises).

Other concern, one might be tempted to return model classes from computed promises, but I think that might be tricky and could make things very non transparent? E.g.:

class TodoStore {
  @computed
  get todoData() {
      return fetch(`myapp/todos`)
  }

  @cmputed
  get todos() {
     if (this.todoData.state !== "fullfilled")
        return []
     return this.todoData.value.map(json => new Todo(json))
  }
}

class Todo {
  @observable title
  @observable done
}

[urugator]

There is no need or benefit from pushing this to own decorator or decorator options

@computed.promise({ initialValue : "", deep: false })
get promise() {
  return fetch(`myapp/todos`);
}
// following is imho much cleaner and easier to understand and doesn't clutter api
@computed
get promise() {
  const promise = fetch(`myapp/todos`)
  return observable.promise(promise, { initialValue: "" , deep: false });
}

[mweststrate]

@benjamingr cc

[urugator]

(likewise, shallow on collections of promises).

What I meant:

@observable.shallow = promise; 
// should work like 
@observable = observable.promise(promise, { deep: false }); // note the deep option 

[spion]

My idea is to not enhance promises, but instead, store a global WeakMap of PromiseStates.

function caseAsync(p:Promise, handlers: Handlers) {
  let promiseState = globalPromiseWeakMap.get(p);
  if (promiseState == null) { globalPromiseWeakMap.set(p, promiseState = new PromiseState(p)); }
  return promiseState.case(handlers);
}

Then you can use this on any promise or thenable:

const UserView = observer(({ user }) => (
   <div>
       <h1>{user.name}<h1/>
        {caseAsync(this.props.userProfileRace, {
            fulfilled: user => <img src={user.profileImageUrl} />
            rejected: err => if (err.duration <span>Unable to load profile</span>,
            pending: () => <Spinner />
        })}
       { user.tags.value.join(" - ") }
   </div>
))

[benjamingr]

We need async iterator support, not promise support.

Adding Symbol.asyncIterator to observables and consuming async iterators would be a huge win for MobX. For example:

 // polling which updates the profile computed property constantly
@computed async profile*() {
    while(true) yield await fetch(`/endpoint/`)
 }

Single async property:

 // polling which updates the profile computed property constantly
@computed async profile*() {
  yield fetch(`/endpoint/`)
 }

You can also yield from other async iterators with yield* and use combined properties.

---

As for regular promises, you can just set the value to their awaited result. One of the selling points of mobx is simplicity and the fact it's "just JavaScript". While I think async iterators are important enough to consider here - I think we should strive to a smaller API of just computed/action/observable/autorun (and that's it).

[spion]

@mweststrate the weakmap approach I described above would play together nicely (and type safe) with some really cool modifiers. Two that I think would be super useful and gamechangers (but need deep access to mobx internals) are:

@computed.options({debounce: N})

When the computed becomes dirty, wait N miliseconds of idle time (no new "dirty" signals) before recomputing and propagating dirtyness to own observers (return previous values in the meantime)

@computed.options({replaceWhenResolved: true})

If the computed returns a promise, don't replace the previous promise until the replacement is actually resolved somehow. Return the previous promise instead.

If the behaviour is sufficiently different this could be a new decorator, @computedAsync

[spion]

You can use replaceWhenResolved to do react style async loading that only replaces old data with a spiner once some time passes.

class LateLoader(fetcher, delay) {
  @computed get promise() { return fetcher(); }
  @computed.options({replaceWhenResolved: true}) 
  get lateValue() {
    // unlike delay, timeout will throw
    return Promise.race(timeout(delay), this.promise)
  }

  case(handlers) { 
    return mobx.caseAsync(this.lateValue, {
      fulfilled: res => handlers.fulfilled(res.value);
      // use original promise when there is an error. 
      // it will automatically fallback to pending in timeout case
      rejected: e => mobx.caseAsync(this.promise, handlers)
      pending: () => handlers.pending()
    })
  }
})

[benjamingr]

Why would we be going against the grain here by adding a relatively big feature - why not do it async iterators @spion ?

With the more standards approach this would just be:

class LateLoader(fetcher, delay) {
   @computed async function* lateValue() {
      while(true) { // remove to only fetch once rather than continously
        try { 
          yield await Promise.race([fetcher(), timeout(delay)]);
        } catch (e) {} // don't replace anything, or rethrow if timeout error
      }
    }
}