search

cashapp / molecule

Build a StateFlow stream using Jetpack Compose
https://cashapp.github.io/molecule/docs/1.x/
Apache License 2.0
1.87k stars 81 forks source link

sample-viewmodel module showcases keeping an always hot StateFlow in the ViewModel #273

Open StylianosGakis opened 1 year ago

StylianosGakis commented 1 year ago

This is an issue due to how AAC ViewModel typically is used. The backstack of screens keep the ViewModels in the backstack in memory, relying on behavior like stateIn and collectAsStateWithLifecycle() to turn the "hot" flows into "cold" ones when there no longer are observers on those states.

The example inside molecule could be altered to show a way which developers who still use ViewModels can adopt in their own apps without this pretty important downside of keeping all molecule StateFlows always hot in the backstack.

This was briefly discussed already in https://github.com/cashapp/molecule/issues/271, but I do believe that it'd be worth it for the sample to be updated to solve this issue.

JakeWharton commented 1 year ago

We are not users of ViewModel (the library), and would very much welcome any improvements to that sample which showcases the "right" way to use it with them.

StylianosGakis commented 1 year ago

Yes, I totally understand this, and I really appreciate that you have a module like that in this repo in the first place. It really helped me for example take the first step to try out molecule, so thank you for that.

I've raised a PR here https://github.com/cashapp/molecule/pull/274. I'd love for someone else who's more familiar with AAC ViewModel to also take a look at it and discuss the approach I've taken to make those two libraries work with each other.

The one thing I am the least happy about is the fact that we lose the free synchronous first value that you get from using launchMolecule, but I don't see a way to make that work without having the StateFlow stay hot basically forever while it's in the backstack, as discussed here https://github.com/cashapp/molecule/issues/271#issuecomment-1644311575.