-
Notifications
You must be signed in to change notification settings - Fork 15
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
@:await JS Promises #46
base: master
Are you sure you want to change the base?
Conversation
I prefer having the user manually covert it via |
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 |
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. |
@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 :) |
I main concern is the scope of this library. I am just not sure if we should include JS promises into our scope. |
Well I'd say that unification of promises to "the tink way" in the scope of a "tink framework" seems fair. |
I can see how the earlier edits raised some concern. But the code change now is just applying |
Oh completely missed that part. well yeah then I agree this is good. |
@piboistudios could you reword the doc update a little to reflect that we simply use Promise.lift as opposed to extracting things? |
This will just extract the
T
fromjs.lib.Promise<T>
(if it is ajs.lib.Promise
) and wrap the expression in a check-type statement:($expr : tink.core.Promise<T>)
.