Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compose state molecule function #89

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

takahirom
Copy link

Compose State can be used in this PR of Compose when creating a UiModel with ViewModel.
It can be used without StateFlow.collectAsState() when the user's UI is Compose.
This change is just a suggestion, I have not written any tests yet. I would first like to get your opinion on what you think about this change.

Why is it necessary to use this function instead of using the Composable function to return State?

We need to keep the State alive through Configuration change by ViewModel.

@JakeWharton
Copy link
Member

Is this meaningfully different than holding a StateFlow? I remember us talking about it and we couldn't come up with reasons that you would explicitly need a State over StateFlow.

@takahirom
Copy link
Author

All I can think of is that you can automatically subscribe when you use it in Compose(As you know, it will not be subscribed if you use StateFlow.value.). Google had a tweet recommending using Compose State in ViewModel and this was discussed once.
https://twitter.com/adamwp/status/1487165193368915968
However, to be honest, the lack of this API is not really a problem, and if there seems to be no reason why this implementation should not be used, it is not a problem because the application can have its own implementation.

@JakeWharton
Copy link
Member

Yeah I'm not really opposed to adding this API. Like I said, we talked about exposing a State-producing factory very early on. I just want to understand its relationship to the others we expose and why one might choose one over the other.

@takahirom
Copy link
Author

In my use case, I was still only thinking about UiModel in ViewModel, but it may be possible to use Compose's State in the entire app. For example, it may be easier to use Compose than Flow's zip. In that case, it would be simpler to not use Flow as a return value for the entire application.
It might be fun to create an Experimental API to enable such a thing and see how it progresses. However, I am not sure if this library should be used for this purpose.

@saket
Copy link

saket commented Jul 27, 2022

Is the benefit of this PR only that it omits an extra call to collectAsState() on a StateFlow returned by launchMolecule()?

@JakeWharton
Copy link
Member

Matt and I talked about this a long time ago. I think the summary is that if you can hand a State directly to Compose UI then why not just expose it as a composable function that returns T instead? The nice thing about Flow and StateFlow is that you can consume them at a different layer than Compose UI. And it's true you can do that with State (via snapshotFlow), but if you do that then why not just produce a Flow or StateFlow directly? Can someone convince me otherwise?

@takahirom
Copy link
Author

Thanks.
Thinking back, I think the only thing I can do with this PR is to be able to delete the collectAsState(). Sorry for the confusion.
It may be easier to understand this PR if you divide it into two patterns.

Usage pattern in ViewModel
By using the function of this PR in ViewModel, we can make it survive through Configuration change, and we can delete collectAsState() with the Composable function.

Other Usage Patterns
Normally, I have not heard of many patterns where you want to start a process with a scope such as Repository or UseCase and keep the values. So I think Jake is right, just return T as a Composable function.

@efemoney
Copy link

efemoney commented Feb 3, 2024

I think the summary is that if you can hand a State directly to Compose UI then why not just expose it as a composable function that returns T instead? ... Can someone convince me otherwise?

I mean yeah if you want to run your molecule in the UI layer composition (meaning it inherently has the lifecycle of the UI). If you instead prefer to run molecule in something with a longer lifecycle, then you need to have a holder like StateFlow or snapshot State.

The difference between the two, for me specifically, boils down to their difference in API. The StateFlow API is clunky to use compared to snapshot State. You need to expose your data type inside a container type and of course call collect { ... } at the use site vs just "normally" accessing your State.
Mapping of one state to another is not supported by default by StateFlow (you gotta write contraptions like DerivedStateFlow keeping in mind StateFlow is not stable for inheritance so all bets are off) versus being able to "map" a State read just by a delegated method/property or via derivedStateOf { ... }.

The tradeoff for the "fluid" API however is the setup you need to have to "listen" to snapshot state. But if the tailend of all your state reads is in a composable (whether UI or Molecule) then its, subjectively, better to expose APIs driven by snapshot State.

@JakeWharton
Copy link
Member

I think your argument overstates the problems with StateFlow, as a call to by stateFlow.collectAsState() gives you the value just the same as by state inside the Compose UI composition.

The problem of mapping a StateFlow shouldn't apply here as you should either be doing it within the Molecule composition or the Compose UI composition on direct values.

The risk of adding this is very low. So we can just add it the next time I'm at a computer. But I also think its utility remains pretty low as well.

@JakeWharton
Copy link
Member

Will need some tests. If you want to add them feel free. Otherwise I'll get to it sometime this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants