rjrjr / compose-backstack

Simple composable for rendering transitions between backstacks.
Other
501 stars 23 forks source link

Map-based backstack crashes when popping. #63

Open rjrjr opened 2 years ago

rjrjr commented 2 years ago

If you modify BackstackTransitionsTest.assertTransition to build its backstacks out of Map<Int, String>, like so, you crash with NoSuchElementException when popping. We're popping to a list that no longer includes the information to paint the outgoing screen, which is a pretty realistic situation.

    val firstBackstack = mapOf(1 to "one")
    val secondBackstack = mapOf(1 to "one", 2 to "two")
    var backstack by mutableStateOf(if (forward) firstBackstack else secondBackstack)
    compose.mainClock.autoAdvance = false
    compose.setContent {
      Backstack(
        backstack.keys.toList(),
        frameController = rememberTransitionController(
          animationSpec = animation,
          transition = transition
        )
      ) { BasicText(backstack.getValue(it)) }
    }
rjrjr commented 2 years ago

No matter how I try to put BackstackFrame in charge of both the key and the @Composable (T) -> Unit, I can't get away from this same crash, where the popped key is passed to the updated lambda, which is unable to interpret it.

Looking at the stack trace, no matter how I package things up, the composable lambda passed to the original Backstack() call is always invoked directly by the recomposer with the updated map from the later call. Kind of smells like the Backstack() function as implemented makes it impossible to properly hoist state? Something like that.

I think I can make it work by changing the API to encourage the back stack elements to be self sufficient -- make them implement an interface similar to Workflow's ComposableRendering. Note that the existing unit tests and even the BackstackViewerApp effectively make that assumption themselves. They're all built around List<String>, and the @Composable () -> Unit renders the String keys directly. They are never used to look something up in other state that can get out of sync.

I'm kind of sad about that because it'll make the API feel more OO and less Composesque, but I'm getting to the point where I don't believe it can be made to work any other way.

Basically:

fun interface WithContent {
  @Composable fun Content()
}

fun <T : WithContent> Backstack(
  backstack: List<T>,
  modifier: Modifier = Modifier,
  frameController: FrameController<T>
)