-
-
Notifications
You must be signed in to change notification settings - Fork 771
Fix Compose Koin Context Loader and Entry Points #2327
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
Fix Compose Koin Context Loader and Entry Points #2327
Conversation
… to pass it again, avoid restarting new context over
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes Koin context recreation on Android theme changes and unifies the KoinApplication API. The core issue was that RememberObserver.onForgotten() was being misused for cleanup - this callback fires during temporary composition changes (like theme switches), not true abandonment. The fix moves cleanup logic to onAbandoned() for the application context and prevents module re-registration by conditionally creating KoinApplication based on existing context.
Key changes:
- Fixed
CompositionKoinApplicationLoaderto only deallocate references inonForgotten(), not stop Koin entirely - Modified application/scope loaders to accept nullable
KoinApplicationand check for existing Koin context - Introduced new unified
KoinApplication(configuration: KoinConfiguration, logLevel: Level)API with proper deprecations
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| CompositionKoinScopeLoader.kt | CRITICAL BUG: Changed onForgotten() to call close(), contradicting PR intent and fix |
| RememberKoinApplication.kt | Added null-checks to conditionally create KoinApplication only when Koin doesn't exist |
| CompositionKoinApplicationLoader.kt | Made KoinApplication nullable, fixed onForgotten() to only deallocate (not stop), updated start() logic |
| KoinApplication.kt | Added new unified API with deprecations, but has incorrect documentation and ReplaceWith parameter name |
| build.gradle | Changed minSdkVersion from 21 to 23 without explanation in PR description |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...pose/koin-compose/src/commonMain/kotlin/org/koin/compose/scope/CompositionKoinScopeLoader.kt
Show resolved
Hide resolved
| * @throws org.koin.core.error.KoinApplicationAlreadyStartedException if Koin is already started | ||
| * |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation states that this function throws KoinApplicationAlreadyStartedException if Koin is already started, but this is inaccurate.
The implementation now handles the case where Koin is already started by passing null to CompositionKoinApplicationLoader (via rememberKoinMPApplication), which then re-attaches to the existing Koin instance instead of throwing an exception. This is part of the fix to handle theme changes gracefully.
The @throws documentation should be removed or updated to reflect the actual behavior.
| * @throws org.koin.core.error.KoinApplicationAlreadyStartedException if Koin is already started | |
| * |
projects/compose/koin-compose/src/commonMain/kotlin/org/koin/compose/KoinApplication.kt
Outdated
Show resolved
Hide resolved
| defaultConfig { | ||
| applicationId "org.koin.sample.androidx.compose" | ||
| minSdkVersion 21 | ||
| minSdkVersion 23 |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The minSdkVersion is changed from 21 to 23, but this change is not explained in the PR description. While this is just a sample app and API 23 aligns with other sample projects in the repository, it would be helpful to clarify whether this change is:
- Required for the Koin Compose fixes in this PR
- Simply aligning with Compose library requirements
- Unintentional and unrelated to this PR
If this change is unintentional or unrelated, consider removing it to keep the PR focused.
| minSdkVersion 23 | |
| minSdkVersion 21 |
| koin?.logger?.debug("CompositionKoinApplicationLoader - onForgotten") | ||
| //don"t stop here, premature | ||
| koin?.logger?.debug("CompositionKoinApplicationLoader - onForgotten - $this") | ||
| //don"t stop context, premature. Only de-allocate |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect apostrophe character in comment. Use don't instead of don"t.
| //don"t stop context, premature. Only de-allocate | |
| //don't stop context, premature. Only de-allocate |
| koin = KoinPlatform.getKoin() | ||
| koin!!.logger.debug("$this -> re-attach Koin instance $koin") | ||
| } else { | ||
| error("can't start Koin context, no koinApplication argument found nor existing context") |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message should start with a capital letter for consistency with other error messages in the codebase (e.g., line 43: "Can't start Koin from Compose context").
Change to: error("Can't start Koin context, no koinApplication argument found nor existing context")
| error("can't start Koin context, no koinApplication argument found nor existing context") | |
| error("Can't start Koin context, no koinApplication argument found nor existing context") |
…ompose/KoinApplication.kt Co-authored-by: Copilot <[email protected]>
Fix Koin context recreation on theme changes and unify KoinApplication API
Problem
When using KoinMultiplatformApplication() (or KoinApplication()), theme changes on Android were causing Koin to restart, re-registering all modules and recreating dependencies. This was caused by:
Related to #2274
Solution
This PR builds on #2326 and adds:
Migration Guide
Benefits