Fixes popping when outgoing key references missing object.

`BackstackFrame` now holds on to both a `key` and a `@Composable` derived from the `content` function it was originally seen with. Previously during a pop we would try to display the outgoing `key` by passing it to the `content(T)` function passed with the updated `backstack` list.

Fixes #63

buildSrc/src/main/resources/versions.properties

@@ -1,3 +1,3 @@
 # -SNAPSHOT will automatically be appended. Pass -PisRelease=true to gradlew to release (this will
 # also append the current compose version number after a +).
-releaseVersion=0.10.0
+releaseVersion=0.11.0

compose-backstack-xray/src/main/java/com/zachklipp/compose/backstack/xray/XrayController.kt

@@ -41,7 +41,7 @@ private class XrayController<T : Any> : FrameController<T> {
   private var alpha by mutableStateOf(.4f)
   private var overlayAlpha by mutableStateOf(.2f)
 
-  private var activeKeys by mutableStateOf(emptyList<T>())
+  private var _activeFrames by mutableStateOf(emptyList<BackstackFrame<T>>())
 
   private val controlModifier = Modifier.pointerInput(Unit) {
     detectTransformGestures { _, pan, zoom, _ ->
@@ -54,16 +54,16 @@ private class XrayController<T : Any> : FrameController<T> {
   // Use derivedStateOf to cache the mapped list.
   override val activeFrames by derivedStateOf {
     if (!enabled) wrappedController.activeFrames else {
-      activeKeys.mapIndexed { index, key ->
-        val modifier = Modifier.modifierForFrame(index, activeKeys.size, 1f)
-        return@mapIndexed BackstackFrame(key, modifier)
+      _activeFrames.mapIndexed { index, frame ->
+        val modifier = Modifier.modifierForFrame(index, _activeFrames.size, 1f)
+        return@mapIndexed frame.copy(modifier = modifier)
       }
     }
   }
 
-  override fun updateBackstack(keys: List<T>) {
-    activeKeys = keys
-    wrappedController.updateBackstack(keys)
+  override fun updateBackstack(frames: List<BackstackFrame<T>>) {
+    _activeFrames = frames
+    wrappedController.updateBackstack(frames)
   }
 
   /**

compose-backstack/src/main/java/com/zachklipp/compose/backstack/Backstack.kt

@@ -12,6 +12,7 @@ import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.clip
 import androidx.compose.ui.graphics.RectangleShape
+import com.zachklipp.compose.backstack.FrameController.BackstackFrame
 import kotlin.DeprecationLevel.ERROR
 
 /**
@@ -106,22 +107,24 @@ fun <T : Any> Backstack(
   // However, we do need to give the controller the chance to initialize itself with the initial
   // stack before we ask for its activeFrames, so this is a lazy way to do both that and subsequent
   // updates.
-  frameController.updateBackstack(backstack)
+  frameController.updateBackstack(backstack.map {
+    BackstackFrame(it) { content(it) }
+  })
 
   // Actually draw the screens.
   Box(modifier = modifier.clip(RectangleShape)) {
     // The frame controller is in complete control of what we actually show. The activeFrames
     // property should be backed by a snapshot state object, so this will recompose automatically
     // if the controller changes its frames.
-    frameController.activeFrames.forEach { (item, frameControlModifier) ->
+    frameController.activeFrames.forEach { (item, frameControlModifier, frameContent) ->
       // Even if screens are moved around within the list, as long as they're invoked through the
       // exact same sequence of source locations from within this key lambda, they will keep their
       // state.
       key(item) {
         // This call must be inside the key(){} wrapper.
         stateHolder.SaveableStateProvider(item) {
           Box(frameControlModifier) {
-            content(item)
+            frameContent()
           }
         }
       }

compose-backstack/src/main/java/com/zachklipp/compose/backstack/FrameController.kt

@@ -1,5 +1,6 @@
 package com.zachklipp.compose.backstack
 
+import androidx.compose.runtime.Composable
 import androidx.compose.runtime.Immutable
 import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.Stable
@@ -43,18 +44,19 @@ interface FrameController<T : Any> {
    * or update any state that is not backed by snapshot state objects (such as [MutableState]s,
    * lists created by [mutableStateListOf], etc.).
    *
-   * @param keys The latest backstack passed to [Backstack]. Will always contain at least one
+   * @param frames The latest backstack passed to [Backstack]. Will always contain at least one
    * element.
    */
-  fun updateBackstack(keys: List<T>)
+  fun updateBackstack(frames: List<BackstackFrame<T>>)
 
   /**
    * A frame controlled by a [FrameController], to be shown by [Backstack].
    */
   @Immutable
   data class BackstackFrame<out T : Any>(
     val key: T,
-    val modifier: Modifier = Modifier
+    val modifier: Modifier = Modifier,
+    val content: @Composable () -> Unit
   )
 }
 
@@ -70,7 +72,7 @@ private object NoopFrameController : FrameController<Any> {
   override val activeFrames: List<BackstackFrame<Any>>
     get() = topFrame?.let { listOf(it) } ?: emptyList()
 
-  override fun updateBackstack(keys: List<Any>) {
-    topFrame = BackstackFrame(keys.last())
+  override fun updateBackstack(frames: List<BackstackFrame<Any>>) {
+    topFrame = frames.last()
   }
 }

compose-backstack/src/main/java/com/zachklipp/compose/backstack/TransitionController.kt

@@ -1,5 +1,6 @@
 package com.zachklipp.compose.backstack
 
+import android.annotation.SuppressLint
 import androidx.annotation.VisibleForTesting
 import androidx.annotation.VisibleForTesting.PRIVATE
 import androidx.compose.animation.core.Animatable
@@ -23,9 +24,11 @@ import com.zachklipp.compose.backstack.FrameController.BackstackFrame
 import com.zachklipp.compose.backstack.TransitionDirection.Backward
 import com.zachklipp.compose.backstack.TransitionDirection.Forward
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.collect
 
+typealias OnTransitionStarting<T> =
+    (from: List<BackstackFrame<T>>, to: List<BackstackFrame<T>>, TransitionDirection) -> Unit
+
 /**
  * Returns the default [AnimationSpec] used for [rememberTransitionController].
  */
@@ -48,7 +51,7 @@ import kotlinx.coroutines.flow.collect
 @Composable fun <T : Any> rememberTransitionController(
   transition: BackstackTransition = BackstackTransition.Slide,
   animationSpec: AnimationSpec<Float> = defaultBackstackAnimation(),
-  onTransitionStarting: (from: List<T>, to: List<T>, TransitionDirection) -> Unit = { _, _, _ -> },
+  onTransitionStarting: OnTransitionStarting<T> = { _, _, _ -> },
   onTransitionFinished: () -> Unit = {},
 ): FrameController<T> {
   val scope = rememberCoroutineScope()
@@ -86,8 +89,7 @@ internal class TransitionController<T : Any>(
 
   var transition: BackstackTransition? by mutableStateOf(null)
   var animationSpec: AnimationSpec<Float>? by mutableStateOf(null)
-  var onTransitionStarting: ((from: List<T>, to: List<T>, TransitionDirection) -> Unit)?
-      by mutableStateOf(null)
+  var onTransitionStarting: (OnTransitionStarting<T>)? by mutableStateOf(null)
   var onTransitionFinished: (() -> Unit)? by mutableStateOf(null)
 
   /**
@@ -97,10 +99,10 @@ internal class TransitionController<T : Any>(
    * a forwards or backwards animation. It's a [MutableState] because it is used to derive the value
    * for [activeFrames], and so it needs to be observable.
    */
-  private var displayedKeys: List<T> by mutableStateOf(emptyList())
+  private var displayedFrames: List<BackstackFrame<T>> by mutableStateOf(emptyList())
 
   /** The latest list of keys seen by [updateBackstack]. */
-  private var targetKeys by mutableStateOf(emptyList<T>())
+  private var targetFrames by mutableStateOf(emptyList<BackstackFrame<T>>())
 
   /**
    * Set to a non-null value only when actively animating between screens as the result of a call
@@ -116,7 +118,7 @@ internal class TransitionController<T : Any>(
       } else {
         listOf(transition.fromFrame, transition.toFrame)
       }
-    } ?: listOf(BackstackFrame(displayedKeys.last()))
+    } ?: listOf(displayedFrames.last())
   }
 
   /**
@@ -127,70 +129,74 @@ internal class TransitionController<T : Any>(
   suspend fun runTransitionAnimations() {
     // This flow handles backpressure by conflating: if targetKeys is changed multiple times while
     // an animation is running, we'll only get a single emission when it finishes.
-    snapshotFlow { targetKeys }.collect { targetKeys ->
-      if (displayedKeys.last() == targetKeys.last()) {
+    snapshotFlow { targetFrames }.collect { targetFrames ->
+      if (displayedFrames.last().key == targetFrames.last().key) {
         // The visible screen didn't change, so we don't need to animate, but we need to update our
         // active list for the next time we check for navigation direction.
-        displayedKeys = targetKeys
+        displayedFrames = targetFrames
         return@collect
       }
 
       // The top of the stack was changed, so animate to the new top.
-      animateTransition(fromKeys = displayedKeys, toKeys = targetKeys)
+      animateTransition(fromFrames = displayedFrames, toFrames = targetFrames)
     }
   }
 
-  override fun updateBackstack(keys: List<T>) {
+  override fun updateBackstack(frames: List<BackstackFrame<T>>) {
     // Always remember the latest stack, so if this call is happening during a transition we can
     // detect that when the transition finishes and start the next transition.
-    targetKeys = keys
+    targetFrames = frames
 
     // This is the first update, so we don't animate, and need to show the backstack as-is
     // immediately.
-    if (displayedKeys.isEmpty()) {
-      displayedKeys = keys
+    if (displayedFrames.isEmpty()) {
+      displayedFrames = frames
     }
   }
 
   /**
    * Called when [updateBackstack] gets a new backstack with a new top frame while idle, or after a
-   * transition if the [targetKeys]' top is not [displayedKeys]' top.
+   * transition if the [targetFrames]' top is not [displayedFrames]' top.
    */
-  @OptIn(ExperimentalCoroutinesApi::class)
-  private suspend fun animateTransition(fromKeys: List<T>, toKeys: List<T>) {
+  private suspend fun animateTransition(
+    fromFrames: List<BackstackFrame<T>>,
+    toFrames: List<BackstackFrame<T>>
+  ) {
     check(activeTransition == null) { "Can only start transitioning while idle." }
 
-    val fromKey = fromKeys.last()
-    val toKey = toKeys.last()
-    val popping = toKey in fromKeys
+    val fromFrame = fromFrames.last()
+    val toFrame = toFrames.last()
+    val popping = fromFrames.firstOrNull { it.key == toFrame.key } != null
     val progress = Animatable(0f)
 
     val fromVisibility = derivedStateOf { 1f - progress.value }
     val toVisibility = progress.asState()
 
     // Wrap modifier functions in each their own recompose scope so that if they read the visibility
     // (or any other state) directly, the modified node will actually be updated.
+    @SuppressLint("UnnecessaryComposedModifier")
     val fromModifier = Modifier.composed {
       with(transition!!) {
         modifierForScreen(fromVisibility, isTop = popping)
       }
     }
+    @SuppressLint("UnnecessaryComposedModifier")
     val toModifier = Modifier.composed {
       with(transition!!) {
         modifierForScreen(toVisibility, isTop = !popping)
       }
     }
 
     activeTransition = ActiveTransition(
-      fromFrame = BackstackFrame(fromKey, fromModifier),
-      toFrame = BackstackFrame(toKey, toModifier),
+      fromFrame = fromFrame.copy(modifier = fromModifier),
+      toFrame = toFrame.copy(modifier = toModifier),
       popping = popping
     )
 
-    val oldActiveKeys = displayedKeys
-    displayedKeys = targetKeys
+    val oldActiveFrames = displayedFrames
+    displayedFrames = targetFrames
 
-    onTransitionStarting!!(oldActiveKeys, displayedKeys, if (popping) Backward else Forward)
+    onTransitionStarting!!(oldActiveFrames, displayedFrames, if (popping) Backward else Forward)
     progress.animateTo(1f, animationSpec!!)
     activeTransition = null
     onTransitionFinished!!()