Colton Idle
12/16/2021, 10:27 PM// composable
val currentOnUserLogIn by rememberUpdatedState(onUserLogIn)
LaunchedEffect(viewModel.uiState) {
if (viewModel.uiState.isUserLoggedIn) {
currentOnUserLogIn()
}
}
I have a bunch of questions
1. I don't understand the use of val currentOnUserLogIn by rememberUpdatedState(onUserLogIn)
. Why doesn't this code sample just call onUserLogIn
directly?
2. What goes into LaunchedEffect arg exactly? It seems like its easy to mess up what exactly goes in there and therefore shoot yourself in the foot. I can expand on this if that didn't make sense.
3. Why can't I just pass the ref of onUserLogIn event into the ViewModel and onSuccess of a login network call it would just call onUserLogIn in the VM itself?Alex Vanyo
12/16/2021, 10:47 PMonUserLogIn
is a plain lambda passed to LandingScreen
. Because the LaunchedEffect
doesn’t key on the onUserLogIn
, the LaunchedEffect
wouldn’t restart even if there is a new onUserLogIn
that causes LandingScreen
to recompose. If we called onUserLogIn
directly, we could be calling the stale, old version of it. There’s another example explained in more detail here: https://developer.android.com/jetpack/compose/side-effects#rememberupdatedstate
2. https://developer.android.com/jetpack/compose/side-effects#restarting-effects has details about the general form all of these keys take. In short, you should add the keys that you want to use to restart the Effect
. And yes, this can cause some very subtle behavior changes depending on the exact keys and the effect.
3. If onUserLogIn
captures an Activity
or other UI object (such as to navigate to another activity or navigate via navigation-compose
) you can very easily cause a memory leak or get strange behavior because the AAC ViewModel
outlives the UI. Let’s say onUserLogIn
needs make a network call, which could take a few seconds. In that time, the user could have rotated their device, the activity gets recreated, and your lambda no longer refers to the correct instance of the Activity
. It’s a bit more subtle, but giving a composition-scoped object to something that outlives it is like storing a view reference or activity reference in a ViewModel
Colton Idle
12/17/2021, 1:28 AMAdam Powell
12/17/2021, 2:15 AMif
statement is business logic and you wanted that in your ViewModel. I was implying an implementation of that function that doesn't retain references to those onSuccess/onFailure functions, but that literally just wraps an if
. 🙂
fun validateMembership(
onSuccess: () -> Unit,
onFailure: () -> Unit
) {
if (hasValidMembership) onSuccess() else onFailure()
}
Colton Idle
12/17/2021, 2:44 PMfun doSomething(showLoginPage: () -> Unit){
//before we do something, check to make sure the user is authenticated, if not then force them to login
if (userManager.isAuth.not()) {
showLoginPage()
return
}
//do something
}
Alex Vanyo
12/20/2021, 7:17 PM@Composable
fun SomeContainer(doSomething: (LazyListState) -> Unit) {
val lazyListState by rememberLazyListState()
Button(onClick = { doSomething(lazyListState) }) { /* ... */ }
// ...
}
The exact behavior depends on what doSomething
does, but in general it isn’t safe (and you could classify this as “underhoisting state”)
Suppose doSomething
stores the LazyListState
in a some var
somewhere, and now let’s say that SomeContainer
ends up getting removed from composition (there’s some if
above it that switches to false
).
That LazyListState
will still be in that var
, but it’s a stale reference to something that no longer exists in composition. If SomeContainer
came back on screen, it’d create a different LazyListState
, so now there are two different instances hanging around.
The same sort of issue occurs with the AAC ViewModel
, which by construction outlives the UI. As a consequence, any reference to the UI given or passed to a ViewModel
(including via lambdas) can be dangerous.Adam Powell
12/20/2021, 7:41 PMColton Idle
12/22/2021, 7:34 PMThe general issue at play is having a longer-lived component having a reference to a shorter-lived component, which is something that could happen in a lot of places if you’re not careful.👍
The exact behavior depends on whatInteresting!does, but in general it isn’t safe (and you could classify this as “underhoisting state”)doSomething
The same sort of issue occurs with the AACMakes sense now. Thanks. I think for some reason since my events were being passed from the root activity (using single activity arch) I thought that the activity essentially outlives just about everything else... especially compose screen VMs. I think Adam explained to me that all VMs technically can/will still outlive the activity because that's what they're meant to do., which by construction outlives the UI. As a consequence, any reference to the UI given or passed to aViewModel
(including via lambdas) can be dangerous.ViewModel
Especially in the case where an Activity overrides all config changes, another option is to plain state holders that live within the UI lifetime as opposed to `ViewModel`s that outlive the UI. This section about “State in Compose” goes into a lot more detail about the differences and tradeoffs thereHm. I wonder if this is basically what I said above. I was (originally) under the impression that my Activity is basically the largest scope and largest lifetime and so thats why its safe to just pass stuff from it, right on through to all of my composable "screen" VMs. I will take a look at those links. Thanks Re: Adam
one way to make this sort of thing easier to reason about is to "simply" not hold long-lived references to any function parameter beyond when that function returns (or throws). Thanks to suspend functions you can use coroutine cancellation to clean up when the caller (that owns those shorter-lived references) cancelsHm. I feel like I understood everything up to this point and this sorta went over my head. If you have an example, that would help but no worries. I will re-read after I get some rest. Cheers
Alex Vanyo
12/22/2021, 8:32 PMlogIn
example from the documentation.
fun doSomething(onUserLogIn: () -> Unit) {
viewModelScope.launch {
if (logIn()) {
onUserLogIn()
}
}
}
suspend fun doSomething(onUserLogIn: () -> Unit) {
if (logIn()) {
onUserLogIn()
}
}
Let’s suppose both of those functions are defined in a ViewModel
.
For the first, non-suspending one, doSomething
returns immediately, and we start work in the viewModelScope
. This scope can outlive the UI, but we have a reference to onUserLogIn
(which might call navigate, or have some other reference to the UI). This is a memory leak at least.
For the second, suspending one, doSomething
suspends while trying to log in, and this must be done in some scope. If we call this method from a UI scope (say, rememberCoroutineScope()
), then doSomething
will be cancelled if the UI is destroyed, and therefore we don’t have a memory leak. We can take advantage of structured concurrency, and use things like try/finally
to cleanup resources upon a CancellationException
.
The suspending version is better, but still has a subtle problem for a case like logging in. What if doSomething
gets cancelled, just after “logging in” but before we have a chance to execute onUserLogIn
? To the data layer and the rest of the app, the user is logged in, but we never got a chance to navigate away.
For that reason, we always check the log in state, and do the required navigation away from the login screen in a LaunchedEffect
.Adam Powell
12/22/2021, 9:30 PMColton Idle
12/23/2021, 1:36 AM