Skip to content
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

Kotlin Coroutines support? #2

Closed
linux-china opened this issue Oct 17, 2020 · 7 comments
Closed

Kotlin Coroutines support? #2

linux-china opened this issue Oct 17, 2020 · 7 comments

Comments

@linux-china
Copy link

Upgrade worker to Kotlin 1.4.0 and all is good. But failed to run worker with kotlinx:kotlinx-coroutines-core-js. Cloudflare runtime compatible problem or my code problem?

build.gradle.kts:

    implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core-js:1.3.9")

Example code:

import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.promise
import org.w3c.fetch.Response
import org.w3c.fetch.ResponseInit
import org.w3c.workers.FetchEvent

fun main() {
    addEventListener("fetch") { event: FetchEvent ->
        event.respondWith(handleEvent(event))
    }
}

fun handleEvent(event: FetchEvent) = GlobalScope.promise {
    val headers: dynamic = object {}
    headers["content-type"] = "text/plain"
    Response(
        "Hello Kotlin Worker",
        ResponseInit(headers = headers)
    )
}
@koeninger
Copy link
Contributor

koeninger commented Oct 19, 2020

bash-3.2$ wrangler tail
�  Setting up log streaming from Worker script "projectname". Using ports 8080 and 8081.
Now prepared to stream logs.
{"outcome":"exception","scriptName":null,"exceptions":[{"name":"Error","message":"addEventListener(): useCapture must be false."

https://github.com/Kotlin/kotlinx.coroutines/blob/1b34e1c7dd6207d7683c307bae0b934a3dc18d09/kotlinx-coroutines-core/js/src/JSDispatcher.kt#L103

Editing the generated index.js file to change this line

this.window_0.addEventListener("message",(e=this,function(t){return t.source==e.window_0&&t.data==e.messageName_0&&(t.stopPropagation(),e.process()),l}),!0)

to pass false instead of !0 lets the worker run successfully, so that does seem to be the issue.

That being said, what is your use case for kotlin coroutines on Workers? It looks like a 5x size penalty in the generated javascript.

@linux-china
Copy link
Author

linux-china commented Oct 19, 2020

@koeninger it works now. you did help me :) You know lots of Kotlin frameworks use Kotlinx.coroutines, such as Ktor etc, and suspend method is more friendly than JS Promise. It's true that the final bundle size is some big with Kotlin & coroutines runtime, but still be acceptable. I adjust compiler to use js(IR) and final bundle size just half now.

js(IR) {
        browser {
            binaries.executable()
            testTask {
                useMocha()
            }
        }
    }

@LouisCAD
Copy link

LouisCAD commented Oct 21, 2020

Editing the generated index.js file to change this line

this.window_0.addEventListener("message",(e=this,function(t){return t.source==e.window_0&&t.data==e.messageName_0&&(t.stopPropagation(),e.process()),l}),!0)

to pass false instead of !0 lets the worker run successfully, so that does seem to be the issue.

Does it mean one would need to edit the generated code manually after each compilation to have it run on cloudflare worker?

@linux-china
Copy link
Author

linux-china commented Oct 21, 2020

@LouisCAD you can add some code in build.gradle.kts for replacement. My code:

tasks.register("buildWorker") {
    dependsOn("browserProductionWebpack")
    doLast {
        file("$projectDir/dist/").mkdirs()
        /* Kotlin js output assumes window exists, which it won't on Workers. Hack around it */
        val projectJsCode = file("$projectDir/build/distributions/${rootProject.name}.js")
            .readText()
            // bug fix https://github.com/cloudflare/kotlin-worker-hello-world/issues/2
            .replace("e.process()),l}),!0", "e.process()),l}),false") //legacy
            .replace("}),!0)}", "}),false)}") // IR
        file("$projectDir/dist/index.js").writeText("const window=this; $projectJsCode")
    }
}

@LouisCAD
Copy link

@linux-china Shouldn't you just use js { nodejs() } instead of js() or js { browser() } plus this hackery? Or doesn't it work?

@linux-china
Copy link
Author

you should do the replacement too with following configuration.

kotlin {
    js(IR) {
        nodejs {
            testTask {
                useMocha()
            }
        }
        binaries.executable()
    }
}

@LouisCAD
Copy link

The replacement is still needed when building for the nodejs() target? If so, it's a bug that need to be reported?

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

No branches or pull requests

3 participants