Exposing unique subscription handling for custom flow operations

[podarsmarty]

Exposing unique subscription handling for custom flow operations so that users can write customized subscriptions, for example, one to drop the first element in the flow before continuing:

viewModel.stateFlow
           .drop(1)
           .collectLatest(deliveryMode = UniqueOnly("onCreate_Custom")) { state ->
               ...
         }

Also includes a fix for FragmentViewBindingDelegate which can try to observe on a background thread causing a crash. (reproed in sample app launcher)

[gpeal]

@podarsmarty Can you do this manually like:

var isFirst = true
viewMode.onEach(MyState::myProp) { myprop ->
    if (isFirst) {
        isFirst = false
        return@onEach
    }
}

[elihart]

Yes, that should work @gpeal but we are seeing this need in multiple places and it would be very nice to have a built in way to accomplish it. Unfortunately it is not easy to provide our own internal extensions for this behavior due to how many forEach overloads there are.

I think an alternative approach would be to leverage the state flow and have an extension function we apply there, but the "unique only" logic is internal to Mavericks and available to apply to state flow operators.

[gpeal]

@elihart out of curiosity, what is the use case?

[podarsmarty]

@gpeal the use case I was running into is say the state stores a map. the key/values are what you want to use to populate your query params to the server. If any of them change you want to do a refresh of the page

[gpeal]

@elihart @podarsmarty What about something like:

onViewCreated() {
    viewLifecycleOwner.lifecycleScope.launchRepeatingJob(ON_START) {
        viewModel.stateFlow
            .map { it.foo }
            .distinctUntilChanged()
            .drop(1)
            .collect(::something)
        }
    }
}

Would that work? You could make a helper if your base fragment or something that did that too

[elihart]

I don't think that works because it does not prevent the redelivery of the last value on view recreation, in order to do that the logic in resolveSubscription needs to be exposed, which is what I was referencing above https://github.com/airbnb/mavericks/pull/560#discussion_r662636356

However, @podarsmarty if your use case is to make a network request with query params change shouldn't the subscription be in the ViewModel, not the Fragment? In that case there is no resubscription needed.

[podarsmarty]

@elihart hmm yea that should work if we were only doing the refresh. we are doing a couple things to update things like the toolbar. it is still possible without this but it becomes pretty messy since we will need to subscribe to the same thing.

I think it would also break down if the fragment had 2 view models so that it could share some stuff with a diff screen. ie - viewmodel a has some property that would trigger a refresh in viewmodel b.

@gpeal is the concern b/c we dont want to have a growing list of delivery modes?

[gpeal]

For separate subscriptions for data refreshing in the ViewModel and updating the toolbar in the Fragment, that seems perfectly reasonable to me and I agree with Eli's sentiment about the data refreshing belonging int he ViewModel.

For 2 ViewModels in one fragment, at Tonal, we used to do this and encountered exactly that issue. Now, we utilize Dagger/DI for any shared data so that all data wiring happens in your object graph or view models. Coupling data wiring in Fragments makes things harder to tests and couples functionality to UI more tightly than you usually want.

Yeah, I think one of the biggest benefits of Mavericks is that it has a small API surface area so we need to be really careful about adding anything that isn't necessary or significantly increases the usability of the library.

[podarsmarty]

@gpeal hmm sounds like we prob want to figure out the API @elihart mentioned now rather then later then?

A "Custom" delivery mode that takes a lambda that acts on a flow would work. We would just have to expose lastDeliveredValue though

[podarsmarty]

@gpeal @elihart, aight i went through and updated this so that it is just a singular custom delivery mode. This way people can get the behavior of unique updates only or customize it to suit their needs without adding another delivery mode

[podarsmarty]

@elihart hmm i think that would work. in that case would we even want to keep Delivery? Cause then we'd have Redeliver & Unique via delivery mode and anything custom via a stateflow directly. im guessing wed just want to remove delivery mode in that case and folks would use the uniqueOnly extension on the flow?

[podarsmarty]

collectLatest conflicts with an extension from coroutines stdlib. Both extensions can be called as flow.collectLatest { }, so if you're calling collectLatest from suspending context, it's unclear which one is called.

open to suggestions for the fun name. had a hard time coming up with one

[denis-bezrukov]

open to suggestions for the fun name. had a hard time coming up with one

yep, the best I can think now is consumeLatest { }

[elihart]

I think the name should be left as collectLatest but remove the default value for deliveryMode so that there is no conflict with function resolution. Using the same name will be easier for understanding behavior and discoverability.

Also agree it should be able to act on nullable values