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

Child gets recomposed more than once at startup in Compose Desktop #56

Closed
racka98 opened this issue Mar 23, 2022 · 10 comments · Fixed by #59
Closed

Child gets recomposed more than once at startup in Compose Desktop #56

racka98 opened this issue Mar 23, 2022 · 10 comments · Fixed by #59
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@racka98
Copy link

racka98 commented Mar 23, 2022

I have 2 problems at the moment.

  1. When you launch the app on Compose Desktop the initial configuration destination gets called more than once which causes the Children block to be called more than once causing multiple recompositions which can lead to a crash.

You can see here on the first screenshot that at the launch there are multiple calls to same thing (related to the Home Screen):
Screenshot 2022-03-23 090508

But after you navigate to something like a Settings Screen. You only see one call for each. Even when you pop back to the Home screen the home screen composable will be called only once this time:
Screenshot 2022-03-23 090709

I am using Decompose version: 0.5.2. Github link to the navigation and ui part of the app in question is this: https://github.com/Thinkrchive/Thinkrchive-Multiplatform/tree/main/desktopApp/src/jvmMain/kotlin/work/racka/thinkrchive/v2/desktop/ui

The navigation folder contains RootComponent and it's implementations.
The screen folder contains each screen composable with it component Interface and Implementation.

I followed the official guide for using compose-extensions.

  1. Navigating to my About screen fails.
    When I click the About button the event is not propagated from the comoposable to the component creation inside my NavHostComponent

What's weird is that every other call like for settingsScreen or donateScreen work as expected just the aboutScreen call that has issues.

Also using crossfade() or crossfadeScale() crashes the app or refuses any navigation calls.

@arkivanov
Copy link
Owner

Hello. I didn't deep dive into the issue yet. But from the first sight, we should never rely on how much time recompositions should happen.

Ideally components and their contexts should be created outside of Composables functions. E.g. the Root component should be created in the main function, before Compose application is called. See the example: https://github.com/JetBrains/compose-jb/blob/f1854a9dce4c38fce6101f0844a96ebd75ca2ad6/examples/todoapp/desktop/src/jvmMain/kotlin/example/todo/desktop/Main.kt#L30

If this is not possible for some reason, then there is remember function which remembers values between compositions.

The navigation should be always performed as side effects, never from the composition calls.

@arkivanov arkivanov added the question Further information is requested label Mar 23, 2022
@arkivanov
Copy link
Owner

I believe this should be mentioned in the documentation, I will add. Thanks for raising.

@arkivanov arkivanov added the documentation Improvements or additions to documentation label Mar 23, 2022
@racka98
Copy link
Author

racka98 commented Mar 23, 2022

Hello. I didn't deep dive into the issue yet. But from the first sight, we should never rely on how much time recompositions should happen.

Ideally components and their contexts should be created outside of Composables functions. E.g. the Root component should be created in the main function, before Compose application is called. See the example: JetBrains/compose-jb@f1854a9/examples/todoapp/desktop/src/jvmMain/kotlin/example/todo/desktop/Main.kt#L30

If this is not possible for some reason, then there is remember function which remembers values between compositions.

The navigation should be always performed as side effects, never from the composition calls.

This method seems to have stopped the crashes which were caused by my ViewModel being created more than once rapidly and causing SQLDelight to crash my app due to conflicting transactions.
Even though the first created Child is still being called more than once it no longer crashes or freezes the app like before.
Thanks for pointing that out.

But the navigation to About screen issue still persists. I don't know why my onAboutClicked() event is not propagating to the respective component inside RootComponent implementation despite other similar events being received. Is there something else I am missing?

Screen_Recording.6.mp4

@arkivanov
Copy link
Owner

Do you want to apply my suggestions? Components and contexts should not be created directly in composable functions. Either in the main app or in remember.

@racka98
Copy link
Author

racka98 commented Mar 23, 2022

I understand what you mean. All of my components are created outside of the composables just like in the documentations but my current issue is with the single click (onAboutClick) for navigation to one route (About Screen) that does not activate no matter what I try. Maybe I should open a new issue since it's not directly to recomposition. Thank you for your time.

I'll close this issue since I've already moved component creation outiside of the application compose function. Sorry for the confusion.

@racka98 racka98 closed this as completed Mar 23, 2022
@arkivanov arkivanov reopened this Mar 23, 2022
@arkivanov
Copy link
Owner

Let's keep this issue open for a while, I want to add this to the documentation.

As far I see by the link provided above, components are created inside Composable functions. E.g. in the App function.

@racka98
Copy link
Author

racka98 commented Mar 23, 2022

Already fixed that as per your recommendation. It's already moved to the main function that's why it not longer crashes like before.

See Main.kt in the new commit

@arkivanov
Copy link
Owner

Thanks, I will try to check the project today or tomorrow.

@arkivanov arkivanov added the investigate Something might be broken and needs to be investigated label Mar 23, 2022
@arkivanov
Copy link
Owner

@racka98 It seems like you forgot to add parentheses here. It should be onAboutClicked() not just onAboutClicked.

@arkivanov arkivanov removed the investigate Something might be broken and needs to be investigated label Mar 23, 2022
@racka98
Copy link
Author

racka98 commented Mar 24, 2022

Oh wow. Don't know how I missed that. Thanks for your timel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants