State restoration with Transitions does not work correctly

[Tolriq]

Voyager 1.0.0 RC7

See code below.

Repro, scroll the list to bottom, then click hello 1 then scroll to bottom then click hello 1 then scroll to bottom. Go to Android home wait a couple of seconds then run adb shell am kill cafe.adriel.voyager.sample Restart the app and see that 99% of the time the state is not properly restored in at least one screen.

Video: https://github.com/adrielcafe/voyager/assets/932609/00dc2c1d-6813-4284-97f3-120c998f6678

Code: Clone current master then replace BasicNavigationActivity with the following

package cafe.adriel.voyager.sample.basicNavigation

import android.os.Bundle
import androidx.activity.ComponentActivity
import androidx.activity.compose.setContent
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.verticalScroll
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import cafe.adriel.voyager.androidx.AndroidScreenLifecycleOwner
import cafe.adriel.voyager.core.annotation.ExperimentalVoyagerApi
import cafe.adriel.voyager.core.lifecycle.LocalNavigatorScreenLifecycleProvider
import cafe.adriel.voyager.core.lifecycle.NavigatorScreenLifecycleProvider
import cafe.adriel.voyager.core.lifecycle.ScreenLifecycleOwner
import cafe.adriel.voyager.core.lifecycle.ScreenLifecycleProvider
import cafe.adriel.voyager.core.screen.Screen
import cafe.adriel.voyager.core.screen.ScreenKey
import cafe.adriel.voyager.core.screen.uniqueScreenKey
import cafe.adriel.voyager.navigator.LocalNavigator
import cafe.adriel.voyager.navigator.Navigator
import cafe.adriel.voyager.navigator.currentOrThrow
import cafe.adriel.voyager.transitions.SlideTransition

class BasicNavigationActivity : ComponentActivity() {
    @OptIn(ExperimentalVoyagerApi::class)
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            CompositionLocalProvider(
                LocalNavigatorScreenLifecycleProvider provides EmptyNavigatorScreenLifecycleProvider
            ) {
                Navigator(Screen1("1")) { navigator ->
                    SlideTransition(navigator) {
                        it.Content()
                    }
                }
            }
        }
    }
}

abstract class UniqueScreen : Screen, ScreenLifecycleProvider {
    override val key: ScreenKey = uniqueScreenKey
    override fun getLifecycleOwner(): ScreenLifecycleOwner = AndroidScreenLifecycleOwner.get(this)
}

class Screen1(private val name: String) : UniqueScreen() {
    @Composable
    override fun Content() {
        val navigator = LocalNavigator.currentOrThrow
        Column {
            Text(
                text = "Hello $name!",
                modifier = Modifier.clickable { navigator.push(Screen2(name)) }
            )

            val testData = remember {
                (1..200).map { "Example $it" }
            }
            Column(modifier = Modifier.verticalScroll(rememberScrollState())) {
                for(a in testData) {
                    Text(a)
                }
            }
        }
    }
}

class Screen2(private val name: String) : UniqueScreen() {
    @Composable
    override fun Content() {
        val navigator = LocalNavigator.currentOrThrow
        Column {
            Text(
                text = "Hello $name!",
                modifier = Modifier.clickable { navigator.push(Screen2(name+"3")) }
            )

            val testData = remember {
                (1..200).map { "Example $it" }
            }
            Column(modifier = Modifier.verticalScroll(rememberScrollState())) {
                for(a in testData) {
                    Text(a)
                }
            }
        }
    }
}

@OptIn(ExperimentalVoyagerApi::class)
internal object EmptyNavigatorScreenLifecycleProvider : NavigatorScreenLifecycleProvider {
    override fun provide(screen: Screen): List<ScreenLifecycleOwner> = emptyList()
}

[Tolriq]

@DevSrSouza bump. This will be an issue for shared transitions and predictive back.

[DevSrSouza]

I was able to reproduce with your code. I will start investigate

[DevSrSouza]

Our currently examples, the BasicNavigationScreen is a data class, apparently this why it works of the box.

Still investigating why this happens, but this fix for me:

abstract class UniqueScreen : Screen {
    override val key: ScreenKey = uniqueScreenKey

    override fun hashCode(): Int {
        return key.hashCode()
    }
}

Usage of LocalNavigatorScreenLifecycleProvider provides EmptyNavigatorScreenLifecycleProvider + override fun getLifecycleOwner(): ScreenLifecycleOwner = AndroidScreenLifecycleOwner.get(this) or not using this approach (using the default implementation) will work the same.

I will investigate why hashCode is required for the restoring the content of the Screen (the screen is being restaured with it parameters properly, just only the content with the usage of rememberSaveable that is not working without hashCode)

[DevSrSouza]

The documentation already suggests using data class: https://voyager.adriel.cafe/state-restoration#java-serializable

[Tolriq]

Hum thanks will test next week.

But no half the doc is without data class for the screens and the doc explains that the params needs to be serializable or parcelable and that the screen is already serializable. There no obvious reason or doc saying it should be itself data class.

BTW the doc says both are supported meaning that we can choose, but we actually need to support both to not crash.

Voyager by default expect that it screens can be stored inside a [Bundle](https://developer.android.com/guide/components/activities/parcelables-and-bundles). This means both Java Serializable and Parcelable are supported. By default all Voyager Screen is Java Serializable this means that Screen can be restored if all parameters are Java Serializable.

// ✔️ DO
class ValidScreen : Screen {

    // Serializable properties
    val tag = "ValidScreen"

    // Lazily initialized serializable types
    val randomId by lazy { UUID.randomUUID() }
}

// ✔️ DO
class ValidScreen : Screen {

    @Composable
    override fun Content() {
        // Inject your dependencies inside composables
        val postService = get<PostService>()
    }
}

[DevSrSouza]

I will keep investigating why the hashCode is being required, see if we can do something on Voyager that could improve this, if not, document it.

Really thanks for the reproducible code.

[Tolriq]

I actually just took your code from the other thread without changes (except adding more levels to ensure it triggers all the time).

[DevSrSouza]

Investigating I found out this line of code that call the hashCode on the Screen: https://github.com/androidx/androidx/blob/3fb1c5d452a11e08d15f588d56e0613f02450a9b/compose/animation/animation/src/commonMain/kotlin/androidx/compose/animation/AnimatedContent.kt#L794

AnimatedContent with States, state should be comparable in Compose and in Voyager we use the Screen as State in the transactions, I think a good solution here is providing on a Base Screen, like yours UniqueScreen the hashCode implementation and using data class as much as you can or use data class on all your Screens, should be enough as well.

I will try to figure out a solution directly in Voyager API, but, I had try to not change the API, but it seems not possible. I give you an update when a found another solution, for now, the hashCode will be required at minimum for State restoration using Transition APIs.

[Tolriq]

Ok so hashcode and equals to avoid future issues.

Thanks for the update.

[ubuntudroid]

FWIW data object works as well if you don't happen to have any parameters to pass to the screen.