`onCommit` with input of some `SnapshotMutableState` doesn’t trigger its lambda when value changes....

# compose

[Vsevolod Ganin]

onCommit with input of some SnapshotMutableState doesn’t trigger its lambda when value changes. onCommit with SnapshotMutableState::value does. That’s more likely because SnapshotMutableState doesn’t implement and delegate equals to its value. Is this intentional behavior?

[Vsevolod Ganin]

It gets more inconvenient with SnapshotStateList<MutableState> for list of mutable items

[Adam Powell]

Yes, this is intentional behavior

[Adam Powell]

Can you post your onCommit code where you are trying to use it like this? We're reworking some of these runtime effect functions to be more use case driven and the data point would be useful

[Vsevolod Ganin]

Yeah, no problem. Looks like this

Untitled

[Adam Powell]

thanks; this matches some of the common patterns I've seen elsewhere too

[Adam Powell]

Could you also post EditClickTrackScreenState?

[Adam Powell]

initial thoughts on seeing this are that it looks to be creating multiple sources of truth and syncing them; this code might benefit from removing the `remember`ed states and instead working directly with EditClickTrackScreenState

[Vsevolod Ganin]

The relevant bits of state is like this

Untitled

[Adam Powell]

so the intent is that dispatch is meant to signal the calling code to generate a new state?

[Vsevolod Ganin]

Yes, with a side effect of storing it to database

[Vsevolod Ganin]

It’s a redux thing https://redux.js.org/api/store#dispatchaction

[Adam Powell]

figured 🙂

[Adam Powell]

then yes, I would remove the locally remembered state objects and the onCommit block here entirely

[Adam Powell]

and instead of passing MutableState objects to EditClickTrackScreenContent, pass value/callback pairs, or even some sort of interface that removes the desire to use onCommit as a state-observing dispatch trampoline

[Vsevolod Ganin]

Yeah I thought about that initially but considered this way to be kinda… boilerplaty 🙂 I wanted to replace such value/callback pair with some kind of Rx PublishSubject really. In that way I could pass only one argument for every piece of state that mutates separately instead of two. So I used MutableState and its elevated observer/observable capabilities only for that purpose

[Vsevolod Ganin]

Another point was that it seems I can’t get away with avoiding internal states completely. For example, I have a composable that manages some animations internally. It uses animatedFloat. When animation completes, my composable emits a new value computed from some animation offset. This can be considered as synchronization between two sources of truth as well

[Adam Powell]

this onCommit(myMutableState.value) thing is emerging as a pretty significant antipattern. It doesn't do what you think it does, for one; you will always get events a frame late to have an effect on the current composition, and it forces recomposition of `onCommit`'s caller when it's not necessary

[Adam Powell]

you're right in that some usages of the @Composable animate APIs also encourage this; we're looking into that as well

[Adam Powell]

state often forms a pipeline; the part of the snippets above that give me pause is that the three field states are locally editable; it's a short-circuit in the feedback loop with the caller

[Adam Powell]

deriving more specific state from more general state isn't necessarily an issue, for example deriving an animation state of 0f-1f based on a true/false input parameter

[Adam Powell]

in the snippet above there's also no opportunity for the caller/parent to exercise any say or validation of the content; the local state was copied from the parameter state on initial composition and from there on, the local remembered state is the source of truth. The events dispatched by the onCommit are change notifications, not change requests.

[Adam Powell]

if you really want this kind of interface that bundles the value/set-request pairs using MutableState<T> you could do something like this, I suppose:

inline fun <T> mutableState(
    crossinline get: () -> T,
    crossinline set: (T) -> Unit
): MutableState<T> = object : MutableState<T> {
    override var value: T
        get() = get()
        set(value) {
            set(value)
        }

    override fun component1(): T = value

    override fun component2(): (T) -> Unit = { set(it) }
}

[Adam Powell]

then you're not creating separate sources of truth, you're merely providing an interface to the one source of truth

[Adam Powell]

which then means the setter can directly handle the dispatch duties rather than using the onCommit trampoline

[Vsevolod Ganin]

Ok I think I get what you are saying. There is still a little problem left - a quickness of feedback. There also was an intention to display any changes as quickly as possible to user. I imagine that what I need to do in response to dispatch is to emit a new state quickly and then do more time consuming things (like writing to database) possibly emitting a new state after that

[Vsevolod Ganin]

And dealing with merging possible new changes when time consuming operation completes…

[Vsevolod Ganin]

Actually that’s not a big problem in redux since the merging in response to action is a thing 🙂

[Adam Powell]

Yeah if you're taking a redux approach, "pending application of this change" is a valid state for it to be in