koin-compose: use rememberUpdatedState with ParametersDefinition, optimized KoinApplication, KoinContext, KoinIsolatedContext#1768
Conversation
- remove unused UnknownKoinContext
|
is it optimizing around |
|
| fun getKoin(): Koin = currentComposer.run { | ||
| return LocalKoinApplication.current | ||
| } |
There was a problem hiding this comment.
currentComposer.run is unnecessary
There was a problem hiding this comment.
yeah 🤔 I would need to benchmark this, being sure we don't miss something behind the scene
There was a problem hiding this comment.
@arnaudgiuliani
From AndroidX:
xxx.current === currentComposer.consume(xxx)
@Stable
sealed class CompositionLocal<T> constructor(defaultFactory: () -> T) {
internal val defaultValueHolder = LazyValueHolder(defaultFactory)
internal abstract fun updatedStateOf(value: T, previous: State<T>?): State<T>
/**
* Return the value provided by the nearest [CompositionLocalProvider] component that invokes, directly or
* indirectly, the composable function that uses this property.
*
* @sample androidx.compose.runtime.samples.consumeCompositionLocal
*/
@OptIn(InternalComposeApi::class)
inline val current: T
@ReadOnlyComposable
@Composable
get() = currentComposer.consume(this)
}There was a problem hiding this comment.
add @ReadOnlyComposable to getKoin() will optimize the generated code.
Docs: https://developer.android.com/reference/kotlin/androidx/compose/runtime/ReadOnlyComposable
There was a problem hiding this comment.
I have just fixed the conflict. Could you check this PR again 🙏 ?
| fun currentKoinScope(): Scope = currentComposer.run { | ||
| return LocalKoinScope.current | ||
| } |
There was a problem hiding this comment.
currentComposer.run is unnecessary
| private fun Koin.warnNoContext() { | ||
| logger.info("[Warning] - No Koin context defined in Compose, fallback to default Koin context.\nUse KoinContext(), KoinAndroidContext() or KoinApplication() to setup or create Koin context with Compose and avoid such message.") | ||
| logger.info("[Warning] - No Koin context defined in Compose, fallback to default Koin context." + | ||
| "Use KoinContext(), KoinAndroidContext() or KoinApplication() to setup or create Koin context with Compose and avoid such message.") | ||
| } |
There was a problem hiding this comment.
Format string to 2 lines to improve the readability
| ): T { | ||
| val st = parameters?.let { rememberStableParametersDefinition(parameters) } | ||
| return remember(qualifier, scope) { | ||
| scope.get(qualifier, st?.parametersDefinition) | ||
| } | ||
| } | ||
| ): T = koinInject(qualifier, scope, parameters) |
There was a problem hiding this comment.
delegate to koinInject
| LocalKoinScope provides koinApplication.koin.scopeRegistry.rootScope | ||
| ) { | ||
| content() | ||
| } | ||
| LocalKoinScope provides koinApplication.koin.scopeRegistry.rootScope, | ||
| content = content | ||
| ) |
There was a problem hiding this comment.
reduce an unnecessary lambda allocation
{ content() } -> content=content
…oinInject # Conflicts: # compose/build.gradle # compose/gradle/wrapper/gradle-wrapper.properties # compose/koin-androidx-compose-navigation/build.gradle # compose/koin-androidx-compose/build.gradle # compose/koin-compose/build.gradle # projects/compose/koin-compose/src/commonMain/kotlin/org/koin/compose/error/UnknownKoinContext.kt # projects/compose/koin-compose/src/commonMain/kotlin/org/koin/compose/stable/StableHolders.kt
|
Good work. I need to take time to take a tour :) |
|
It's just a api diff confilct I believe @hoc081098 👍 |
I will update it 🙏 |
|
done edit for you @hoc081098 👍 |
Thanks for your help |
Uh oh!
There was an error while loading. Please reload this page.