Skip to content

Conversation

@piboistudios
Copy link

This will just extract the T from js.lib.Promise<T> (if it is a js.lib.Promise) and wrap the expression in a check-type statement:
($expr : tink.core.Promise<T>).

@piboistudios piboistudios changed the title Convert @:await JS Promises @:await JS Promises Dec 24, 2021
@kevinresol
Copy link
Member

I prefer having the user manually covert it via Promised.ofJsPromise

@piboistudios
Copy link
Author

piboistudios commented Dec 24, 2021

I prefer having the user manually covert it via Promised.ofJsPromise

You should see my edit of the README ;)

Plus this wouldn't be a breaking change (as, this would have caused a compile-time error previously)

It's entirely optional

@kevinresol
Copy link
Member

I don't use tink_await much nowadays and I am not sure if it is good to lift everything to a Promise implicitly. I will let others to comment.

@Pign
Copy link

Pign commented Jun 17, 2022

@kevinresol I am not sure what your concern is? Do you want to avoid wrapping everything? I believe it is up to the dev to decide : they can either use @await and in which case it gets wrapped or if they do not want to wrap it they can just use .then on the object :)

@kevinresol
Copy link
Member

I main concern is the scope of this library. I am just not sure if we should include JS promises into our scope.

@Pign
Copy link

Pign commented Jun 17, 2022

Well I'd say that unification of promises to "the tink way" in the scope of a "tink framework" seems fair.

@benmerckx
Copy link
Member

benmerckx commented Jun 17, 2022

I main concern is the scope of this library. I am just not sure if we should include JS promises into our scope.

I can see how the earlier edits raised some concern. But the code change now is just applying tink.core.Promise.lift to the value that needs awaiting. Which also allows any abstract X to Promise to be awaited and that is pretty nice.

@kevinresol
Copy link
Member

the code change now is just applying tink.core.Promise.lift

Oh completely missed that part. well yeah then I agree this is good.

@benmerckx
Copy link
Member

@piboistudios could you reword the doc update a little to reflect that we simply use Promise.lift as opposed to extracting things?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants