JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
15.17k stars 1.11k forks source link

Remove compose ui fallback from LocalViewModelStoreOwner #4790

Closed DevSrSouza closed 1 month ago

DevSrSouza commented 1 month ago

I'm working on a project using Compose Runtime and I want to be able to use ViewModel Compose KMP without depending on Compose UI. Everything works great but the binary size of the app even using Compose Runtime only it come with the hole Compose UI inside, by suggestion would be remove the fallback on LocalViewModelStoreOwner for KMP targets (jbMain).

For Android Main it makes sense, there are alot of components that could have the ViewModelStore that you can fallback to, on KMP side, I don't think it make much sense, all the Compositions are already initialized by the core of the project, I don't see why it could not provide LocalViewModelStoreOwner on all places that initialize the Content Composable.

This is where it depends on Compose UI. https://github.com/JetBrains/compose-multiplatform-core/blob/jb-main/lifecycle/lifecycle-viewmodel-compose/src/jbMain/kotlin/androidx/lifecycle/viewmodel/compose/LocalViewModelStoreOwner.jb.kt#L27

The Compose Native already has the dependcy in ViewModel, I don't see why it can't have dependency in viewmodel-compose https://github.com/JetBrains/compose-multiplatform-core/blob/3dc1df17bef998f72ff69c942af4be31dc8145f0/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/DefaultViewModelOwnerStore.skiko.kt#L36

https://github.com/JetBrains/compose-multiplatform-core/blob/3dc1df17bef998f72ff69c942af4be31dc8145f0/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/ComposeContainer.uikit.kt#L433-L447

Also, I want to add support built in on Voyager, but Voyager Navigator relies only on Compose Runtime, depending on lifecycle-viewmodel-compose would bring this heavy dependency to it and blocking projects that are using Voyager with Compose Runtime only.

JakeWharton commented 1 month ago

The dependency between compose-ui and viewmodel-compose should likely be inverted. I already made this change for compose-ui and lifecycle-compose for the same reason, so I suspect a similar change could be made here.

MatkovIvan commented 1 month ago

Thanks for getting in touch. Let me summarize my thoughts from our DM here.

suggestion would be remove the fallback on LocalViewModelStoreOwner for KMP targets (jbMain)

I believe this should be the same across the platforms

This is where it depends on Compose UI

Yes, but Android depends on UI only in similar line too.

I don't see why it can't have dependency in viewmodel-compose

This module will bring dependency not only to itself - it has dependencies that UI doesn't. We'd like to avoid extra dependencies in Compose UI module.

My suggestion here is to open an issue in Google's tracker and try to do such a revert in AOSP first.

I already made this change for compose-ui and lifecycle-compose for the same reason, so I suspect a similar change could be made here.

Yep, thanks for that. But I guess there the situation is a bit different - it really depends on UI part in terms of finding UI view as default provider. My best guess here at the moment is to extract the required part rom ui to separate ui-something module and depend only on that.

Anyway, I'm going to close it as not planned for now, since I believe that it should be done in AOSP first. Please post a link to the issue in AOSP once created.

marcellogalhardo commented 1 month ago

@DevSrSouza, could you please open an issue in our AOSP lifecycle issue tracker so we can investigate this further? Please cross-link both issues for additional context and easier tracking. Thank you!