dokar3 / quickjs-kt

A QuickJS binding for idiomatic Kotlin, with Async/DSL/ES Modules support.
Apache License 2.0
41 stars 3 forks source link

[Feature request] Allow strictly typed arguments in FunctionBindings #82

Closed LilyStilson closed 3 months ago

LilyStilson commented 3 months ago

The problem

Currently, both FunctionBinding<R> and AsyncFunctionBinding<R> only allow generic return types

fun interface FunctionBinding<R> : Binding {
    fun invoke(args: Array<Any?>): R
}

fun interface AsyncFunctionBinding<R> : Binding {
    suspend fun invoke(args: Array<Any?>): R
}

The purpose

I propose either making the Binding interface public to be able to create your own implementation of binding with custom arguments type or changing the FunctionBindings to support custom argument types. I am making this an issue first because this would be a breaking change.

fun interface FunctionBinding<A, R> : Binding {
    fun invoke(args: A): R
}

fun interface AsyncFunctionBinding<A, R> : Binding {
    suspend fun invoke(args: A): R
}

Use case

As this QuickJs bindings library doesn't provide a fetch implementation, following the example in your readme, having something like this:

interface FetchArguments {
    val url: String
    val options: FetchOptions
}

interface FetchOptions {
    val method: String
    val headers: Map<String, String>
    val body: String
}

interface FetchResponse {
    val ok: Boolean
    val status: Int
    val statusText: String
    val headers: Map<String, String>
    val body: String
    val responseURL: String
    val responseText: String
}

/* ------------------------- */

quickJs {
    asyncFunction<FetchArguments, FetchResponse>("fetch") { args: FetchArguments ->
        // Custom fetch implementation
    }
}

would simplify the way we parse arguments, allowing for fewer type errors overall and providing closer to the original JavaScript experience.

The alternative

Currently, the only alternative option I see is using Reflection. However, the documentation makes it unclear, whether we can provide pure functions through reflection, or we need to wrap them with objects (which is also not ideal, since calling http.get is non-standard JavaScript and not every developer is willing to read the documentation to understand, why there is no fetch function.

LilyStilson commented 3 months ago

Adding to this, trying to return data class from function or asyncFunction yields an error. Could you please add an example of how to properly return Objects from declared functions? Should those objects be inherited from something?

com.dokar.quickjs.QuickJsException: TypeError: Cannot convert java type 'org.nightskystudio.ibuki.lib.FetchResponse' to a js value.
    at com.dokar.quickjs.QuickJs.executePendingJob(Native Method)
    at com.dokar.quickjs.QuickJs.access$executePendingJob(QuickJs.jni.kt:56)
    at com.dokar.quickjs.QuickJs$invokeAsyncFunction$job$1.invokeSuspend(QuickJs.jni.kt:318)

Code example (FetchArguments, FetchOptions and FetchResponse were all converted to data class):

fun fetch(args: FetchArguments): FetchResponse {
    return FetchResponse(
        ok = true,
        status = 200,
        statusText = "OK",
        headers = mapOf(),
        body = "",
        responseURL = "",
        responseText = ""
    )
}
/* ----------------------------- */
asyncFunction("fetch") {
    val url = it[0] as String
    val opts = it[1] as LinkedHashMap<*, *>
    val method = opts["method"] as String
    val headers: Map<String, String> = (opts["headers"] as LinkedHashMap<*, *>).map { e ->
        e.value.toString() to e.key.toString()
    }.toMap()
    val body = opts["body"] as String

    fetch(FetchArguments(url, FetchOptions(method, headers, body)))
}
dokar3 commented 3 months ago

Interesting proposal! The library currently has no serialization/deserialization support for custom classes, so even if making Binding open, you still have to serialize/deserialize custom classes manually.

In your fetch() case, due to the above limitations, you need to serialize your object to a JsObject to return a custom object from function bindings.

fun FetchResponse.toJsObject(): JsObject {
    val fields = mutableMapOf<String, Any>()
    // Read fields into the map
    return JsObject(fields)
}

asyncFunction("fetch") { args ->
    fetch(FetchArguments(url, FetchOptions(method, headers, body))).toJsObject()
}
LilyStilson commented 3 months ago

Thanks for your response, @dokar3

I found a workaround for my use case already, I was mostly looking at the problem from the wrong angle. So I decided to implement fetch using both JavaScript and Kotlin, the way I already did it in one of my projects. The polyfill:

function fetch(url, options) { 
    options = options || {
        method: "GET"
    }
    return new Promise(async (resolve, reject) => {
        let request = await doFetch(JSON.stringify({"url": url, "options": options}))

        const response = () => ({
            ok: ((request.status / 100) | 0) == 2, // 200-299
            statusText: request.statusText,
            status: request.status,
            url: request.responseURL,
            text: () => Promise.resolve(request.responseText),
            json: () => Promise.resolve(request.responseText).then(JSON.parse),
            blob: () => Promise.resolve(new Blob([request.response])),
            clone: response,
            headers: request.headers,
        })

        if (request.ok) resolve(response())
        else reject(response())
    })
}

The Kotlin's doFetch function:

suspend fun doFetch(args: FetchArguments): FetchResponse { }
/* ----------------------------------- */
asyncFunction<Any>("doFetch") {
    val json = it[0] as String
    val opts = Json.decodeFromString<FetchArguments>(json)
    val response = doFetch(opts)

    response.toMap()
}

Passing parameters encoded as JSON string in JS, then decoding it back in Kotlin was the way to go, as is making a proper fetch polyfill, as I am not sure whether it would be possible to achieve await response.blob()/await response.json() using pure Kotlin x)

Also, I found out that Kotlin's Map transforms perfectly into JavaScript's Object. In any case, with availability of JSON in JS this should not pose a problem as objects can be passed encoded as strings :)

dokar3 commented 3 months ago

Glad to see that!

I'm also considering adding a serializer API for this, so the solution for custom classes could be cleaner like you proposed, and we wouldn't have to use JSON as an intermediary. But I'm not sure about this as it would probably require a lot of changes, so I'll check if this is possible first when I have the time.

Anyway, thanks for the proposal!

dokar3 commented 3 months ago

Sorry for the delay, the new version with type converters and generics parameter support is out! Please feel free to try.

https://github.com/dokar3/quickjs-kt/releases/tag/v1.0.0-alpha11

https://github.com/dokar3/quickjs-kt?tab=readme-ov-file#custom-types