-
Notifications
You must be signed in to change notification settings - Fork 529
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
Swap Thenable
for Promise
in Async.fromPromise
#2237
Conversation
def fromThenable[A](iot: IO[Thenable[A]]): IO[A] = | ||
asyncForIO.fromThenable(iot) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just make fromPromise
take a Thenable
instead? That would mirror the JavaScript convention of treating "everything that has a then
" as being secretly a Promise
. It's even fully source and binary compatible in IO
, and fully binary compatible in Async
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, makes sense!
It's even fully source and binary compatible in IO, and fully binary compatible in Async.
I feel I am never going to understand binary compatibility 🤔 I didn't think this would be binary compatible because when relaxing the queue signatures to the source/sink supertypes in fs2 in typelevel/fs2#2466, it wasn't binary compatible. Why is it different here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, because type erasure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The binary type is erased to IO
in the former case, and erased to Object
in the latter case, so anything goes. :-P
Async.fromThenable
Async.fromThenable
~ Swap Thenable
for Promise
in Async.fromPromise
Async.fromThenable
~ Swap Thenable
for Promise
in Async.fromPromise
Thenable
for Promise
in Async.fromPromise
Fixes #2236.