Shouldn’t `DrawerState` have a mutable `value`? Th...

# compose

[Vsevolod Ganin]

Shouldn’t DrawerState have a mutable value? The way things are working now forces to synchronize internal drawer state with external one (i.e. view model, app state). Wouldn’t it be more idiomatic to export callbacks of user intention to change drawer state so I could update state appropriately myself? Currently it looks like this

fun MyComposable(showDrawer: Boolean, dispatch: (Action) -> Unit) {
    val drawerState = rememberDrawerState(DrawerValue.Closed)
    LaunchedEffect(showDrawer) {
        if (showDrawer) drawerState.open() else drawerState.close()
    }

    // If user swipes or taps on free zone, drawerState.value will change
    LaunchedEffect(drawerState.value) {
        when (drawerState.value) {
            DrawerValue.Closed -> dispatch(CloseDrawer)
            DrawerValue.Open -> dispatch(OpenDrawer)
        }
    }
    ...
}

[Vsevolod Ganin]

So the given way of synchronizing states is ok? I may add additional checks to compare external (showDrawer) and internal state (drawerState) to avoid redundant calls to open / close / dispatch. But is it the best practice overall in such cases?

[jim]

@Vsevolod Ganin Sorry, no, you still seem to have multiple sources of truth. The problem is not with the declarative/imperative thing, but rather with the sources of truth. If the parent of MyComposable needs to control if the drawer is open or closed, they should be passing in a DrawerState instead of having it being owned by rememberDrawerState inside MyComposable. Your problem comes up because you did not hoist the state up out of the composable and so you have a source of truth problem.

[Vsevolod Ganin]

I don’t think I can hoist DrawerState enough. The problem is that my top state is data class (which is serializable for its reasons by the way) but DrawerState is not simple data and have connection with composition. I want my main source of truth to be as much compose-agnostic as possible. It seems that there is no choice but sync two sources of truth…

[jim]

No, syncing two sources of truth is almost always the wrong solution. Instead, consider the following:

class UiState {
  val appState: AppState = ...
  val drawerState = ...
}

Now you can pass down a single UI state, and still delegate to AppState as needed. Now you've avoided synchronization and have a single source of truth. You can still keep your app state somewhat detached from the state that exists to drive your UI.

[Vsevolod Ganin]

Do you mean to emit new UiState with new drawerState for every new AppState?

[jim]

Not if you don't want to lose the content of drawer state. It depends on how heavily you are leaning into immutable vs. how heavily you are leaning into mutableStateOf. Either way, you're going to need to retain some data somewhere, and you're going to need to update your data somewhere, and at some point that is likely going to involve `var`s.

If your AppState is mutable, then you just create your UiState once at the top of the activity and hold on to it (including the DrawerState) or if it is immutable, then perhaps the field on UiState is a var (probably backed by mutableStateOf) and you update it whenever your AppState value changes. Any of these approaches are fine, it's a matter of preference on your side, but neither one requires multiple sources of truth.

[jim]

Alternatively, if you are going heavily into immutable and want to create a new UiState each time, you could retain a reference to the drawer state and reuse that drawer state each time you create your UiState. It's totally up to you where you want to draw your lines between immutable and mutable data, but just be aware that DrawerState is mutable so you are going to need to retain it at some point.

[Vsevolod Ganin]

Sorry, I still don’t see how AppState should be tied up with drawerState. Let me describe a little example. Let’s say I have data class AppState(val showDrawer: Boolean). Let’s say that my state must have that property because it affects business logic. Then there should be a place where I should write something like if (state.showDrawer) drawerState.open() else drawerState.close(), right? This seems ok to me because AppState still dictates what should be drawn. The single source of truth is still maintained. However this breaks when drawerState is updated from the bottom of the chain - by user who swipes the drawer! At that moment there become two sources of truth - AppState.showDrawer and drawerState. If I had callback for user actions I could update AppState.showDrawer myself which then update drawerState transitively

[jim]

This seems ok to me because AppState still dictates what  should be drawn. The single source of truth is still maintained.

No, single source of truth is NOT maintained. This is evidenced by your very next sentence. If the derived state can be modified without going through your source of truth, then it is by definition not single-source-of-truth.

[jim]

Let’s say I have data class AppState(val showDrawer: Boolean). Let’s say that my state must have that property because it affects business logic.

I think you are subconsciously adding a constraint in your mind, that isn't actually dictated by your user journey. If I had to guess, I think you are somehow afraid that DrawerState is a compose-specific thing, and are thus afraid to include it in your data model upstream of your business logic. But DrawerState is mostly just a simple POJO and if you really didn't want to use it for some reason, you could write your own implementation. Importantly, it contains data about the state of your application - data which your business logic apparently needs to consume. You should find a way to hoist all the relevant data up above all the down stream consumers who need to use that data.

[Vsevolod Ganin]

There is a dependency on Compose library after all which is quite a burden for pure code/logic. Staying Compose agnostic helps me to isolate pure business logic which I can port to iOS app in the future, for example.

