psteiger / flow-lifecycle-observer

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

Consider using launchWhenStarted instead of launch #2

Closed p-lr closed 3 years ago

p-lr commented 3 years ago

I recently came across an issue that was due to the fact that the flow collection started with observe operator is actually performed right before the lifecycle reaches the STARTED state.

Checking the source code, we can indeed see that:

/**
  * Started state for a LifecycleOwner. For an {@link android.app.Activity}, this state
  * is reached in two cases:
  * <ul>
  *     <li>after {@link android.app.Activity#onStart() onStart} call;
  *     <li><b>right before</b> {@link android.app.Activity#onPause() onPause} call.
  * </ul>
  */
  STARTED,

Well, I can at least tell that onStart() on a lifecycle observer is indeed invoked right before an androidx.Fragment reaches the STARTED state.

It may all sound not really important, but it turned out to matter today for some particular case. I could easy fix that with using launchWhenStarted instead of launch, inside the operator implementation.

This what I'm using in my project (it also has different naming, which I find more explicit. It also has a collectWhileResumed variant):

@file:Suppress("unused")

package com.peterlaurence.trekme.util

import androidx.lifecycle.DefaultLifecycleObserver
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.lifecycleScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.collect

/**
 * Utility extension function to start collecting a flow when the lifecycle is started,
 * and *cancel* the collection on stop, with a custom collector.
 * This is different from `lifecycleScope.launchWhenStarted{ flow.collect{..} }`, in which case the
 * coroutine is just suspended on stop.
 */
inline fun <reified T> Flow<T>.collectWhileStarted(
        lifecycleOwner: LifecycleOwner,
        noinline collector: suspend (T) -> Unit
) {
    ObserverWhileStartedImpl(lifecycleOwner, this, collector)
}

/**
 * Utility extension function on [Flow] to start collecting a flow when the lifecycle is started,
 * and *cancel* the collection on stop.
 */
inline fun <reified T> Flow<T>.collectWhileStartedIn(
        lifecycleOwner: LifecycleOwner
) {
    ObserverWhileStartedImpl(lifecycleOwner, this, {})
}

class ObserverWhileStartedImpl<T>(
        lifecycleOwner: LifecycleOwner,
        private val flow: Flow<T>,
        private val collector: suspend (T) -> Unit
) : DefaultLifecycleObserver {

    private var job: Job? = null

    override fun onStart(owner: LifecycleOwner) {
        job = owner.lifecycleScope.launchWhenStarted {
            flow.collect {
                collector(it)
            }
        }
    }

    override fun onStop(owner: LifecycleOwner) {
        job?.cancel()
        job = null
    }

    init {
        lifecycleOwner.lifecycle.addObserver(this)
    }
}

inline fun <reified T> Flow<T>.collectWhileResumed(
        lifecycleOwner: LifecycleOwner,
        noinline collector: suspend (T) -> Unit
) {
    ObserverWhileResumedImpl(lifecycleOwner, this, collector)
}

class ObserverWhileResumedImpl<T>(
        lifecycleOwner: LifecycleOwner,
        private val flow: Flow<T>,
        private val collector: suspend (T) -> Unit
) : DefaultLifecycleObserver {

    private var job: Job? = null

    override fun onResume(owner: LifecycleOwner) {
        job = owner.lifecycleScope.launchWhenResumed {
            flow.collect {
                collector(it)
            }
        }
    }

    override fun onPause(owner: LifecycleOwner) {
        job?.cancel()
        job = null
    }

    init {
        lifecycleOwner.lifecycle.addObserver(this)
    }
}
psteiger commented 3 years ago

Hi @peterLaurence,

Your change looks good. Could you please tell more about the use case where launchWhenStarted mattered?

Please consider opening a pull request for your changes - both the naming of the functions and the launchWhenStarted change. I'll be happy to review them and integrate.

Thanks!

p-lr commented 3 years ago

Hi @psteiger,

When the flow collection is done inside a fragment, and when we want to trigger a view change based on a particular event, we can use collectWhileStarted operator. The issue I had was due to the fact that I had to remove a child view and add it back to the view hierarchy, assigning the same id. If this is done when the state is STARTED, the newly added view has its state automatically restored (using the state of the previous view). While if this is done when the state isn't yet STARTED, the view state isn't automatically restored.

On the other hand, for other use cases, I don't think using launchWhenStarted instead of launch will make a difference. Anyway, I think that this is a good thing that the name of the operator explicitly tells when actions are triggered. If that sounds ok to you, I can do a PR.

psteiger commented 3 years ago

Ok @peterLaurence , I understand the failing use case and I don't think using a lifecycle-specific launch would break other use cases.

A PR would be welcome! Thanks.

p-lr commented 3 years ago

Ok great, I'll do that.

psteiger commented 3 years ago

Solved by PR #3

gmk57 commented 3 years ago

onStart() on a lifecycle observer is indeed invoked right before an androidx.Fragment reaches the STARTED state

@peterLaurence Are you sure? I see that lifecycle.currentState is indeed CREATED during Activity/Fragment onStart() (in accordance to the docs), but it is already STARTED during LifecycleObserver.onStart(), where flow collection is launched.

You can also check the code: mState is currentState, and sync() is where observers are notified.

p-lr commented 3 years ago

@gmk57 I'll double check. Anyway, there was a difference between using launchWhenStarted and launch. Even though, if you're right, there shouldn't have been a difference.