psteiger / flow-lifecycle-observer

A plugin for collecting Kotlin Flow in an Android Lifecycle-aware manner.
MIT License
57 stars 7 forks source link

Is it possible to also make those extension functions for StateFlow? #8

Open benjdero opened 3 years ago

benjdero commented 3 years ago

The problem with using Flow or SharedFlow as a replacement for LiveData with Jetpack Compose is that Flow.collectAsState() require to pass as argument the initial value of the Flow. StateFlow.collectAsState() doesn't have this requirement as it use the initial value of the StateFlow. Architecture wise it seems problematic to specify the initial value from within the View, it should be up to the ViewModel (which contain this Flow) to define the initial value.

I've found your library after reading the blog posts you linked in its description, and was hopping you got a solution for this.

psteiger commented 3 years ago

I'm thinking maybe you should convert your SharedFlow or Flow to StateFlow in your ViewModel using stateIn(). Views are Stateful and match better with StateFlow. They need an initial value, even if it's empty value. That approach would not affect your repositories, meaning non-view components could still rely on SharedFlow behavior from your repos, and would play well with your composable views.

Another approach would be the MVI architecture approach, where you actually collect the flows in your ViewModel and deliver the whole view-state to the view. Then it would not matter to the view what is happening behind the scene -- they would get snapshots of a full, complete state over time. Of course that would be quite the rework, assuming you're using the Google-suggested MVVM approach.

What do you think? Would that work out? I haven't used Compose in Android yet, but given your problem that's what I came up with at first thought.

benjdero commented 3 years ago

I'm thinking maybe you should convert your SharedFlow or Flow to StateFlow in your ViewModel using stateIn().

I already use StateFlow in my ViewModel. The problem is that the solution proposed in this blog post doesn't work.

@Composable
fun LocationScreen(locationFlow: Flow<Flow>) {
    val lifecycleOwner = LocalLifecycleOwner.current
    val locationFlowLifecycleAware = remember(locationFlow, lifecycleOwner) {
        locationFlow.flowWithLifecycle(lifecycleOwner.lifecycle, Lifecycle.State.STARTED)
    }
    val location by locationFlowLifecycleAware.collectAsState()
    // Current location, do something with it
}

It does not compile because collectAsState() need an initial value as it is used on a Flow and not a StateFlow (flowWithLifecycle() always return a Flow even when used on a StateFlow).

This was reported 2 months ago in the comments of this blog post but so far no answer from his author.

So I took a look around for different solutions, like how to use LifecycleOwner.addRepeatingJob instead, and found zero example, beside your library, which if I understood correctly, provide simple syntax wrapping all this boilerplate code. But your extensions functions seems to have the same problem, as they return a Flow, not a StateFlow.

Do you see a solution to this problem? Would it make a nice addition to your library?

psteiger commented 3 years ago

I see, @benjdero.

Perhaps the collectAsState() method was modified after its introduction to require an initial value.

Is your locationFlow a StateFlow? If it is, would it work to change the locationFlow parameter of your Composable method to StateFlow, and pass locationFlow.value as initial value to collectAsState()? Not very elegant, but might work. A Flow<T>.collectAsState(initial: R) would need an initial value, as Flow will have none, but StateFlow<T>.collectAsState() should not need it. But if there's no StateFlow<T>.collectAsState(), Flow<T>.collectAsState(initial: R) passing the StateFlow.value as parameter for initial value should work.

I will need some time to experiment and study this matter, and might have better input later.

benjdero commented 3 years ago

Perhaps the collectAsState() method was modified after its introduction to require an initial value.

I think the author made some small tweaks to his code while writing this blog post and didn't double check if it still work after that. The Flow<Flow> parameter make no sense to me for example.

Is your locationFlow a StateFlow? If it is, would it work to change the locationFlow parameter of your Composable method to StateFlow, and pass locationFlow.value as initial value to collectAsState()? Not very elegant, but might work.

This may be a workaround, I will take a deeper look at all of this tomorrow.