Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toTaskX functions do not consider empty observables #61

Open
mlegenhausen opened this issue Apr 30, 2021 · 6 comments
Open

toTaskX functions do not consider empty observables #61

mlegenhausen opened this issue Apr 30, 2021 · 6 comments
Labels

Comments

@mlegenhausen
Copy link
Collaborator

mlegenhausen commented Apr 30, 2021

🐛 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
Copy link
Owner

gcanti commented Apr 30, 2021

@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
Copy link
Collaborator Author

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

@gcanti
Copy link
Owner

gcanti commented May 3, 2021

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
Copy link
Collaborator Author

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

@gcanti
Copy link
Owner

gcanti commented May 3, 2021

Yes, like Task's never.

@mlegenhausen
Copy link
Collaborator Author

PR updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants