-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Move Promise-related functions to the webMain source set #4563
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
base: develop
Are you sure you want to change the base?
Conversation
Fixes #4544 Fixes #4526 On Wasm/JS, before this commit, we had: ```kotlin fun <T> CoroutineScope.promise( context: CoroutineContext = EmptyCoroutineContext, start: CoroutineStart = CoroutineStart.DEFAULT, block: suspend CoroutineScope.() -> T ): Promise<JsAny?> fun <T> Deferred<T>.asPromise(): Promise<JsAny?> fun <T> Promise<JsAny?>.asDeferred(): Deferred<T> suspend fun <T> Promise<JsAny?>.await(): T ``` These signatures are either losing type information (`promise`, `asPromise`) or are type-unsafe (`asDeferred`, `await`). The way an arbitrary `T` is converted into a `JsAny?` is by calling ```kotlin public actual fun <T : Any> T.toJsReference(): JsReference<T> = implementedAsIntrinsic ``` Note the `Any` type bound, it's going to be important. The opposite direction is taken by just an unchecked type cast: ```kotlin value as T ``` The *correct* types for the current implementations of the functions we have are: ```kotlin fun <T> CoroutineScope.promise( context: CoroutineContext = EmptyCoroutineContext, start: CoroutineStart = CoroutineStart.DEFAULT, block: suspend CoroutineScope.() -> T? ): Promise<JsReference<T & Any>?> fun <T> Deferred<T?>.asPromise(): Promise<JsReference<T & Any>?> fun <T> Promise<T: JsAny?>.asDeferred(): Deferred<T> suspend fun <T: JsAny?> Promise<T>.await(): T ``` In principle, since Kotlin/Wasm/JS is experimental, we can break the existing usages if it is necessary. 1. Of course, we don't have `& Any`, which is an issue for this use case. If `block` returns a `null`, then the whole `Promise` resolves to `null`, and if not, it resolves to the proper `JsReference<T>` where `T` is non-`null`. 2. There is a behavior in Kotlin/Wasm/JS that makes the correct types for `await()` and `asDeferred()` incompatible with the current ones: `JsReference<T> as T` works, but does not happen automatically, so where you had `await<String>()` before, now, you will only be able to do `await<JsReference<String>>().get()`. The first issue can be worked around by having two overloads: ```kotlin fun <T: Any> CoroutineScope.promise( context: CoroutineContext = EmptyCoroutineContext, start: CoroutineStart = CoroutineStart.DEFAULT, block: suspend CoroutineScope.() -> T? ): Promise<JsReference<T>?> fun <T: Any> Deferred<T?>.asPromise(): Promise<JsReference<T>?> // These last two are questionable: fun <T: Any> CoroutineScope.promise( context: CoroutineContext = EmptyCoroutineContext, start: CoroutineStart = CoroutineStart.DEFAULT, block: suspend CoroutineScope.() -> T ): Promise<JsReference<T>> fun <T: Any> Deferred<T>.asPromise(): Promise<JsReference<T>> ``` This prevents writing code polymorphic in `T: Any?` that uses these functions, but this limitation may be okay in practice. The second issue may also be solved by introducing extra overloads, but it doesn't look pretty: ```kotlin suspend fun <T: JsAny?> Promise<T>.await(): T // This one is questionable: suspend fun <T: Any> Promise<JsReference<T>>.await(): T ``` These two signatures are also interchangeable with those that Kotlin/JS had before this commit: ```kotlin fun <T> Promise<T: JsAny?>.asDeferred(): Deferred<T> suspend fun <T: JsAny?> Promise<T>.await(): T ``` Since both Kotlin/JS and these functions are stable, we need to preserve their behaviors, which means These, however, are a challenge: ```kotlin fun <T: JsAny?> CoroutineScope.promise( context: CoroutineContext = EmptyCoroutineContext, start: CoroutineStart = CoroutineStart.DEFAULT, block: suspend CoroutineScope.() -> T ): Promise<T> fun <T: JsAny?> Deferred<T>.asPromise(): Promise<T> ``` On Kotlin/JS, `promise` and `asPromise` are polymorphic in `T` over an arbitrary `T`, but Wasm/JS extends the universe of types with Kotlin-specific ones. The ideal solution would be to provide both on Wasm/JS: have `fun promise` in `webMain` that would only work with `JsReference`, and in `wasmJsMain`, also have a `fun promise` that would accept arbitrary types; the same goes for `Deferred.asPromise`. Then, overload resolution could kick in and choose the most specific overload. Unfortunately, Kotlin does not allow this: ```kotlin internal interface A internal fun <T: A> b(block: () -> T) { } internal fun <T> b(block: () -> T) { } internal fun c() { b { 3 } // does not compile } ``` We could mark the general implementation in `webMain` with `LowPriorityInOverloadResolution`, but then, that implementation does not get chosen at all in Kotlin/Wasm/JS code: ```kotlin internal interface A @Suppress("INVISIBLE_REFERENCE") @kotlin.internal.LowPriorityInOverloadResolution internal fun <T: A> b(block: () -> T): String { return "" } internal fun <T> b(block: () -> T): T { return block() } internal fun c() { val v: Int = b { 3 } // does compile now val s: String = b { object: A {} } // but this doesn't } ``` The same goes for `Deferred.asPromise`: ```kotlin fun <T: Any> Deferred<T>.asPromise(): Promise<JsReference<T>> { TODO() } fun <T: JsAny?> Deferred<T>.asPromise(): Promise<T> { TODO() } val v = async { "OK".toJsReference() } v.asPromise() // fails to compile ``` Therefore, any `promise` or `asPromise` implementations in `webMain` necessarily *preclude* the implementations that works with arbitrary Kotlin types in Kotlin/Wasm/JS. For compatibility with JS, `promise` and `Deferred.asPromise` common to `web` are limited to the JS types, even on Wasm/JS, which can't be fixed by adding extra overloads to Wasm/JS. On the Wasm/JS side, this is a breaking change. It can usually be mitigated by sticking a `JsReference::get` or `Any::toJsReference` to relevant places, but not always. - For example, if third-party code exposes a `Deferred<String>`, the users will be out of luck trying to obtain a `Promise<JsString>` out of that. - More annoyingly, `GlobalScope.promise { }` launched for the side effects will no longer compile, as `Unit` is not `JsAny`. Every such lambda now has to end with some `JsAny` value, for example, `null`. This change was introduced throughout our codebase as well, as we are also affected by this breakage.
| */ | ||
| @ExperimentalWasmJsInterop | ||
| public fun <T> CoroutineScope.promise( | ||
| @OptIn(ExperimentalWasmJsInterop::class) |
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.
It's unclear to me if we want to propagate the opt-in here instead. Are these functions also part of the experimental Wasm/JS interop, or are they fine, even when Promise is not? The description of the annotation https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.js/-experimental-wasm-js-interop/ does not make it clear to me.
The approach taken with Promise (which this PR could also replicate when needed) is to have @ExperimentalWasmJsInterop expect in web, @ExperimentalWasmJsInterop actual in wasmJs, and actual in js (without the annotation).
Fixes #4544
Fixes #4526
On Wasm/JS, before this commit, we had:
These signatures are either losing type information (
promise,asPromise) or are type-unsafe (asDeferred,await).The way an arbitrary
Tis converted into aJsAny?is by callingNote the
Anytype bound, it's going to be important.The opposite direction is taken by just an unchecked type cast:
The correct types for the current implementations of the functions we have are:
In principle, since Kotlin/Wasm/JS is experimental, we can break the existing usages if it is necessary.
& Any, which is an issue for this use case. Ifblockreturns anull, then the wholePromiseresolves tonull, and if not, it resolves to the properJsReference<T>whereTis non-null.await()andasDeferred()incompatible with the current ones:JsReference<T> as Tworks, but does not happen automatically, so where you hadawait<String>()before, now, you will only be able to doawait<JsReference<String>>().get().The first issue can be worked around by having two overloads:
This prevents writing code polymorphic in
T: Any?that uses these functions, but this limitation may be okay in practice.The second issue may also be solved by introducing extra overloads, but it doesn't look pretty:
These two signatures are also interchangeable with those that Kotlin/JS had before this commit:
Since both Kotlin/JS and these functions are stable, we need to preserve their behaviors, which means
These, however, are a challenge:
On Kotlin/JS,
promiseandasPromiseare polymorphic inTover an arbitraryT, but Wasm/JS extends the universe of types with Kotlin-specific ones.The ideal solution would be to provide both on Wasm/JS: have
fun promiseinwebMainthat would only work withJsReference, and inwasmJsMain, also have afun promisethat would accept arbitrary types; the same goes forDeferred.asPromise. Then, overload resolution could kick in and choose the most specific overload.Unfortunately, Kotlin does not allow this:
We could mark the general implementation in
webMainwithLowPriorityInOverloadResolution, but then,that implementation does not get chosen at all in Kotlin/Wasm/JS code:
The same goes for
Deferred.asPromise:Therefore, any
promiseorasPromiseimplementations inwebMainnecessarily preclude the implementations that works with arbitrary Kotlin types in Kotlin/Wasm/JS.For compatibility with JS,
promiseandDeferred.asPromisecommon towebare limited to the JS types, even on Wasm/JS, which can't be fixed by adding extra overloads to Wasm/JS.On the Wasm/JS side, this is a breaking change.
It can usually be mitigated by sticking a
JsReference::getorAny::toJsReferenceto relevant places, but not always.Deferred<String>, the users will be out of luck trying to obtain aPromise<JsString>out of that.GlobalScope.promise { }launched for the side effects will no longer compile, asUnitis notJsAny. Every such lambda now has to end with someJsAnyvalue, for example,null. This change was introduced throughout our codebase as well, as we are also affected by this breakage.