Rxjs: toPromise should reject if no value is emitted.

[benlesh]

Current Behavior

toPromise will resolve with undefined if the source observable simply completes without emitting a value.

EMPTY.toPromise().then(x => console.log(x)); // logs "undefined"

The problem

Promises are a guarantee to a future value. With toPromise() we should be saying "this is the last value we've gotten prior to completion". But what we're actually saying is "this might be the last value we've gotten prior to completion". It's semantically unsound, IMO.

If the developer wants a guaranteed value from a source, and that value might be undefined, there's currently no way for that developer to discern whether or not the undefined that toPromise resolved with was the undefined from the source, or just happenstance from completion without a value.

const promiseTheresAValue1 = of(1, 2, 3, undefined).toPromise();

const promiseTheresAValue2 = EMPTY.toPromise(); // Oops, there wasn't actually a value

Proposal

Have toPromise reject with an EmptyError if no value is emitted prior to completion of the source.

EMPTY.toPromise()
  .then(() => console.log('not hit'))
  .catch(err => console.log(err instanceof EmptyError)); // logs "true"

Optionally we could introduce a last() or lastValue() method to Observable and deprecate toPromise to reduce confusion while migrating to this change, while also being able ot introduce this better semantic sooner.

Related: https://github.com/ReactiveX/rxjs/issues/4993

[benlesh]

Related #5072

[cartant]

we could introduce a last() or lastValue() method to Observable

I'd be wary of introducing anything else to Observable whilst there any chance the TC39 proposal might be resurrected. IMO, toPromise should be deprecated for eventual removal from the Observable prototype and a static function should be exported instead.

[benlesh]

@cartant, that's an interesting proposal. As far as the observable proposal goes, it might still be a moonshot, so I think RxJS, as a library, should work towards making sure our developer ergonomics make sense.

await lastValueFrom(source$);
await source$.lastValue();
await source$.toPromise();

Honestly, they're all a little unpalatable. lol

[cartant]

One reason for my wanting it removed from the protoype, is that I cannot recall using it outside of some tests that I wrote years ago.

It seems a little weird to have switched to static, pipeable, tree-shakeable operators, but still have (what ought to be, IMO) a rarely-used method on the prototype.

[kwonoj]

lastAsync(source)? Just borrowed .net binding semantic (last/firstorDefaultAsync). Doesn't look great too though. Or as we discussed, start from __unsafeSomething and collect opinion maybe.

I tend to agree making this as static method, mostly I would prefer observable itself to be thin enough.

[benlesh]

Decided that we want to go with static methods and deprecate toPromise. Since we would be adding new features and not introducing breaking changes, we can do this in 7.1,7.2 timeframe.