Alter viewmodel-sample to show how to use a not always hot StateFlow

[StylianosGakis]

Turn the StateFlow in MoleculeViewModel into one created using stateIn so that it can turn cold when there are no observers.

This PR is an effort to address #273

Uses the ideas from here #271 (comment) in order to solve the issue with when the ViewModel comes back from the backstack to still have the last emitted value available to it

[tunjid]

LGTM from a ViewModel POV, deferring to molecule maintainers for if its still idiomatic molecule from their POV.

Looks like you may need to update some tests.

[StylianosGakis]

Tests are fixed, thanks a lot for the help on this Tunji!

[jingibus]

This is a step up over the existing example, but seed here really does sell me on the merits of having a started parameter for launchMolecule, or some other solution that provides similar capabilties

[jingibus]

This seed thing sure is awkward. There "should" be no need for it — as a practical matter, moleculeFlow will emit immediately. But obviously that is not something a Flow does...

[tunjid]

The need for it mostly stems from restarting. Consider a timer that counts how much time is spent on a screen doing an activity, like the NYT crossword. It has the following presenter Composable:

@Composable
fun TimerPresenter(seed: Long): Long {
    var elapsed by remember { mutableStateOf(seed) }

    LaunchedEffect(Unit) {
        while(true) {
            delay(1.seconds)
            ++elapsed
       } 
    }
}

Without the seed, there is no way to restore elapsed to its last seen state in between presenter stop and restarts save for writing it to a data source. If the data source exposed a cold flow, the last seen value will immediately be overwritten:

@Composable
fun TimerPresenter(dataSource: Flow<Long>): Long {
    // There is no good default to use here upon resumption
    val savedElapsed by dataSource.collectAsState(0)
    // elapsed will start back at 0
    var elapsed by remember { mutableStateOf(savedElapsed) }

    LaunchedEffect(Unit) {
        while(true) {
            delay(1.seconds)
            ++elapsed
       } 
    }

    // Assuming the data source is well behaved and only emits once,
    // add the last time elapsed to the current one
    LaunchedEffect(savedElapsed) {
        elapsed += savedElapsed
    }
}

In the above, the UI will briefly flash 0 when the screen is returned to after state production is resumed after being stopped. The seed provides a way of "freezing" the last seen state for restoration when state production resumes as Compose Presenters unlike Flow require a synchronous (in as much as a Composable function can be deemed synchronous) initial return value.

[tunjid]

This isn't an issue with Flow operators like combine or merge because state won't be updated till all Flows emit in the case of the former, and till independent Flows emit in the latter. The immediate value requirement that Compose imposes seemingly mandates a seed value.

[Sripadmanabans]

I don't understand the TimerPresenter example. It seems to only emit one value.

LaunchedEffect(Unit) {
    delay(1000L)
    ++elapsed
}

Should it be inside a while (true) loop?

[tunjid]

Yes, it should. Thanks for pointing it out!

[Sripadmanabans]

If we do end up using a while (true) in the presenter and convert the molecule flow with stateIn and SharingStarted.WhileSubscribed(5.seconds) I think the behavior would be wrong. In the sense that we would be tracking an additional 5 seconds after the last collector has been removed.
Would it be better to use molecule to create a StateFlow that is always active and using events to start and stop the timer be a better way to keep track of screen time?

[tunjid]

In the particular case of this example, a no argument SharingStarted.WhileSubscribed() would have been used. The example is a bit trivial however, as other flows may also have been used in the presenter.

The larger point in context being, it would be nice if molecule presenters could be used to create a StateFlow that offers behaviors that match that in all permutations of flow.stateIn(...). Especially in the case where the Flow is stopped and restarted, without overwriting the last emitted value.

[jingibus]

The need for this initialValue does make the case that having a started parameter on launchMolecule is maybe not the worst idea in the world. Hooking it up with the existing API surface is uhh waves hands possible, for a certain definition of possible, but not one useful to someone who wants to do what MoleculeViewModel is doing

[StylianosGakis]

I'd personally be happy to see this merged as it is right now, and for any thoughts that may arise around seed and if anything could be done on the library side can be addressed in a separate PR, after considering the discussion here.

[JakeWharton]

This whole pattern really feels like it should be encapsulated in something.

I could see it being one (or more) of the following:

