Skip to content
This repository has been archived by the owner. It is now read-only.

Add Observable#then to turn it into a Promise directly. #470

Closed
mattpodwysocki opened this issue Dec 30, 2014 · 3 comments
Closed

Add Observable#then to turn it into a Promise directly. #470

mattpodwysocki opened this issue Dec 30, 2014 · 3 comments

Comments

@mattpodwysocki
Copy link
Member

@mattpodwysocki mattpodwysocki commented Dec 30, 2014

Instead of relying on Rx.Observable#toPromise, implement Rx.Observable#then to allow for being a natural part of Promises.

mattpodwysocki added a commit that referenced this issue Dec 30, 2014
@mattpodwysocki
Copy link
Member Author

@mattpodwysocki mattpodwysocki commented Dec 30, 2014

Implementation could be as follows:

  /*
  * Converts an existing observable sequence to an ES6 Compatible Promise and calls the fulfillment or rejection methods.
  * @param {Function} [onFulfilled] the handler to call on fulfillment of the Promise.
  * @param {Function} [onRejected] the handler to call on the rejection of the Promise.
  * @returns {Promise} An ES6 compatible promise with the last value from the observable sequence and executing the handlers.
  */
  Rx.Observable.prototype.then = function (onFulfilled, onRejected) {
    var promiseCtor = Rx.config.Promise;
    if (!promiseCtor) { throw new TypeError('Promise type not provided in Rx.config.Promise'); }
    var source = this;
    var p = new promiseCtor(function (resolve, reject) {
      // No cancellation can be done
      var value, hasValue = false;
      source.subscribe(function (v) {
        value = v;
        hasValue = true;
      }, reject, function () {
        hasValue && resolve(value);
      });
    });

    return p.then(onFulfilled, onRejected);
  };
@spion
Copy link

@spion spion commented Dec 31, 2014

I'd recommend against this as it makes it impossible to deliver a promise for an observable (every observable will be assimilated by promises). A slightly contrived example:

function getMessageStream(id:string):Promise<Observable<Message>> {
  return db.connect(opts)
    .then(c => getUserQuery(c, id))
    .then(user => createMessageStream(user.channel));
}

Adding then to Observable will make the result of this fn be Promise<Message> which will be the last message of the user.

No problem, we say, we can wrap the stream with a promise explicitly:

function getMessageStream(id:string):Promise<Observable<Message>> {
  return db.connect(opts)
    .then(c => getUserQuery(c, id))
    .then(user => Promise.resolve(createMessageStream(user.channel)));
}

Nope, that wont work either. Promise.resolve also assimilates thenables.

What can we do? The only remaining option is a hack:

function getMessageStream(id:string):Promise<{stream:Observable<Message>}> {
  return db.connect(opts)
    .then(c => getUserQuery(c, id))
    .then(user => ({stream: createMessageStream(user.channel)})));
}

We have to pack the messagestream inside an array or an object that doesn't have a then method.

I'd say that there is nothing wrong with toPromise - its much less surprising, much more explicit. Implicit conversions always lead to a mess.

@mattpodwysocki
Copy link
Member Author

@mattpodwysocki mattpodwysocki commented Dec 31, 2014

@spion fair point, will remove and close the issue.

mattpodwysocki added a commit that referenced this issue Dec 31, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants