Hi all, id like a sanity check on an architectural...
# compose
a
Hi all, id like a sanity check on an architectural decision our team has made with structuring compose. Code in replies
Copy code
fun Screen(
    viewModel = hiltViewModel(),
    y: () -> Unit,
    y: () -> Unit,
    y: (y) -> Unit
) {
    val scaffoldState = rememberScaffoldState()
    XTheme {
        XScaffold(
            scaffoldState = scaffoldState,
            content = {
                with(viewModel.uiState) {
                    Content(
                        x = x,
                        y = y,
                        [etc],
                        onCLick = viewModel::func,
                        [etc]
                    )
                }
anonymised for professional reasons. the pattern here is we use the Screen class to decouple the uiState from the Content, which is where the bulk of the ui exists
so the content can be previewed and exist without any reference to business logic. my worry is about how this affects recomposition and thuis performance. if the Content is recomposing any time there is a change in the uiState, does that have significant implications for Composables it contains?
r
It shouldn't affect performance. If you had all the inner composables you have on Content, here, same recompositions would happen.
If the state updates multiple times a second like on an animation maybe, then you might want to pass the actual State<T> down instead of the T. This would deferr reads until last second. But for most screens this will not be needed.
Another detail, Theme shouldn't be needed I believe. If you're using compose navigation at least, you can put theme outside the NavHost call, and all screens should inherit this.
Same for Scaffold. You might not need scaffold on all screens if you put it outside of the NavHost. Although this will depend more on how and what you're doing with it of course 🙂
Overall, its a good thing to do. Have a Screen Composable which is registered on NavHost and it gets the view model, etc. Then it calls a more "pure" composable which doesn't know anything about Android or viewmodels or navigation. It only receives state and lambdas.
a
@Rafael Costa thank you very much for your response and feedback 🙂
@Rafael Costa ive been thinking about the points you made, the Theme thing is definitely unnecessary as we do set it at a lower level, but we are building Compose on top of fragments (our app is very large and the majority of screens are still in xml. this is the function we use to set the content, which is called
onCreateView
Copy code
fun Fragment.setContent(
    onStart: () -> Unit = {},
    onStop: () -> Unit = {},
    key: Any = Any(),
    content: @Composable () -> Unit
): View =
    ComposeView(requireContext()).apply {
        setContent {
            Theme {
                DisposableEffect(key1 = key) {
                    onStart.invoke()
                    onDispose {
                        onStop.invoke()
                    }
                }
                content.invoke()
            }
        }
    }
the point is ive been investigating the correct way to implement scaffolds. i dont want them sitting in the screen composable unless necessary, but of course there may be little to do about it seeing as we dont use compose navigation
r
I see. Ok that makes sense then 🙂
Yeah, no.. you can forget about my points on Scaffold and Theme
you'd need to be using compose at the root so that all these get used on all screens. Since the NavHost would sit inside them
a
even then, might it not be an issue if some screens needed a BottomSheetScaffold specifically, for example? just trying to think it through since in the long run we will likely want to factor out the fragments altogether