Hi I have 3 questions regarding state. I have the ...

# compose

[Lilly]

Hi I have 3 questions regarding state. I have the following composable:

@Composable
fun BluetoothDeviceListAdapterComponent(
    items: List<BluetoothDeviceWrapper>,
    connectVM: ConnectViewModel,
    onConnected: () -> Unit
) {

    // initial state is a problem here
    val connectState: BluetoothConnectState by connectVM.state.collectAsState()

    onCommit(connectState) {
        Timber.w("connectState changed")
        when(connectState) {
            is BluetoothConnectState.Loading -> {
                Timber.w("Loading")
            }
            is BluetoothConnectState.Success -> {
                Timber.w("GoTo next screen.")
                onConnected()
            }
            is BluetoothConnectState.Failure -> {
                // how to call toast from here?
            }
        }
    }

state is a MutableStateFlow and can be Loading, Success or Failure(val errorMessage). First problem is that MutableStateFlow needs an initial value which triggers onCommit on first start but it should only be triggered when viewModel changes its state explicitly. Second problem is when state is Success and next screen is started: When I get back to the screen, onCommit is trigged with value Success and starts the next screen again, although the state hasn't changed. Last question would be how to call a Toast from onCommit ? Is this approach even optimal for my use case?

[Sean McQuillan [G]]

First pass – I'd suggest taking a read through https://developer.android.com/jetpack/compose/state. This code has some side-effects from recomposition (e.g. onConnected()) which will be hard to manage in recomposition.

It appears you're treating connectedState as an event in this code and trying to modify other states in response to it changing. This route will make UDF hard to implement.

There's a few bits of state here that I see, that you may find easier if you model them separately:

1. The current screen
2. A toast message
Then, in compose consume the states that describe the UI at the present time.

[Sean McQuillan [G]]

For snackbar you can use a snackbar host

[Lilly]

@Sean McQuillan [G] Thanks for your answer.

It appears you're treating connectedState as an event in this code and trying to modify other states in response to it changing.

The scenario is a little bit different:

@Composable
fun BluetoothDeviceListAdapterComponent(
    items: List<BluetoothDeviceWrapper>,
    viewModel: ConnectViewModel,
    onConnected: () -> Unit
) {
    val uiState: BluetoothConnectState by viewModel.state.collectAsState() /* state */

    when (uiState) {
        is BluetoothConnectState.Loading -> Text("Loading...")
        is BluetoothConnectState.Success -> onConnected() // --> next screen
        is BluetoothConnectState.Failure -> // TODO
    }

    LazyColumnFor(items) { item ->
        BluetoothDeviceListItemComponent(item, onActionClick = { device ->
            viewModel.connect(device) /* event */
        }
    })
}

Every BluetoothDeviceListItemComponent has a Button which triggers the onActionClick event which in turn triggers a non-suspending function in the VM that is consuming a flow. The results are stored in a StateFlow<BluetoothConnectState> which is observed by the BluetoothDeviceListAdapterComponent . The first result is Loading, when bt device is connected, state changes to Success otherwise Failure. I can't figure out how to handle the initial state. A Loading composable should only be displayed after event onActionClick is triggered because the flow itself already exposes a Loading state at the right time.

[Sean McQuillan [G]]

👀

[Sean McQuillan [G]]

Immediately jumping out at me is that onConnected is called by recomposition instead of an event handler. Trying to figure out how to advise an alternative

[Sean McQuillan [G]]

Ah figured it out – so the underhoisting stems from passing both the viewModel and onConnected

[Sean McQuillan [G]]

What's happening is that onConnected as an event changes state that's hoisted somewhere higher in the tree. This leads you to build a composition -> event bridge like here where you call onConnected by recomposition

[Sean McQuillan [G]]

First thing to note about this code style is (1) onConnected may be called multiple times due to recomposition and (2) onConnected may be called during a composition that is later discarded when it shouldn't

[Lilly]

The code does not have to be in the BluetoothDeviceListAdapterComponent composable. It could be also in the highest level

[Sean McQuillan [G]]

This is the third part of the hoisting advice in the state in compose page (it's a bit subtle)

[Sean McQuillan [G]]

"When two states are modified in response to the same events, they should be hoisted together"

[Sean McQuillan [G]]

In this case, the "current screen" state should be determined at the same place as the connected status state

[Sean McQuillan [G]]

which is in the ViewModel

[Sean McQuillan [G]]

That will make this end up working a bit closer to expectations – as another cleanup you can also remove the direct dependency on the viewModel here and pass onConnectDevice: (Device) -> Unit as a parameter to this composable

[Lilly]

Your are too fast for me. Can you please explain what you mean with

What's happening is that onConnected as an event changes state that's hoisted somewhere higher in the tree

[Sean McQuillan [G]]

Ok got the general gist of everything. I'd suggest replacing this with composable that takes items: ..., uiState: LoadingState, onConnectDevice: ...) and simply displays stuff

[Sean McQuillan [G]]

then the caller is responsible for managing all this state

[Sean McQuillan [G]]

For the initial loading screen for collectAsState – if you intend for there to be a pre-loading state I'd suggest modeling it (either explicitly as`Initial` or by using null)

[Lilly]

What's happening is that onConnected as an event changes state that's hoisted somewhere higher in the tree

What state does onConnected change?

[Sean McQuillan [G]]

To get to the next screen it has to modify some state higher in the tree?

[Sean McQuillan [G]]

(I don't have code so I had to fill in some details 😉 )

[Lilly]

Yes thats right. I'm using the router-compose library from zsoltk

[Lilly]

In this case, the "current screen" state should be determined at the same place as the connected status state -> which is in the ViewModel

What is the current screen state here?

[Sean McQuillan [G]]

Ok so taking a step back – to display this composable there is a state in the router library that says "this is the current screen"

[Sean McQuillan [G]]

Then, there's some code between there and here that produces a ViewModel

[Sean McQuillan [G]]

And then this composable?

[Lilly]

So far so good

[Sean McQuillan [G]]

calling onConnected is this composables request to modify that state (it's an event)

[Lilly]

True

[Sean McQuillan [G]]

This code here (which pretty much everyone including me has written once in Compose) is a state to event bridge

[Sean McQuillan [G]]

The uiState is driving recomposition of this composable, then as a side-effect of recomposition it's calling onConnected

[Sean McQuillan [G]]

the fundamental reason for this is that onConnected and uiState are hoisted to different levels

[Sean McQuillan [G]]

There's two basic solutions to this:

1. Hoist uiState to the same place as the state onConnected modifies
2. Call onConnected from an event handler (instead of in response to state changes)

[Lilly]

ok got you

[Sean McQuillan [G]]

Given the existence of a viewModel – it seems likely that it's this viewModels responsibility to dispatch the follow on event (just taking a guess at the rest of the code)

[Sean McQuillan [G]]

Then, if you encapsulate that, then this composable doesn't need to know about anything except a header state, items, and the event to notify the viewModel about an item click

[Sean McQuillan [G]]

(which, makes this composable a lot easier to write 🙂 )

[Lilly]

Give me a moment

[Lilly]

@Sean McQuillan [G] ok I can reduce the composable to items and a onConnectDevice: (Device) -> Unit parameter. This will hoist the state up to a higher level but how to go further from here. Fact is that onConnected is a reaction to a state change not? or what do you mean with

it seems likely that it's this viewModels responsibility to dispatch the follow on event

[Sean McQuillan [G]]

So the desired result is that calling the event handler (viewModel.connect in the code you pasted) eventually calls onConnected

[Lilly]

Thats the VM part:

private val _state: MutableStateFlow<ConnectBluetoothDeviceUseCase.BluetoothConnectState> =
        MutableStateFlow(
            ConnectBluetoothDeviceUseCase.BluetoothConnectState.Loading
        )    

    val state: StateFlow<ConnectBluetoothDeviceUseCase.BluetoothConnectState> = _state.asStateFlow()

    fun connect(device: BluetoothDevice) {
        viewModelScope.launch {
            connectBluetoothDevicesUseCase.connect(device).collect {
                _state.value = it
            }
        }
    }

[Sean McQuillan [G]]

ah perfect

[Lilly]

Sorry for the inconvenience 🙈

[Sean McQuillan [G]]

So, you already have the continuation where you're doing _state.value = it

[Sean McQuillan [G]]

Nah this is more fun than the docs I'm putting off 🙂

[Sean McQuillan [G]]

Right there you can call onConnected safely (though since it's an event I'd recommend ensuring it's on Main dispatcher before calling it)

[Sean McQuillan [G]]

The big insight here is that this ViewModel fits itself into a UDF system, and is not the "top" of the UDF diagram – the navigation composable above it is above this ViewModel in a UDF diagram

[Sean McQuillan [G]]

so you can call an event "up" here while still doing UDF

[Lilly]

I have updated my code snippet for the missing _state.value part. onConnected is this:

@Composable
fun DcInternEntryPoint(defaultRouting: Routing) {
    Router(defaultRouting) { backStack ->
        when (backStack.last()) {
            is Routing.Scanner -> {
                ScannerScreen(onConnected =  {
                    backStack.push(Routing.Test)
                })
            }
        }
    }
}

[Sean McQuillan [G]]

This looks correct

[Sean McQuillan [G]]

So now everything should be working except the header state?

[Lilly]

I have really problems with your terms ^^ What do you mean with header state. And I'm also not aware of an UDF diagram

[Sean McQuillan [G]]

Moment

[Sean McQuillan [G]]

[Sean McQuillan [G]]

Er I forgot one

[Sean McQuillan [G]]

[Sean McQuillan [G]]

practically you can consider the VM an "implementation detail" of ScannerScreen and not draw it

[Sean McQuillan [G]]

But this is a full UDF diagram

[Sean McQuillan [G]]

"Header state" is the state that tells this composable what to display in the header (or if it should be hidden)

[Sean McQuillan [G]]

In the code we started with it was derived from uiState

[Sean McQuillan [G]]

(er sorry defining acronym: UDF = Unidirectional Data Flow)

[Lilly]

Ah ok. Thanks for the diagram! So you said I should call onConnect in VM but then I'm aksing how should this look like. I have no reference to the backStack in VM???

[Lilly]

onConnected:

ScannerScreen(onConnected = {
		backStack.push(Routing.Test)
	})

[Sean McQuillan [G]]

Oh it's just a lambda – you can pass it to the VM

[Sean McQuillan [G]]

You wouldn't actually deal with the back stack in the ScannerScreenViewModel, but you can call onConnected() from it

[Lilly]

@Sean McQuillan [G] I refactored screen implementation and cleaned up the code. So thats what I have so far for the VM:

fun connect(device: BluetoothDevice, onConnected: () -> Unit) {
        viewModelScope.launch {
            connectBluetoothDevicesUseCase.connect(device).collect { connectState ->
                when (connectState) {
                    BluetoothConnectState.Loading -> _state.value = connectState
                    BluetoothConnectState.Success -> onConnected()
                    is BluetoothConnectState.Failure -> _state.value = connectState
                }

            }
        }
    }

[Lilly]

And the UI stuff

UI

[Lilly]

It's working and now its not affected from recomposition. But I guess it's still not optimal. I also have a question regarding hoisting. The more I hoist state to the higher level composable the more levels have to recompose. Isn't this a circumstance which should be avoided? For example lets assume my ScreenContent function would display additional elements like a Button, Text or whatever. Now with the current implementation, every time discoveryState is changed, not only the BluetoothDeviceListAdapterComponent composable recomposes but also the additional elements like Button, Text would recompose. --> So wouldn't it be better to keep the discoveryState in BluetoothDeviceListAdapterComponent so that this is the only composable that is recomposed?

[Sean McQuillan [G]]

Compose skips recomposition aggressively for both parents and children of a composable. So I wouldn't motivate under hoisting for these performance reasons

[Sean McQuillan [G]]

There's two major problems that arise from underhoisting:

1. The state->event trampoline always introduces one recomposition delay to trigger the event. This may be as long as a frame.
2. It leads to duplicated state

[Sean McQuillan [G]]

Iff you profiled and found that this state was a trigger for unacceptable levels of work after skipping there's a few solutions:

1. Break up the composable into smaller sibling components that have a better skipping story (while leaving state hoisted).
2. Work with Compose to make this object more skippable by improving it's stability
3. Pass a state-lambda parameter (a lambda that returns the current value of the state) instead of the raw value, this has the effect of making a box that allows recomposition to skip until it's unboxed

[Sean McQuillan [G]]

But, taking a step back, the compose compiler/runtime is designed intentionally to produce efficient programs in the presence of hoisting like this.

[Sean McQuillan [G]]

Took a look at the code, looks good!

[Lilly]

@Sean McQuillan [G] Thank you so much for your help. Really appreciated! 🙂 ❤️

[Lilly]

So in some scenarios, it wouldn't be a problem to hoist the state to the level below the navigation-composable, that says the composable where I define the Scaffold for instance?

[Sean McQuillan [G]]

Yea – it seems like that will be pretty common thinking about it

[Sean McQuillan [G]]

The big thing to avoid is reading that state in composition and triggering navigation events from inside the execution of recomposition. As long as the navigation events are called by event handlers it'll work great

[Lilly]

ok got it. Just 2 things I don't understand fully. What do you mean with:

The state->event trampoline always introduces one recomposition delay to trigger the event.

and what skipping do you mean here?

Iff you profiled and found that this state was a trigger for unacceptable levels of work after skipping there's a few solutions

[Sean McQuillan [G]]

1: When you read a state from the execution of a composable, that happens the next time recomposition is scheduled (the recomposition was scheduled by the state change). This scheduling is roughly coordinated with frame times so it tends to introduce 1frame delays.

2. The compose compiler and runtime skip the execution of all composables they can prove can't have changed since the last recomposition due to their inputs

[Lilly]

@Sean McQuillan [G] Thanks for your answer