Avoid retaining outdated LocalKoinApplication/LocalKoinScope

[jjkester]

The CompositionLocals that Koin uses only work when the (GlobalContext) Koin application is not changed at runtime, see #1557

Option 1: The KoinApplication composable

The Koin application is looked up in the Context tree and the Koin CompositionLocals are provided with/from this value. This (still) requires this function to be called around the first time Koin is used from Compose. A practical place for this is the setContent function. This means that (boilerplate) code needs to be added to every Activity or Fragment using Compose, and every test case where a Composable is tested in isolation.

Benefits:

• Works in Kotlin Compose Multiplatform
• Follows principles of already existing APIs

Drawbacks:

• Use is required to keep Compose and Context based APIs in sync, easy to forget to add

Option 2: Throwing UnknownKoinContext in the default value factory

By throwing an exception as default factory for the CompositionLocals we signal that there is no explicit value set. This will trigger the default lookup behavior that used to be the result of the factory function, as long as the appropriate functions getKoin() and getKoinScope() are used. By using the internal Compose API we are able to catch the exception to run the lookup code. We remember the result of the try/catch block to ensure we only incur the overhead of the exception once per getKoin() call per composition.

The default value of a CompositionLocal is singleton for the JVM process; when remembering a value as done in this approach the specific value is tied to the composition where it is used.

Benefits:

• Developers do not have to add the KoinApplication composable as compared to option 1

Drawbacks:

• Exceptions means a performance hit
• Uses an internal Compose API that we are not supposed to call

Other options

• Always retrieve the Koin application from the Android Context on Android
  • This means a specific Android implementation while the MPP code stays the same
• Reuse the initially referenced objects in Koin
  • This means keeping the same global Koin and Scope objects, even when a different Koin application is started using the test rule
  • Unsure about viability
• Ask Compose devs nicely to create a CompositionLocal variant that evaluates the default value expression every time instead of using lazy (and wait a long time for them to implement it)

Open tasks

• [ ] Investigate alternatives
• [ ] Investigate integrations with test frameworks for testing standalone composables
• [ ] Analyze performance of exception strategy
• [ ] Add test cases
  • [ ] Looking up the right Koin application from the Android context
  • [ ] Resolving #1557 for Robolectric and Android instrumented tests

Open questions to maintainer(s)

• Where to place Robolectric and instrumented tests?
• What is the preferred solution?

[arnaudgiuliani]

Hey @jjkester

I've ported your work on 61a88bbf79d593c9ed777f5b1acb07caa5e6db2e

Lets see how it goes with KoinContext and KoinAndroidContext

[jjkester]

@arnaudgiuliani Thanks for the heads-up, do to a busy time at work and the summer holidays I completely forgot about this.

Your commit looks good at a first glance, I'll definitely replace the workaround in our project with this approach once released.

I'd still would like to write a regression test for this, earlier I could not find a place in the codebase where to put it. Do you have any thoughts on this?

I'd like to make the tests as "real" as possible by using the Koin test utilities on a dummy project, of course ensuring that the tests fail without applying KoinScope/KoinAndroidScope.

[arnaudgiuliani]

@jjkester there is an issue on that: https://github.com/InsertKoinIO/koin/issues/1668

I believe I could make access to scope like this:

fun currentKoinScope(): Scope = currentComposer.run {
    try {
        consume(LocalKoinScope)
    } catch (_: UnknownKoinContext) {
        val ctx = getKoinContext()
        ctx.warnNoContext()
        getKoinContext().scopeRegistry.rootScope
    }
}


fun rememberCurrentKoinScope(): Scope = currentComposer.run {
    remember {
        try {
            consume(LocalKoinScope)
        } catch (_: UnknownKoinContext) {
            val ctx = getKoinContext()
            ctx.warnNoContext()
            getKoinContext().scopeRegistry.rootScope
        }
    }
}

With or without remember for LocalKoinScope, to help reevaluate scope if needed PR I'm opening: https://github.com/InsertKoinIO/koin/pull/1706

[jjkester]

All the more reason to create some extensive test cases for this kind of thing (it is just fairly complicated!)

Have not had time yet to delve into the mentioned issue, but from the surface it seems to make sense. Because the composition local is accessed from within remember it won't invalidate the computed result (since it is not passed as a parameter).

Besides plenty of regression tests I'd also advise to check the performance without remember (not sure how though). Maybe there's a more low-level API that can be leveraged to claw back some of the performance.

From a performance standpoint you'd want to avoid the UnknownKoinContext exception being thrown very often on your main thread.

In my project we did not run into this issue since we do not use koinInject.

[arnaudgiuliani]

All the more reason to create some extensive test cases for this kind of thing (it is just fairly complicated!)

Have not had time yet to delve into the mentioned issue, but from the surface it seems to make sense. Because the composition local is accessed from within remember it won't invalidate the computed result (since it is not passed as a parameter).

Besides plenty of regression tests I'd also advise to check the performance without remember (not sure how though). Maybe there's a more low-level API that can be leveraged to claw back some of the performance.

From a performance standpoint you'd want to avoid the UnknownKoinContext exception being thrown very often on your main thread.

In my project we did not run into this issue since we do not use koinInject.

I went with this PR https://github.com/InsertKoinIO/koin/pull/1723

I've tried manual benchmarking around the internals, with local measure calls. Seems to be faster without remember.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.