Emit immediately by default

[BenTilbrook]

I don't agree with the current false default value for emitImmediately. I would argue most idiomatic reactive code using FlowBinding will want the initial value. In addition, RxBinding emits the initial value by default, and FlowBinding clearly advertises itself as its direct counterpart.

Obviously this is a behavior breaking change, but an important one to get right early.

[ychescale9]

I'm not against it.

Can you elaborate on why most idiomatic reactive code using FlowBinding will want the initial value?

[BenTilbrook]

I see the purpose of FlowBinding (and RxBinding) as a bridge from the imperative APIs of Android widgets to reactive. As such, they're supplying an API that models the behavior of the widget. A critical component of modelling that behavior is the initial state - the first emission. In a sense, they're analogous to a BehaviorRelay or StateFlow.

Typical reactive code will do something like:

val callback = onBackPressedDispatcher.addCallback(this) { 
    drawerLayout.closeDrawer(navigationView) 
}
drawerLayout.drawerStateChanges(Gravity.START, emitImmediately = true)
    .collect { callback.isEnabled = it }

Note how this code needs to know the initial state because otherwise callback won't be configured until the first change event. This pattern has the desirable trait of only having to configure callback.isEnabled in one place - in collect {}.

The alternative to the above is something like:

val callback = onBackPressedDispatcher.addCallback(this) { 
    drawerLayout.closeDrawer(navigationView) 
}
callback.isEnabled = drawerLayout.isDrawerOpen(Gravity.START)
drawerLayout.drawerStateChanges(Gravity.START, emitImmediately = false)
    .collect { callback.isEnabled = it }

This pattern configures callback.isEnabled twice, violating the DRY principle. It also confusingly mixes and matches imperative and reactive APIs when it could simply be using reactive.

As a result of this proposed change, I think naming conventions will have to also change. In this case drawerStateChanges would become drawerOpen, which RxBinding uses, and I think makes more sense in this paradigm.

[ychescale9]

a bridge from the imperative APIs of Android widgets to reactive. As such, they're supplying an API that models the behavior of the widget.

Callback APIs are also reactive. They just don't implement something like reactive-streams.

The default behavior of the original callback-based APIs for those widgets with an emitImmediately option is to not invoke the callback / listener when subscribed (e.g. fun ViewPager2.pageSelections(emitImmediately: Boolean = false): Flow<Int>). The ones which does invoke the callback immediately upon subscription do not have the emitImmediately option (e.g. fun ViewPager2.pageScrollStateChanges(): Flow<Int>), i.e. consumers will get the same behavior offered by the original callback-based APIs.

The intention was to respect the behavior of the framework / unbundled libraries by default, and offer useful behavior change where it makes sense (like your example, and with the bindings which emit state changes instead of an events) by offering an optional parameter through which you can opt into.

This example you provided makes lot of sense to have emitImmediately set to true. But I don't see this being true by default helps make things more "idiomatic" or "reactive" for consumers of FlowBinding in general.

The bottom line is FlowBinding is just a tool that turns callback-based APIs into Flow, and is not opinionated on how you use it. It's just a thin wrapper around the current generation of UI toolkits.

I'm definitely interested in getting more feedback / examples from you and hopefully some other users, as I don't personally use most of the bindings myself other than writing tests for them in this project.

@svenjacobs @R4md4c any thoughts on this?

[BenTilbrook]

The intention was to respect the behavior of the framework / unbundled libraries by default

That was the impression I got, and it's a noble goal in most circumstances.

As a heavy user of RxBinding I know for sure that 99.9% of my use cases utilized the initial value emission. Therefore the FlowBinding library value is diminished by having to declare emitImmediately as true, due to the extra noise.

Another point is that RxBinding emits the initial value by default, and I see FlowBinding as satisfying that exact same audience.

If you'd prefer that this library simply mirrors the semantics of the underlying callback APIs, I understand, but I think it's significantly more useful to model Android widgets' reactive behavior instead.