if you really didn’t want to use it for some reason, you could write your own implementation

You mean the implementation for ModalDrawer composable? 😄

By the way, is there any particular reason not to export callback of drawer intention to change its state? It would be very consistent with other standard composables I believe

[jim]

You mean the implementation for ModalDrawer composable?

No, I meant you could model the data (your source of truth) using your own data class of whatever kind you wanted), and just create an instance of DrawerState (you know, using the constructor and passing parameters) that reads and writes to your source of truth.

By the way, is there any particular reason not to export callback of drawer intention to change its state? It would be very consistent with other standard composables I believe

As a general rule of thumb, we separate the value and setter when we think it is likely people will want to override the setter, and we combine them into an object if we think there is a default operation that we believe is almost always the desired behavior. But when we do the later, we always try to provide a way for you to intercept the operation if you want to (by making the parameter an interface, or by allowing you to pass in constructor parameters to customize the behavior).

[Vsevolod Ganin]

just create an instance of DrawerState (you know, using the constructor and passing parameters) that reads and writes to your source of truth.

Oh, I didn’t realize I could force DrawerState to drop the change! Did you mean something like

@Composable
private fun drawerState(drawerScreenState: DrawerScreenState, dispatch: Dispatch): DrawerState {
    val initialDrawerValue = if (drawerScreenState.isOpened) DrawerValue.Open else DrawerValue.Closed
    return DrawerState(initialDrawerValue) { newDrawerValue ->
        when (newDrawerValue) {
            DrawerValue.Closed -> dispatch(CloseDrawer) // This updates `drawerScreenState`
            DrawerValue.Open -> dispatch(OpenDrawer) // This updates `drawerScreenState`
        }
        false // This tells DrawerState to reject the value
    }
}

In this snippet animations will not work - it will obviously snap to state each time state changes. The second variantion that fixes it will be

@Composable
private fun drawerState(drawerScreenState: DrawerScreenState, dispatch: Dispatch): DrawerState {
    val drawerValue = if (drawerScreenState.isOpened) DrawerValue.Open else DrawerValue.Closed
    return rememberDrawerState(drawerValue) { newDrawerValue ->
        when (newDrawerValue) {
            DrawerValue.Closed -> dispatch(CloseDrawer)
            DrawerValue.Open -> dispatch(OpenDrawer)
        }
        false
    }.apply {
        rememberCoroutineScope().launch {
            when (drawerValue) {
                DrawerValue.Closed -> close()
                DrawerValue.Open -> open()
            }
        }
    }
}

Is that what you had in mind?

[jim]

No, I think you are getting yourself confused by trying to incorporate rememberDrawerState and you are still getting source of truth problems. This is why I hate all the remember APIs, they send people down the wrong path and then people don't know how to dig themselves back out.

Maybe @Adam Powell will have ideas how to better phrase this, because I think he is always good at coming up with ways to describe things in understandable ways.

In your data model, perhaps you have something like:

class MyApplicationState {
  var isDrawerOpen = false
}

And in your composable:

val drawerState = DrawerState(applicationState.isDrawerOpen, {applicationState.requestDrawerOpen(it); return false;})

[Adam Powell]

Definitely don't CoroutineScope.launch in a @Composable function body. 🙂

[jim]

Yeah, that too.

[Adam Powell]

re. the rememberFooState APIs, yeah they're on that dodgy edge of Android's old ListActivity problem: a shortcut API that elides some boilerplate but has a shape that invites trying to use it long after your use case has outgrown it. We had a lot of debate over them and a general rule of thumb that everything they do must be implemented as public API so that someone may construct and aggregate the resulting state object together with others outside of composition.

[Adam Powell]

if I'm reading the rest of the thread right, I see two challenges: one, aggregating drawer state into a wider hoisted state object, and two, compressing the drawer state into a boolean true/false value

[Adam Powell]

that compression is lossy so long as you support any form of animation, and crossing a lossy state compression boundary always gets tricky. It's not always insurmountable, but you need to decide which side of that abstraction boundary knows what, and be consistent in both directions: state flowing down the composition and events flowing back up.

[Adam Powell]

The DrawerState object has full fidelity state; not just "open" or "closed" but the exact position of how open or closed, record of current animation or mutation in progress, etc.

[Vsevolod Ganin]

val drawerState = DrawerState(applicationState.isDrawerOpen, {applicationState.requestDrawerOpen(it); return false;})

This is equivalent to my first snippet (the one above “rememberCoroutineScope().launch” snippet), isn’t it? As I said, the animations will not work because the state will be new every time. What I want is to animate every change in hoisted state.

Definitely don’t CoroutineScope.launch in a @Composable function body.

Lol, that was silly. I changed it to LaunchedEffec(drawerValue), thanks.

Do you think the second snippet still has multiple sources of truth problem because of animation state inside DrawerState? That is probably what I want: compressed state in business logic and full fidelity state in UI which get synchronized only in one direction (from top to bottom, from state to UI). Effectively it’s implicit UiState which Jim proposed earlier in thread.