Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Recomposition happens twice! #122

Closed
hadilq opened this issue Sep 22, 2022 · 5 comments
Closed

Recomposition happens twice! #122

hadilq opened this issue Sep 22, 2022 · 5 comments

Comments

@hadilq
Copy link

hadilq commented Sep 22, 2022

I just pushed source code of a sample here. Everything looks good, except that if you run the code, and click the buttons, you'll see in the Logcat that recomposition is happening twice, once because we launched an action, which is expected, but the other one appeared to happen because of changing of the return value of the nested compose, which is unexpected! I made a workaround for the "previous page" button by launching two actions in a row!

    sealed interface PopStack : MainAction {
        object Flip : PopStack
        object Flop : PopStack
    }

To clarify the problem, I created this commit, so just check out to the-infinit-loop-problem branch. In this branch, after clicking the "previous page" button, it'll call the MainPresenter in some kind of a loop until the stack becomes empty! How can I avoid that infinit loop? Pull requests are welcome. By the way, thanks guys for this awesome library.

@mattprecious
Copy link
Collaborator

It looks like you have a lot of side-effects that are not contained within effects. This is likely causing or contributing to the issue. Please take a look at Side-effects in Compose.

@hadilq
Copy link
Author

hadilq commented Sep 23, 2022

Thanks, but can you mention one of them? This sample doesn't have any IO call! No network or disk access, just the UI, which I cannot think of a way to launch a UI action as an affect! Just give me one example.

@mattprecious
Copy link
Collaborator

mattprecious commented Sep 23, 2022

I misunderstood what your code was doing. The particular thing I thought was a side effect is not, at least not directly.

I believe the issue here is that you're passing in "actions" as parameters to composable functions, which has a side-effect on the upstream code, causing recomposition. Your MainPresenter is being composed with the Flop action, which then returns an updated MainState, which then re-composes MainPresenter with the new state and the old Flop action, which then executes the action again.

Actions are transient and should be handled within callbacks or effects. Instead of accepting an action as a parameter, you would accept a Flow<Action> and then use a LaunchedEffect:

LaunchedEffect {
  actions.collect {
    when (action) {
      // ...
    }
  }
}

This isn't a comprehensive code snippet because you need to do a lot of restructuring of your project.

This is an issue with your code at the Compose level. Molecule isn't contributing to this.

@hadilq
Copy link
Author

hadilq commented Sep 23, 2022

Thank you. This is exactly what's implemented in the sample of Molecule in this repository. I want to follow these steps, but I have two problems!

  • I need to somehow map the actions flow and pass it to the nested composables, which I can make it work I think!
  • I need to call the nested composables in the LaunchEffect, which I have no idea how! In fact it looks like impossible!

do you have any idea?

@JakeWharton
Copy link
Member

I am going to convert this to a discussion because it seems like:

  1. It's not a Molecule bug. We have no control over recomposition.
  2. You're now asking about general Compose patterns which is not Molecule-specific (in fact, almost nothing about Molecule is specific to Molecule, thankfully).

@cashapp cashapp locked and limited conversation to collaborators Sep 23, 2022
@JakeWharton JakeWharton converted this issue into discussion #123 Sep 23, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants