gcanti / fp-ts-rxjs

fp-ts bindings for RxJS
https://gcanti.github.io/fp-ts-rxjs/
MIT License
187 stars 29 forks source link

toTaskX functions do not consider empty observables #61

Open mlegenhausen opened 3 years ago

mlegenhausen commented 3 years ago

šŸ› Bug report

Current Behavior

import * as _ from 'fp-ts-rxjs/Observable'

const t = await _.toTask(_.zero<number>())() // Returned type number, Expected type number |Ā undefined
assert.deepStrictEqual(t, undefined)

Expected behavior

Instead of Task<A> we need to return Task<Option<A>>

export const toTaskOption = <A>(o: Observable<A>): Task<O.Option<A>> => () => o.toPromise().then(O.fromNullable)

Reproducible example

See current behavior

Suggested solution(s)

We need to change all toTaskX function to toTaskOptionX which get really ugly and before providing a PR I would like to discuss how the function for ObservableThese and ObservableEither should look like. I would actually consider to deprecate or even to remove them cause they are buggy and add no additional value over the toTaskOption function from Observable.

Additional context

Currently it is only a runtime bug. With rxjs@7 we will get also a type error cause the signature of toPromise has changed to cover the undefined case.

Your environment

Which versions of fp-ts-rxjs are affected by this issue? Did this work in previous versions of fp-ts-rxjs?

All versions since 0.6.5

Software Version(s)
fp-ts 2.10.4
fp-ts-rxjs 0.6.14
rxjs 6.6.7
TypeScript 4.2.4
gcanti commented 3 years ago

@mlegenhausen thanks for reporting, toPromise's signature is inaccurate:

toPromise<T>(this: Observable<T>): Promise<T>

Is it possible to detect if an observable completes without emitting a value?

mlegenhausen commented 3 years ago

Yes, this is demonstrated in a new rxjs@7 function lastValueFrom.

gcanti commented 3 years ago

Ok, so I guess we can do the same and define an internal lastValueFrom helper (except we can't reject) and use that instead of toPromise

mlegenhausen commented 3 years ago

So we would not resolve when a complete occurred and no value was emitted?

gcanti commented 3 years ago

Yes, like Task's never.

mlegenhausen commented 3 years ago

PR updated.