Compose and Events! I finally got a chance to read the newly released arch docs for android. Lots of good compose related snippets in there. Probably the biggest piece of help has been event handling and how events should always result in a UI state update. I do have questions though and I suppose either manuel, florina, adam or ian might be best to chime in (among others) since they helped write the docs. https://developer.android.com/jetpack/guide/ui-layer/events?continue=https%3A%2F%2Fdeve[…]eveloper.android.com%2Fjetpack%2Fguide%2Fui-layer%2Fevents The example being

// 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?

1.

onUserLogIn

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

@Alex Vanyo 1. & 2. oh boy. this seems super prone to shooting yourself in the foot.

I was being a little bit facetious in that thread since you had made the claim that an

if

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()
}

Okay. So the tldr is that... for example, on button press when I call vm.doSomething() I should not be passing in any functions from the UI layer. So like vm.doSomething(showLoginPage)

fun 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

}

The 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. Consider the following:

@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.

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) cancels

The 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 what 

doSomething

 does, but in general it isn’t safe (and you could classify this as “underhoisting state”)

Interesting!

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.

Makes 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.

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 there

Hm. 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) cancels

Hm. 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

Here’s an example with alternatives to the

logIn

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

.

Or alternatively, check the login state and just compose either the logged in content or the login flow without trampoline navigation at all, but that gets into a different topic 🙂

Ah thanks. I forgot that coroutine cancellation (when adam mentioned it) technically still leaves me with another avenue to do work. i.e. rememberCoroutineScope, and essentially doing it in the coroutine. But I also understand the issues you pointed out. Thanks! I feel much better about this!