(Obviously none of this discussion applies to things like button clicks! They're events, not behavior)

[ychescale9]

Could you share a few more examples of this pattern? Do you also mediate these events / state changes into a ViewModel / presenter to trigger actions?

Again I’m not completely against changing this behavior before 1.0, just want to make sure that this is actually the default behavior most users want.

[BenTilbrook]

Presenting a message edit text with a char limit:

editText.textChanges(emitImmediately = true)
    .map { it.toString().length }
    .collect {
        remainingCharsTextView.text = "${MAX_LEN - it} characters remaining"
        button.isEnabled = it < MAX_LEN
    }

If I was mediating the state through a view model, I'd still want the initial value, because the view state could've been restored from saved instance state:

// Dispatch widget behavior via actions
// We want the initial value because the edit text may have been restored after a config change
editText.textChanges(emitImmediately = true)
    .collect {
        dispatch(TextAction(it))
    }

// Observe view state
viewState.map { it.text }
    .collect {
        editText.text = it
    }

[BenTilbrook]

Some discussion on RxBinding about initial values: JakeWharton/RxBinding#153

[ychescale9]

What if we exposed StateFlow for those state change bindings and force consumers to opt-out (skip(1)) ?

[BenTilbrook]

~I'm not sure using StateFlow would be wise, because consumers might assume they can freely access its value, but we would have no way of ensuring it was actually up-to-date. For example, if we DID supply a StateFlow, when would we assign its value? When would we stop updating it?~ Actually since it's an interface, maybe we could supply a subclass that proxies to the widget's getX() method.

As for opting-out using skip(1), I'm all for it otherwise.

[BenTilbrook]

Could also return a MutableStateFlow and complete the circle, if applicable for a given widget, such as a TextView.

[ychescale9]

Yeah I like how StateFlow explicitly communicates that it will emit the latest value on collection through the type system, but don’t like that it allows consumers to access value imperatively.

If we are to make a change, I’d prefer removing emitImmediately over making it true by default.

[BenTilbrook]

Yeah I like how StateFlow explicitly communicates that it will emit the latest value on collection through the type system, but don’t like that it allows consumers to access value imperatively.

StateFlows strong equality-based conflation property could also be at odds with some of the APIs being adapted. The values being delivered by those callbacks may not have such a property.

I’d prefer removing emitImmediately over making it true by default

Works for me.

[ychescale9]

Cool. Let’s wait and see if others have opinions to share.

[svenjacobs]

Good morning 😉

I see the benefits of providing an initial value by default in @BenTilbrook's examples where a related view must be properly initialized (button enabled/disabled etc.). However since there are bindings that can provide an initial value and others that can't (event-based bindings like button clicks), how is this distinction and different behaviour clear to the user?

I guess using StateFlow alone does not solve this problem. The user could accidentally apply skip(1) to a StateFlow of clicks() and wonder why the first button click is never handled 🤷‍♂️

How can we document this behaviour with code and the type system and not with comments or naming conventions? Like in the discussion of RxBinding, maybe we can provide a wrapper class InitialValueStateFlow that provides a skipInitialValue(): StateFlow<T> function for those Flows that supply an initial value?

[svenjacobs]

Just thought about StateFlow a bit more and I think I misunderstood you. Returning a StateFlow of an event-based binding like clicks() does not make any sense, right? As a consumer I'm not interested in a button click that might have happened a few seconds ago (since StateFlow conflates the latest value).

So your distinction between an initial-value binding and a non-initial-value binding is returning either StateFlow or Flow, correct? In this case I guess we don't need a wrapper class InitialValueStateFlow.

[ychescale9]

Yeah I think we are only concerned with the bindings that emit state changes, and specifically whether the current value should be emitted immediately upon subscription.

StateFlow has some additional behavior e.g. strong equality-based conflation @BenTilbrook mentioned, and the unwanted side-effect of allowing consumers to access the current value imperatively. So this is probably not what we want even for bindings that emit state changes.

We currently have 3 options:

1. Do nothing - respect the callback-based APIs provided by the widgets and provide an optional emitImmediately: Boolean = false for the state changes bindings
2. Make emitImmediately = true by default.
3. Follow RxBinding and emit the current value immediately on collection, forcing consumers to skip(1) if they don't want the initial emission and relying on documentation to communicate this behavior.

I'm open to the idea of introducing a custom wrapper class for improved type safety, but I can't think of an API that can consistently improve the UX without adding noise and overhead to the common / average use cases.

[R4md4c]

I don't have a strong opinion on this but I guess having emitImmediately = true as default makes sense and let the user decides if he wants to skip the initial value or not.

Regarding the API UX maybe wait until SharedFlow is available? and use a custom InitialValueSharedFlow since I don't think StateFlow is suitable for emitting UI events as it does an implicit distinctUntilChanged between values.

[ychescale9]

SharedFlow has val replayCache: List<T>. Does it make sense for consumers to have access to historical emissions?

[R4md4c]

For average user, I don't think so IMO. Then I guess just a normal InitialValueFlow would just do it, and in case the user opts for sharing he can do a shareIn on the binding.

[svenjacobs]

What about making emitImmediately = true the default and remove that parameter and distinguish between an initial-value flow with InitialValueFlow and a non initial-value flow with Flow as usual.

Implementation of InitialValueFlow could look like this

class InitialValueFlow<T>(private val flow: Flow<T>) : Flow<T> by flow {

    fun skipInitialValue(): Flow<T> = flow.drop(1)
}

or as an inline class

inline class InitialValueFlow<T>(private val flow: Flow<T>) : Flow<T> {

    fun skipInitialValue(): Flow<T> = flow.drop(1)

    @InternalCoroutinesApi
    override suspend fun collect(collector: FlowCollector<T>) = flow.collect(collector)
}

I still think representing that different behaviour through the type system is better than having a boolean parameter.

[ychescale9]

@svenjacobs I like it.

• skipInitialValue() is discoverable
• Users who prefer the RxBinding behavior are happy.
• The change is source-compatible for those weren't passing in emitImmediately explicitly. Those who were passing in emitImmediately = true would probably enjoy this breaking change.

Let's wait and see how others feel about this.

[ychescale9]

@afollestad @chris-horner do you have any thoughts on this 😃 ?

[chris-horner]

Having the flow emit immediately by default feels more natural to me too.

Thinking on it a bit, I agree that @svenjacobs's approach is probably the best bet. I always appreciated the explicit skipInitialValue() in RxBinding, and providing this behaviour through a type rather than a parameter reads a little nicer.

Thumbs up from me 👍

[ychescale9]

Ok let's go with InitialValueFlow with a skipInitialValue() for skipping initial emission.

Thanks @svenjacobs for the idea and everyone else for the feedback!

[ychescale9]

PR is ready 😃