• A valve. These existed for RxJava and basically let you control the upstream subscription based on another stream. In this case, the lifecycle would be filtered and mapped to a true/false stream based on whether the activity was resumed and that would control the valve on the upstream collection.
• Replaying share. Another RxJava thing, where an observable instance would cache the latest seen value, replay it to new subscribers, and unsubscribe upstream when the subscriber count dropped to 0.
• A custom frame clock (or frame clock wrapper) which only ticks when the lifecycle is resumed. The flow collector would always be present, but instead of using active collection to control whether recomposition occurs we use the frame clock.

I legitimately cannot figure out what to do with this PR. The use case is certainly valid. But the amount of code required to accomplish it as-is seems like it would overwhelm someone trying to learn how to use Molecule with AndroidX View Model rather than guide them.

[JakeWharton]

(This is not really a reflection of your PR, more of the current library design, the design of View Model, and the general sadness of lifecycle.)

[StylianosGakis]

This is not really a reflection of your PR , more of the current library design, the design of View Model, and the general sadness of lifecycle.

Yes I understand and I agree. It really shouldn't be this way, but it is the current reality unfortunately.

The use case is certainly valid. But the amount of code required to accomplish it as-is seems like it would overwhelm someone trying to learn how to use Molecule with AndroidX View Model rather than guide them.

So my entire reasoning behind starting this PR came down to the steps we took to adopt molecule in the app I work for:

We were looking into adopting Molecule for our codebase for some time now.

1. A long time ago there was no viewmodel sample, and I looked briefly into it and I though "eh, it's probably gonna be hell to do in a ViewModel" so I dropped the idea.
2. Fast forward some time, the sample is there, and I look into it again. It all works using this tiny bit of glue code so I'm thinking let's go ahead and use it in this way exactly as it seems to work well.
3. I realize that the state flow is always hot, keeping potentially expensive flows hot on the backstack for all entries in the backstack which starts becoming a big deal. I pause this idea briefly
4. Then I tried to look for a way to make these two worlds play well together, hence this PR was made, where Tunji was very kind to help me through understanding how to work around ViewModel to make it happen.

Basically then, I fear that people who use AAC ViewModel will go up to step # 2 here, and then stop there without realizing there's more to be done. Then eventually when they figure out what happens, they'll blame Molecule instead of ViewModel since that's when this change would have been introduced to them.
So I thought if the sample played well with the lifecycle in the first place, all the people who would come here can see the easy and straightforward sample from the sample module here. And if they want a proper integration with ViewModel, they can look into the more involved sample-viewmodel module in this repo.

If we can do something to make this PR even better and make the code more understandable, perhaps from the points you suggest I'd love for that to happen of course. Other than that I personally still feel like it's a net positive if the sample which a lot of people are probably going to follow blindly is actually playing well with AAC ViewModel.

[jingibus]

@JakeWharton, if we consider LaunchedEffect this gets even gnarlier. Those are likely the pieces that you'd actually want to shut down — recomposition is designed not to have so many side effects.

On reflection, I think the happy places to land are likely to be:

• Include rememberSaveable support, then kill everything with a stateIn type solution, without any seed-type mechanism
• Just keep the molecule running, killing it when the ViewModel dies
• Include support for a rememberRetained, like Circuit has

The last two are most appealing to me. But even rememberRetained has weird consequences — IIRC one of the points of retention in ViewModel is to allow those long-running processes to keep going. The only approach we have that would achieve that would be to just keep it running.

And is keeping it running really so bad? How many would actually be kept running at one time in the VM framework? Is it just the VMs currently on screen during config change, or have I fallen off the boat here?

[StylianosGakis]

It's all the VMs in screens in the backstack, so potentially many of them.

[tunjid]

It's worth noting that a rememberSaveable solution would still be supsceptible to the overwrite on restart issue if the value to be restored is not parcelable.

The recommended period for state production to be halted in the absence observers with ViewModels is 5 seconds. If a screen is in the back stack for 5 seconds, it should still have access to it's last produced non parcelable state when resumed IMO. Otherwise, leaving a screen for > 5 seconds and returning to it will have the same behavior as the app cold starting on that screen.

In the case of a screen with a paginated list for example, this may mean a progress bar till the list is refreshed and the scroll position properly restored.

[jingibus]

It's worth noting that a rememberSaveable solution would still be supsceptible to the overwrite on restart issue if the value to be restored is not parcelable.

Yep. launchMolecule should be capable of supporting all of stateIn's parameters, which would fix that issue in a nice way.