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

derivedStateOf doesn't work for MVIComposable #16

Closed
OKatrych opened this issue May 25, 2023 · 2 comments
Closed

derivedStateOf doesn't work for MVIComposable #16

OKatrych opened this issue May 25, 2023 · 2 comments
Assignees

Comments

@OKatrych
Copy link

When I want to use the derivedStateOf for my state, which comes from the MVIComposable, is not working. The reason is that the entire content composable is recomposed every time the state change.
derivedStateOf works only with the androidx.compose.runtime.State<S>, but in MVIComposable the state is already unwrapped.

Example of the issue:

MVIComposable(provider = viewModel) { state ->
    Column(modifier = modifier) {
        val shouldShowButton by remember {
            derivedStateOf {
                //This will be invoked only once, but should be every time the state changes
                state.shouldShowButton()
            }
        }
}

To fix it I created a slightly changed function that returns androidx.compose.runtime.State<S> and allows the developer to unwrap the state manually:

@Composable
fun <S : MVIState, I : MVIIntent, A : MVIAction, VM : MVIProvider<S, I, A>> MVIComposableWithState(
    provider: VM,
    lifecycleState: Lifecycle.State = Lifecycle.State.STARTED,
    content: @Composable ConsumerScope<I, A>.(state: State<S>) -> Unit,
) {
    val scope = rememberConsumerScope(provider, lifecycleState)

    // see [LifecycleOwner.subscribe] in :android for reasoning behind the dispatcher
    val state = provider.states.collectAsStateOnLifecycle(Dispatchers.Main.immediate, lifecycleState)

    content(scope, state)
}

And this allows me to modify my example:

MVIComposable(provider = viewModel) { wrappedState ->
    val state by wrappedState // Unwrap the state manually and don't recompose the entire content

    Column(modifier = modifier) {
        val shouldShowButton by remember {
            derivedStateOf {
                //This will be invoked every time the state changes -> as expected
                state.shouldShowButton()
            }
        }
}

I propose adding the method overload which will return the wrapped State<S> instead of just S.

@Nek-12 Nek-12 self-assigned this May 29, 2023
@Nek-12
Copy link
Member

Nek-12 commented May 29, 2023

Hello, I believe that you are trying to solve a problem that does not exist here.
The MVIComposable does not expose State for a reason - we want it to recompose on state changes. I believe that behavior is normal.

Because that composable is only recomposed when the state changes, that's essentially equal to using derivedStateOf, isn't it?

What you probably wanted to do here is create some kind of computed property of a state that would be evaluated when the state is copied or created. In this case, it's far more efficient to make it a property of your MVIState itself, because state creation can be easily offloaded to background threads (unlike derivedStateOf { } computation).

Example:

 data class DisplayingSignInForm(
        val email: Input = empty(),
        val password: Input = empty(),
        val isPasswordVisible: Boolean = false,
    ) : EmailSignInState {
    
        // evaluated every time
        val isInputValid get() = email.isValid && password.isValid
        // evaluated when state is created ✅
        // works because state is immutable
        val isInputValid = email.isValid && password.isValid

Does this solve your problem?

@OKatrych
Copy link
Author

Makes sense, sorry for the late reply.

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

No branches or pull requests

2 participants