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

did you consider to integrate kvision with ktor client? #523

Open
CodeServant opened this issue Mar 20, 2024 · 5 comments
Open

did you consider to integrate kvision with ktor client? #523

CodeServant opened this issue Mar 20, 2024 · 5 comments

Comments

@CodeServant
Copy link

CodeServant commented Mar 20, 2024

I think its good idea. It provides lot of tools for clients like cookies management, authentication, logging, web sockets, caching.

Do you see a reason why this is not good idea?

@rjaros
Copy link
Owner

rjaros commented Mar 20, 2024

@CodeServant
Copy link
Author

I did some tests and this seems not an issue now.

the size of my bundle.js is:
2 032 KB my own project without modifications (with kvsion stuff, kotlinx-datetime, kotlin-serialization)
2 338 KB after creating empty ktor Client
2 362 KB after adding auth and configuration of basic auth
2 444 KB after defining cookies and cache and Charsets
2 479 KB after installing logging

my methodology

requirements:

windows 10
kotlin = 2.0.0-Beta4
kvision = 7.4.3
ktor client = 2.3.9
gradle wrapper = 8.6

gradle command

.\gradlew clean jsBrowserDistribution

the added script looks like this

private suspend fun createFuss() {
    val client = HttpClient(Js) {
        install(Auth) {
            basic {
                credentials {
                    BasicAuthCredentials(username = "jetbrains", password = "foobar")
                }
                realm = "Access to the '/' path"
            }
        }
        install(HttpCookies) {
            storage = ConstantCookiesStorage(Cookie(name = "user_name", value = "jetbrains", domain = "0.0.0.0"))
        }
        install(HttpCache)
        Charsets {
            register(Charsets.UTF_8)
            register(Charsets.ISO_8859_1, quality = 0.1f)
        }
        install(Logging)
    }
}

gradle dependencies

...
val ktorVersion = "2.3.9"
implementation("io.ktor:ktor-client-js:$ktorVersion")
implementation("io.ktor:ktor-client-logging-js:$ktorVersion")
implementation("io.ktor:ktor-client-auth:$ktorVersion")
...

and here are imports I've added

import io.ktor.client.*
import io.ktor.client.request.*
import io.ktor.client.statement.*
import io.ktor.client.plugins.*
import io.ktor.client.plugins.logging.*
import io.ktor.client.engine.js.*
import io.ktor.client.plugins.Charsets
import io.ktor.client.plugins.auth.*
import io.ktor.client.plugins.auth.providers.*
import io.ktor.client.plugins.cache.*
import io.ktor.client.plugins.cookies.*
import io.ktor.http.*
import io.ktor.utils.io.charsets.*
...

other test

did the test with this too, and my bundle.js is 422 KB so it's pretty decent.

What do you think? Are the tests valid for KVision?

@rjaros
Copy link
Owner

rjaros commented Mar 22, 2024

In general nothing stops you from using Ktor client. KVision REST client is totally optional. If you prefer Ktor and don't mind a bit larger bundle size, you can use it without any problems. So the question is - what kind of integration do you think of?

@CodeServant
Copy link
Author

I thought it would enable more feature out of the box and limit those that are actually duplicated like rest api calls. Since it is Jetbrains, this will get official support and in the end it should be easier to maintain.

@rjaros
Copy link
Owner

rjaros commented Mar 22, 2024

But KVision doesn't use rest api calls internally. So there is nothing to replace. Of course we could drop kvision-rest module, but it would only break compatibility with existing apps.

The fullstack interfaces are based on fetch api directly and it would be probably to complicated to use ktor client instead.

So in the end I think currently there is nothing to do with this issue. Everyone is free to use Ktor client with KVision apps.

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

2 participants