Status Update
No update yet.
Comments
an...@google.com
reemission of the same liveData is racy
du...@google.com
du...@google.com
an...@google.com
@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}
il...@google.com
ap...@google.com
ki...@protonmail.com
I actually have a WIP fix for it:
if your case is the one i found (emitting same LiveData multiple times, as shown in #5) you can work around it by adding a dummy transformation.
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData.map {it })
emitSource(subLiveData.map {it} )
}
al...@mercari.com
Branch: androidx-master-dev
commit af12e75e6b4110f48e44ca121466943909de8f06
Author: Yigit Boyar <yboyar@google.com>
Date: Tue Sep 03 12:58:11 2019
Fix coroutine livedata race condition
This CL fixes a bug in liveData builder where emitting same
LiveData source twice would make it crash because the second
emission registry could possibly happen before first one is
removed as source.
We fix it by using a suspending dispose function. It does feel
a bit hacky but we cannot make DisposableHandle.dispose async
and we do not want to block there. This does not mean that there
is a problem if developer disposes it manually since our emit
functions take care of making sure it disposes (and there is
no other way to add source to the underlying MediatorLiveData)
Bug: 140249349
Test: BuildLiveDataTest#raceTest_*
Change-Id: I0b464c242a583da4669af195cf2504e2adc4de40
M lifecycle/lifecycle-livedata-ktx/api/2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/current.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_current.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_current.txt
M lifecycle/lifecycle-livedata-ktx/src/main/java/androidx/lifecycle/CoroutineLiveData.kt
M lifecycle/lifecycle-livedata-ktx/src/test/java/androidx/lifecycle/BuildLiveDataTest.kt
du...@google.com
Is this the same issue reported in
which was set as Won't Fix? Because we did follow the recommendation from there to skip the first state, so we will have to revert that code. https://issuetracker.google.com/issues/190788885
Yes, part of the intention here is that people will no longer need to add this workaround in their code.
ha...@gmail.com
Let's take a search screen example. When the user first enters the search screen, nothing is loading until the user enters its search in the textfield then clicks the search button. Then the state should be Loading.
I guess we should use the workaround provided here
ma...@gmail.com
It might be too late for some people, but if people want to know how to tackle this I've played around with it on a pet project ( didn't test this in a prod environment, so a heads up there ):
@Composable
fun <T : Any> LazyPagingItems<T>.refreshLoadState(): State<LoadState> = produceState<LoadState>(
LoadState.NotLoading(false),
) {
snapshotFlow { loadState }
.drop(1)
.map { it.refresh }
.collect { value = it }
}
cl...@google.com
Re
Given the most common use case displays paged data right away, defaulting LoadState to Loading means the most common use case will not have to drop first LoadState (versus if it were NotLoading).
re...@gmail.com
I don't get how initial state being anything would affect the use case of displaying paged data right away. Wouldn't the LoadState change before the user has time to notice?
IMHO, initial state being NotLoading would be the sanest choice. In the case of waiting before starting to load data, the correct state would be shown to the user and in the case of loading data right away, it would immediately change to Loading, so the correct state would be shown to the user in this case as well.
du...@google.com
cl...@google.com
Re collectAsLazyPagingItems compose launches collection on Pager right away. By the time pager.collectAsLazyPagingItems() returns, data is loading, so initial Loading is the correct state.
Initializing with NotLoading also causes other issues (which is why Paging with Views was also refactored to not initialize with NotLoading):
- For users who rely on first
NotLoadingstate as indication of refresh complete, they have to manually filter out the firstNotLoading - If loading animation (i.e. spinners) are based on LoadStates, you could end up seeing a blank screen --> spinner --> data loaded in, and that is not a good user experience.
Description
In Paging 3.1 we made the behavior change to always wait for real
LoadStateupdates fromPagingSource/RemoteMediatorbefore sending them downstream to user. This allows a developer to react on page loads without needing to filter out the initial / intermediateNotLoadingstates. This also prevents sending aCombinedLoadStateswheremediatorstates isnulleven though we are usingRemoteMediator.Ideally we would not send any "initial" load state downstream, I'm not sure if that means we need to make
.loadStatenullable inLazyPagingItemsas it may not be idiomatic for Compose.Note that this initial state will also end up affecting invalidation, retry, recompose, etc.