Invalid (?) screen object causes app crash

[noe]

When migrating from compose-backstack 0.6.0-alpha04 to 0.8.0+beta02, my app is crashing with the following FATAL EXCEPTION backtrace:

java.lang.IllegalArgumentException: Type of the key XXXX@179c61d is not supported. On Android you can only use types which can be stored inside the Bundle.
        at androidx.compose.runtime.saveable.SaveableStateHolderImpl.SaveableStateProvider(SaveableStateHolder.kt:78)
        at com.zachklipp.compose.backstack.BackstackKt.Backstack(Backstack.kt:135)
        ...

The key, redacted here as XXXX, refers to the screen object, which is a normal sealed class. It worked perfectly on the old compose-backstack version.

From the example in the readme file, I would say that the old way of expressing screens has not changed. Do screens have to meet certain requirements so that they can be stored inside the Bundle?

[noe]

As I am not familiar with saving state in the Bundle, I asked in the compose channel in the Kotlin slack:

I am getting an exception related to SaveableStateProvider that says java.lang.IllegalArgumentException: Type of the key XXXX is not supported. On Android you can only use types which can be stored inside the Bundle. . What does it mean that a type can be stored inside the Bundle? Here, XXXX is a mere sealed class, nothing exotic.

And got this response:

Bundle (and BaseBundle, the class it extends) has a bunch of specific types it supports - you'll note the putString, putParcelable, etc. methods on the class: https://developer.android.com/reference/android/os/Bundle One of the parameters rememberSaveable takes is a Saver that lets you translate your mere sealed class into something that can be persisted across process death/recreation in a Bundle: https://developer.android.com/reference/kotlin/androidx/compose/runtime/saveable/package-summary#remembersaveable

[zach-klippenstein]

This is a real regression in this library, caused by the migration to SaveableStateHolder. Previously we used the compound hash key as the state key for each screen, but now we're just passing key object directly. That's just an oversight on my part, shouldn't be too hard to fix and test. Compare https://github.com/zach-klippenstein/compose-backstack/blob/fa25d0072652b6de2d9e89d2a55b983d02776f17/compose-backstack/src/main/java/com/zachklipp/compose/backstack/ChildSavedStateRegistry.kt#L20 https://github.com/zach-klippenstein/compose-backstack/blob/fa25d0072652b6de2d9e89d2a55b983d02776f17/compose-backstack/src/main/java/com/zachklipp/compose/backstack/ChildSavedStateRegistry.kt#L63 to https://github.com/zach-klippenstein/compose-backstack/blob/06ec3cc4fe3a6e7fd99a8d18c5d6d03f3af1258d/compose-backstack/src/main/java/com/zachklipp/compose/backstack/Backstack.kt#L135.

[noe]

Having Parcelable screens also makes the issue go away without changes in compose-backstack.

Changing my screen types to Parcelable made me realize that I am using some heavyweight objects (part of my domain layer) as members of my screen objects, and this made it more difficult to make everything Parcelable.

I have read that using heavy parcelable classes is not a good idea in terms of performance. However, if compose-backstack gets back to storing the state key instead of the key object, I understand that the hypothetical performance problem of having heavy parcelable object would not happen, right?

Would having screen types (and forcing them to) implement Parcelable in compose-backstack provide any benefit?

[zach-klippenstein]

I don't think it would.

By using the compound hash key as the persistence key, that effectively means that as long as your keys are .equals equal when the composition is restored, your screen states will be restored as well. But the screen keys don't necessarily have to be serializable because it might make more sense to recreate them at runtime.

That said, if your screen keys are that heavy, maybe they should be storing IDs and the screens themselves should be resolving IDs via a repository or something - not something I think this library should enforce though, since that's more of a guideline than a rule.

[zach-klippenstein]

It took a little more code than I expected, I fixed this so that non-bundleable types can also be saved. This restores the original behavior, and means your backstack can contain whatever types you want.