From 78c9076281cc0e7c87162f2d72538b28ed692db7 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Mon, 23 Mar 2026 18:07:00 +0100 Subject: [PATCH] Fix `TransactionTooLargeExceptions` caused by Appyx (#6410) * Fix `TransactionTooLargeExceptions` caused by Appyx After a long debugging session, we discovered the code Appyx uses to clear the saved state of nodes that have been removed is not working because of a race condition, causing this saved state to grow indefinitely. To fix it, we need to wait until the node has been disposed, which will call `SaveableStateHolder.removeState` once, removing the associated `SaveableStateRegistry`, and *then* call `removeState` again when we detect the node has been removed from the navigation graph. Since these classes and APIs are private in Appyx, we had to copy and modify and use these copies. * Remove ktlint checks on `SafeChildrenTransitionScope.kt` * Don't count the new code for coverage --- build.gradle.kts | 3 + .../libraries/architecture/BaseFlowNode.kt | 10 +- .../appyx/SafeChildrenTransitionScope.kt | 267 ++++++++++++++++++ .../main/kotlin/extension/KoverExtension.kt | 2 + 4 files changed, 277 insertions(+), 5 deletions(-) create mode 100644 libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/appyx/SafeChildrenTransitionScope.kt diff --git a/build.gradle.kts b/build.gradle.kts index 7b0e672bcc..f699378d54 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -76,6 +76,9 @@ allprojects { filter { exclude { element -> element.file.path.contains(generatedPath) } exclude("io/element/android/tests/konsist/failures/**") + + // This file comes from another project and we want to keep it as close to the original as possible + exclude("**/SafeChildrenTransitionScope.kt") } } // Dependency check diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/BaseFlowNode.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/BaseFlowNode.kt index 0ec0c1debe..ce89e8a9d9 100644 --- a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/BaseFlowNode.kt +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/BaseFlowNode.kt @@ -16,7 +16,6 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.Stable import androidx.compose.ui.Modifier import com.bumble.appyx.core.children.ChildEntry -import com.bumble.appyx.core.composable.Children import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.navigation.model.combined.plus import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel @@ -27,6 +26,7 @@ import com.bumble.appyx.core.plugin.Plugin import com.bumble.appyx.navmodel.backstack.BackStack import com.bumble.appyx.navmodel.backstack.transitionhandler.rememberBackstackFader import com.bumble.appyx.navmodel.backstack.transitionhandler.rememberBackstackSlider +import io.element.android.libraries.architecture.appyx.SafeChildren import io.element.android.libraries.architecture.overlay.Overlay /** @@ -66,9 +66,9 @@ inline fun BaseFlowNode.BackstackView( transitionSpec = { spring(stiffness = Spring.StiffnessMediumLow) }, ), ) { - Children( - modifier = modifier, + SafeChildren( navModel = backstack, + modifier = modifier, transitionHandler = transitionHandler, ) } @@ -78,9 +78,9 @@ inline fun BaseFlowNode.OverlayView( modifier: Modifier = Modifier, transitionHandler: TransitionHandler = rememberBackstackFader(), ) { - Children( - modifier = modifier, + SafeChildren( navModel = overlay, + modifier = modifier, transitionHandler = transitionHandler, ) } diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/appyx/SafeChildrenTransitionScope.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/appyx/SafeChildrenTransitionScope.kt new file mode 100644 index 0000000000..00e49d7bee --- /dev/null +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/appyx/SafeChildrenTransitionScope.kt @@ -0,0 +1,267 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.libraries.architecture.appyx + +import android.annotation.SuppressLint +import androidx.compose.animation.ExperimentalSharedTransitionApi +import androidx.compose.animation.SharedTransitionLayout +import androidx.compose.foundation.layout.Box +import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.Immutable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.derivedStateOf +import androidx.compose.runtime.getValue +import androidx.compose.runtime.key +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveableStateHolder +import androidx.compose.runtime.setValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.onSizeChanged +import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.IntSize +import com.bumble.appyx.core.composable.Child +import com.bumble.appyx.core.composable.ChildRenderer +import com.bumble.appyx.core.composable.ChildTransitionScope +import com.bumble.appyx.core.navigation.NavKey +import com.bumble.appyx.core.navigation.NavModel +import com.bumble.appyx.core.navigation.transition.JumpToEndTransitionHandler +import com.bumble.appyx.core.navigation.transition.TransitionBounds +import com.bumble.appyx.core.navigation.transition.TransitionDescriptor +import com.bumble.appyx.core.navigation.transition.TransitionHandler +import com.bumble.appyx.core.navigation.transition.TransitionParams +import com.bumble.appyx.core.node.LocalMovableContentMap +import com.bumble.appyx.core.node.LocalNodeTargetVisibility +import com.bumble.appyx.core.node.LocalSharedElementScope +import com.bumble.appyx.core.node.ParentNode +import io.element.android.libraries.core.coroutine.withPreviousValue +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.map +import timber.log.Timber +import kotlin.reflect.KClass + +////////////////////////////////////////////////////////////////////////////////////////////////////////// +// All the components in this file come from Appyx, and they've been modified to fix an issue with +// the saved state. The parts that are modified are marked. +////////////////////////////////////////////////////////////////////////////////////////////////////////// + +@Immutable +class SafeChildrenTransitionScope( + val transitionHandler: TransitionHandler, + val transitionParams: TransitionParams, + val navModel: NavModel +) { + + @Composable + inline fun ParentNode.children( + noinline block: @Composable ChildTransitionScope.( + child: ChildRenderer, + transitionDescriptor: TransitionDescriptor + ) -> Unit, + ) { + safeChildren(V::class, block) + } + + @Composable + inline fun ParentNode.children( + noinline block: @Composable ChildTransitionScope.(child: ChildRenderer) -> Unit, + ) { + safeChildren(V::class, block) + } + + @Composable + @SuppressLint("ComposableNaming") + fun ParentNode.safeChildren( + clazz: KClass, + block: @Composable ChildTransitionScope.(ChildRenderer) -> Unit, + ) { + _safeChildren(clazz) { scope, child, _ -> + scope.block(child) + } + } + + @Composable + @SuppressLint("ComposableNaming") + fun ParentNode.safeChildren( + clazz: KClass, + block: @Composable ChildTransitionScope.( + ChildRenderer, + TransitionDescriptor + ) -> Unit, + ) { + _safeChildren(clazz) { scope, child, descriptor -> + scope.block( + child, + descriptor, + ) + } + } + + @SuppressLint("ComposableNaming") + @Composable + private fun ParentNode._safeChildren( + clazz: KClass, + block: @Composable ( + transitionScope: ChildTransitionScope, + child: ChildRenderer, + transitionDescriptor: TransitionDescriptor + ) -> Unit + ) { + val saveableStateHolder = rememberSaveableStateHolder() + + val disposedNavKeys = remember { mutableSetOf>() } + + LaunchedEffect(navModel) { + navModel + .removedElementKeys() + .map { list -> + list.filter { clazz.isInstance(it.navTarget) } + } + ////////// MODIFIED //////////// + .filter { it.isNotEmpty() } + .collect { deletedKeys -> + deletedKeys.forEach { navKey -> + // Wait for the NavKey to be disposed before removing its key from saveableStateHolder: + // Otherwise, the child SaveableStateRegistry will be removed but not the `SavedState`, which will accumulate + // and may cause TransactionTooLargeExceptions + while (!disposedNavKeys.contains(navKey)) { + delay(10) + } + disposedNavKeys.remove(navKey) + Timber.v("Removed NavKey ${navKey} from saveableStateHolder. NavTarget: ${navKey.navTarget}") + saveableStateHolder.removeState(navKey) + } + } + ////////// END OF MODIFIED //////////// + } + + val screenStateFlow = remember { + this@SafeChildrenTransitionScope + .navModel + .screenState + } + + val children by screenStateFlow.collectAsState() + + children + .onScreen + .filter { clazz.isInstance(it.key.navTarget) } + .forEach { navElement -> + key(navElement.key.id) { + CompositionLocalProvider( + LocalNodeTargetVisibility provides + children.onScreenWithVisibleTargetState.contains(navElement) + ) { + Child( + navElement, + saveableStateHolder, + transitionParams, + transitionHandler, + block + ) + + ////////// MODIFIED //////////// + DisposableEffect(navElement.key) { + onDispose { + Timber.v("Disposed NavKey ${navElement.key}. NavTarget: ${navElement.key.navTarget}") + disposedNavKeys.add(navElement.key) + } + } + ////////// END OF MODIFIED //////////// + } + } + } + } +} + +@OptIn(ExperimentalSharedTransitionApi::class) +@Composable +inline fun ParentNode.SafeChildren( + navModel: NavModel, + modifier: Modifier = Modifier, + transitionHandler: TransitionHandler = remember { JumpToEndTransitionHandler() }, + withSharedElementTransition: Boolean = false, + withMovableContent: Boolean = false, + noinline block: @Composable SafeChildrenTransitionScope.() -> Unit = { + children { child -> + child() + } + } +) { + val density = LocalDensity.current.density + var transitionBounds by remember { mutableStateOf(IntSize(0, 0)) } + val transitionParams by remember(transitionBounds) { + derivedStateOf { + TransitionParams( + bounds = TransitionBounds( + width = Dp(transitionBounds.width / density), + height = Dp(transitionBounds.height / density) + ) + ) + } + } + if (withSharedElementTransition) { + SharedTransitionLayout(modifier = modifier + .onSizeChanged { + transitionBounds = it + } + ) { + CompositionLocalProvider( + /** LocalSharedElementScope will be consumed by children UI to apply shareElement modifier */ + LocalSharedElementScope provides this, + LocalMovableContentMap provides if (withMovableContent) mutableMapOf() else null + ) { + block( + SafeChildrenTransitionScope( + transitionHandler = transitionHandler, + transitionParams = transitionParams, + navModel = navModel + ) + ) + } + } + } else { + Box(modifier = modifier + .onSizeChanged { + transitionBounds = it + } + ) { + CompositionLocalProvider( + /** If sharedElement is not supported for this Node - provide null otherwise children + * can consume ascendant's LocalSharedElementScope */ + LocalSharedElementScope provides null, + LocalMovableContentMap provides if (withMovableContent) mutableMapOf() else null + ) { + block( + SafeChildrenTransitionScope( + transitionHandler = transitionHandler, + transitionParams = transitionParams, + navModel = navModel + ) + ) + } + } + } +} + +internal fun NavModel.removedElementKeys(): Flow>> { + return this.elements.withPreviousValue() + .map { (previous, current) -> + val previousKeys = previous?.map { it.key }.orEmpty() + val currentKeys = current.map { it.key } + previousKeys.filter { element -> + !currentKeys.contains(element) + } + } +} diff --git a/plugins/src/main/kotlin/extension/KoverExtension.kt b/plugins/src/main/kotlin/extension/KoverExtension.kt index fe73157450..27e44e31b9 100644 --- a/plugins/src/main/kotlin/extension/KoverExtension.kt +++ b/plugins/src/main/kotlin/extension/KoverExtension.kt @@ -123,6 +123,8 @@ fun Project.setupKover() { "io.element.android.libraries.designsystem.theme.components.bottomsheet.*", // Konsist code to make test fails "io.element.android.tests.konsist.failures", + // Copied from Appyx + "io.element.android.libraries.architecture.appyx.SafeChildrenTransitionScope", ) annotatedBy( "androidx.compose.ui.tooling.preview.Preview",