Vsevolod Ganin

    Vsevolod Ganin

    1 year ago
    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?
    It gets more inconvenient with
    SnapshotStateList<MutableState>
    for list of mutable items
    Adam Powell

    Adam Powell

    1 year ago
    Yes, this is intentional behavior
    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

    Vsevolod Ganin

    1 year ago
    Yeah, no problem. Looks like this
    Adam Powell

    Adam Powell

    1 year ago
    thanks; this matches some of the common patterns I've seen elsewhere too
    Could you also post
    EditClickTrackScreenState
    ?
    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 remembered states and instead working directly with
    EditClickTrackScreenState
    Vsevolod Ganin

    Vsevolod Ganin

    1 year ago
    The relevant bits of state is like this
    Adam Powell

    Adam Powell

    1 year ago
    so the intent is that
    dispatch
    is meant to signal the calling code to generate a new
    state
    ?
    Vsevolod Ganin

    Vsevolod Ganin

    1 year ago
    Yes, with a side effect of storing it to database
    Adam Powell

    Adam Powell

    1 year ago
    figured 🙂
    then yes, I would remove the locally remembered state objects and the
    onCommit
    block here entirely
    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

    Vsevolod Ganin

    1 year ago
    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
    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

    Adam Powell

    1 year ago
    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
    you're right in that some usages of the
    @Composable animate
    APIs also encourage this; we're looking into that as well
    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
    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
    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.
    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) }
    }
    then you're not creating separate sources of truth, you're merely providing an interface to the one source of truth
    which then means the setter can directly handle the dispatch duties rather than using the onCommit trampoline
    Vsevolod Ganin

    Vsevolod Ganin

    1 year ago
    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
    And dealing with merging possible new changes when time consuming operation completes…
    Actually that’s not a big problem in redux since the merging in response to action is a thing 🙂
    Adam Powell

    Adam Powell

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