Do not create permanentNavModel inside ParentNode. Provide it via constructor to ParentNode

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## Pending changes
 
--
+- [#606](https://github.com/bumble-tech/appyx/pull/606) – **Breaking change**: Do not create permanentNavModel inside ParentNode. Provide it via constructor to ParentNode
 
 ---
 

libraries/core/src/androidTest/kotlin/com/bumble/appyx/core/node/PermanentChildTest.kt

@@ -11,29 +11,43 @@ import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.test.hasTestTag
 import com.bumble.appyx.core.AppyxTestScenario
 import com.bumble.appyx.core.children.nodeOrNull
+import com.bumble.appyx.core.composable.PermanentChild
 import com.bumble.appyx.core.modality.BuildContext
-import com.bumble.appyx.core.navigation.EmptyNavModel
+import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel
 import kotlinx.parcelize.Parcelize
 import org.junit.Assert.assertEquals
 import org.junit.Rule
 import org.junit.Test
 
 class PermanentChildTest {
 
+    var nodeFactory: (buildContext: BuildContext) -> TestParentNode = {
+        TestParentNode(buildContext = it)
+    }
+
     @get:Rule
     val rule = AppyxTestScenario { buildContext ->
-        TestParentNode(buildContext)
+        nodeFactory(buildContext)
     }
 
     @Test
-    fun permanent_child_is_rendered() {
+    fun `WHEN_permanent_model_contains_relevant_nav_key_THEN_permanent_child_is_rendered`() {
+        createPermanentNavModelWithNavKey()
         rule.start()
 
         rule.onNode(hasTestTag(TestParentNode.NavTarget::class.java.name)).assertExists()
     }
 
     @Test
-    fun permanent_child_is_reused_when_visibility_switched() {
+    fun `WHEN_permanent_model_does_not_contain_relevant_nav_key_THEN_permanent_child_is_not_rendered`() {
+        rule.start()
+
+        rule.onNode(hasTestTag(TestParentNode.NavTarget::class.java.name)).assertDoesNotExist()
+    }
+
+    @Test
+    fun `WHEN_visibility_switched_THEN_permanent_child_is_reused`() {
+        createPermanentNavModelWithNavKey()
         rule.start()
         rule.node.renderPermanentChild = false
         val childNodes = rule.node.children.value.values.map { it.nodeOrNull }
@@ -46,11 +60,27 @@ class PermanentChildTest {
         assertEquals(childNodes, rule.node.children.value.values.map { it.nodeOrNull })
     }
 
+    private fun createPermanentNavModelWithNavKey() {
+        nodeFactory = {
+            TestParentNode(
+                buildContext = it,
+                permanentNavModel = PermanentNavModel(
+                    TestParentNode.NavTarget,
+                    savedStateMap = null,
+                )
+            )
+        }
+
+    }
+
     class TestParentNode(
         buildContext: BuildContext,
+        private val permanentNavModel: PermanentNavModel<NavTarget> = PermanentNavModel(
+            savedStateMap = buildContext.savedStateMap
+        ),
     ) : ParentNode<TestParentNode.NavTarget>(
         buildContext = buildContext,
-        navModel = EmptyNavModel<NavTarget, Any>(),
+        navModel = permanentNavModel
     ) {
 
         @Parcelize
@@ -69,7 +99,7 @@ class PermanentChildTest {
         @Composable
         override fun View(modifier: Modifier) {
             if (renderPermanentChild) {
-                PermanentChild(NavTarget)
+                PermanentChild(permanentNavModel, NavTarget)
             }
         }
     }

libraries/core/src/main/kotlin/com/bumble/appyx/core/composable/PermanentChild.kt

@@ -0,0 +1,64 @@
+package com.bumble.appyx.core.composable
+
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.collectAsState
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.rememberCoroutineScope
+import androidx.compose.ui.Modifier
+import com.bumble.appyx.core.mapState
+import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel
+import com.bumble.appyx.core.node.Node
+import com.bumble.appyx.core.node.ParentNode
+import kotlinx.coroutines.flow.SharingStarted
+
+@Composable
+fun <NavTarget : Any> ParentNode<NavTarget>.PermanentChild(
+    permanentNavModel: PermanentNavModel<NavTarget>,
+    navTarget: NavTarget,
+    decorator: @Composable (child: ChildRenderer) -> Unit
+) {
+    val scope = rememberCoroutineScope()
+    val child by remember(navTarget, permanentNavModel) {
+        permanentNavModel
+            .elements
+            // use WhileSubscribed or Lazy otherwise desynchronisation issue
+            .mapState(scope, SharingStarted.WhileSubscribed()) { navElements ->
+                navElements
+                    .find { it.key.navTarget == navTarget }
+                    ?.let { childOrCreate(it.key) }
+            }
+    }.collectAsState()
+
+    child?.let {
+        decorator(PermanentChildRender(it.node))
+    }
+
+}
+
+@Composable
+fun <NavTarget : Any> ParentNode<NavTarget>.PermanentChild(
+    permanentNavModel: PermanentNavModel<NavTarget>,
+    navTarget: NavTarget,
+) {
+    PermanentChild(permanentNavModel, navTarget) { child -> child() }
+}
+
+private class PermanentChildRender(private val node: Node) : ChildRenderer {
+
+    @Suppress(
+        "ComposableNaming" // This wants to be 'Invoke' but that won't work with 'operator'.
+    )
+    @Composable
+    override operator fun invoke(modifier: Modifier) {
+        node.Compose(modifier)
+    }
+
+    @Suppress(
+        "ComposableNaming" // This wants to be 'Invoke' but that won't work with 'operator'.
+    )
+    @Composable
+    override operator fun invoke() {
+        invoke(modifier = Modifier)
+    }
+}

libraries/core/src/main/kotlin/com/bumble/appyx/core/node/ParentNode.kt

@@ -2,14 +2,7 @@ package com.bumble.appyx.core.node
 
 import androidx.annotation.CallSuper
 import androidx.annotation.VisibleForTesting
-import androidx.compose.runtime.Composable
-import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.Stable
-import androidx.compose.runtime.collectAsState
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.remember
-import androidx.compose.runtime.rememberCoroutineScope
-import androidx.compose.ui.Modifier
 import androidx.lifecycle.DefaultLifecycleObserver
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.LifecycleOwner
@@ -24,21 +17,15 @@ import com.bumble.appyx.core.children.ChildEntryMap
 import com.bumble.appyx.core.children.ChildNodeCreationManager
 import com.bumble.appyx.core.children.ChildrenCallback
 import com.bumble.appyx.core.children.nodeOrNull
-import com.bumble.appyx.core.composable.ChildRenderer
 import com.bumble.appyx.core.lifecycle.ChildNodeLifecycleManager
-import com.bumble.appyx.core.mapState
 import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.navigation.NavKey
 import com.bumble.appyx.core.navigation.NavModel
 import com.bumble.appyx.core.navigation.Resolver
 import com.bumble.appyx.core.navigation.isTransitioning
-import com.bumble.appyx.core.navigation.model.combined.plus
-import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel
-import com.bumble.appyx.core.navigation.model.permanent.operation.addUnique
 import com.bumble.appyx.core.plugin.Plugin
 import com.bumble.appyx.core.state.MutableSavedStateMap
 import kotlinx.coroutines.Job
-import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.suspendCancellableCoroutine
@@ -50,7 +37,7 @@ import kotlin.reflect.KClass
 @Suppress("TooManyFunctions")
 @Stable
 abstract class ParentNode<NavTarget : Any>(
-    navModel: NavModel<NavTarget, *>,
+    val navModel: NavModel<NavTarget, *>,
     buildContext: BuildContext,
     view: ParentNodeView<NavTarget> = EmptyParentNodeView(),
     childKeepMode: ChildEntry.KeepMode = Appyx.defaultChildKeepMode,
@@ -62,12 +49,6 @@ abstract class ParentNode<NavTarget : Any>(
     plugins = plugins + navModel + childAware
 ), Resolver<NavTarget> {
 
-    private val permanentNavModel = PermanentNavModel<NavTarget>(
-        savedStateMap = buildContext.savedStateMap,
-        key = KEY_PERMANENT_NAV_MODEL,
-    )
-    val navModel: NavModel<NavTarget, *> = permanentNavModel + navModel
-
     private val childNodeCreationManager = ChildNodeCreationManager<NavTarget>(
         savedStateMap = buildContext.savedStateMap,
         customisations = buildContext.customisations,
@@ -96,35 +77,6 @@ abstract class ParentNode<NavTarget : Any>(
     fun childOrCreate(navKey: NavKey<NavTarget>): ChildEntry.Initialized<NavTarget> =
         childNodeCreationManager.childOrCreate(navKey)
 
-    @Composable
-    fun PermanentChild(
-        navTarget: NavTarget,
-        decorator: @Composable (child: ChildRenderer) -> Unit
-    ) {
-        LaunchedEffect(navTarget) {
-            permanentNavModel.addUnique(navTarget)
-        }
-        val scope = rememberCoroutineScope()
-        val child by remember(navTarget) {
-            permanentNavModel
-                .elements
-                // use WhileSubscribed or Lazy otherwise desynchronisation issue
-                .mapState(scope, SharingStarted.WhileSubscribed()) { navElements ->
-                    navElements
-                        .find { it.key.navTarget == navTarget }
-                        ?.let { childOrCreate(it.key) }
-                }
-        }.collectAsState()
-        child?.let {
-            decorator(child = PermanentChildRender(it.node))
-        }
-    }
-
-    @Composable
-    fun PermanentChild(navTarget: NavTarget) {
-        PermanentChild(navTarget) { child -> child() }
-    }
-
     override fun updateLifecycleState(state: Lifecycle.State) {
         super.updateLifecycleState(state)
         childNodeLifecycleManager.propagateLifecycleToChildren(state)
@@ -225,8 +177,6 @@ abstract class ParentNode<NavTarget : Any>(
     @CallSuper
     override fun onSaveInstanceState(state: MutableSavedStateMap) {
         super.onSaveInstanceState(state)
-        // permanentNavModel is not provided as a plugin, store manually
-        permanentNavModel.saveInstanceState(state)
         childNodeCreationManager.saveChildrenState(state)
     }
 
@@ -266,26 +216,8 @@ abstract class ParentNode<NavTarget : Any>(
 
     companion object {
         const val ATTACH_WORKFLOW_SYNC_TIMEOUT = 5000L
-        const val KEY_PERMANENT_NAV_MODEL = "PermanentNavModel"
     }
 
-    private class PermanentChildRender(private val node: Node) : ChildRenderer {
-
-        @Suppress(
-            "ComposableNaming" // This wants to be 'Invoke' but that won't work with 'operator'.
-        )
-        @Composable
-        override operator fun invoke(modifier: Modifier) {
-            node.Compose(modifier)
-        }
 
-        @Suppress(
-            "ComposableNaming" // This wants to be 'Invoke' but that won't work with 'operator'.
-        )
-        @Composable
-        override operator fun invoke() {
-            invoke(modifier = Modifier)
-        }
-    }
 
 }