haxetink / tink_await

Haxe async/await
MIT License
58 stars 15 forks source link

@:await JS Promises #46

Open piboistudios opened 2 years ago

piboistudios commented 2 years ago

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>).

kevinresol commented 2 years ago

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

piboistudios commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

Pign commented 2 years ago

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

benmerckx commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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