growthbook/growthbook-kotlin

Feature Request: Allow users to provide their own NetworkDispatcher and extract Ktor dependency into a separate artifact

ankushg opened this issue · 7 comments

Ktor can be a pretty heavy dependency in apps that already have networking stacks

We're using this approach to tie networking within KMP to our existing networking stack

It would be great to be able to provide my own NetworkDispatcher implementation and avoid Ktor

@way2jatin
What do you think about removing ktor dependency and consuming library users implementions of NetworkDispatcher?

@Bohdan-Kim By removing the Ktor dependency and adopting this approach, it would become flexible and agnostic for sure. Since the approach mentioned above is already live in production, so we can safely assume that it is reliable in nature.

So, according to the request, 'ktor-client-core' dependency should be removed. This also leads to removing @DelicateCoroutinesApi annotation, because 'ktor-client-core' transitively depends on 'kotlinx-coroutines-core-jvm'.
image

@DelicateCoroutinesApi annotation is used through the entire our codebase.
@ankushg @way2jatin Do you prefer to get rid of this annotation or to leave it and have 'kotlinx-coroutines-core' dependency?

image

image

@Bohdan-Kim
Can you please change GBSDKBuilderAppconstructor api and pass networkDispatcher as a param. And also make CoreNetworkClient class public. It will allow us to exclude ktor library dependency from our app and also Proguard will remove unused CoreNetworkClient class.

@AlParhimchik
Sure
Should it look like this?

In this pull request networkDispatcher was moved to GBSDKBuilder params.
If app already has networking stack NetworkDispatcher implementation can be passed through GBSDKBuilder parameters. ktor dependency from GrowthBook Kotlin SDK will be optimized and removed by ProGuard tool in such case.

Current implementation still does not allow to shrink ktor normally, as it's direct classes or dependant are used as default params or returning params.
Please consider below PR:
#82