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

KoinApplication Multiplatform Config - Logger + Androic context automatic injection #2086

Merged
merged 3 commits into from
Dec 23, 2024

Conversation

arnaudgiuliani
Copy link
Member

Unlock capacity to help KoinApplication to have common DI modules config only.

Allow KoinApplication composable to hook current configuration to:

  • preconfigure platform related logger
  • inject Android context

@arnaudgiuliani arnaudgiuliani added this to the 4.1.0 milestone Dec 23, 2024
@arnaudgiuliani arnaudgiuliani merged commit 3c18fac into 4.1.0 Dec 23, 2024
8 checks passed
@arnaudgiuliani arnaudgiuliani deleted the koinapplication_mp_config branch December 23, 2024 13:52
internal actual fun composeConfiguration(loggerLevel : Level,config : KoinApplication.() -> Unit) : KoinConfiguration {
val current = LocalContext.current
return koinConfiguration {
androidContext(current)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect as it will capture activity context and leak it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, might assure to capture only app context 👍


@Composable
internal actual fun composeConfiguration(loggerLevel : Level,config : KoinApplication.() -> Unit) : KoinConfiguration {
return koinConfiguration {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this builds configuration on every recomposition and will lead to performance issues

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

Successfully merging this pull request may close these issues.

2 participants