Key that contains an enum will not get restored after process kill

[matejdro]

Screen created by the following key:

data class DemoKey(val enum: DemoEnum): DefaultComposeKey {
    enum class DemoEnum {
        A, B
    }
    
    ...
}

will not get state restored after app's process is killed and restored.

Reason for that is that we use screen keys as compose keys to make compose distinguish between different screens. Key function uses hashCode() underneath to perform its function. Unfortunately, JVM Enums's hashcode is not stable across multiple JVM processes. Thus, when this screen is restored, key's hashcode will be different from the previous one (even though data will be the same) and Android thinks that restored key is different than the one provided before and it recreates the screen from scratch.

Workaround is to manually override hashCode of the data class and hashcode stable value instead of enum itself, for example enum's name works:

```kotlin
data class DemoKey(val enum: DemoEnum): DefaultComposeKey {
    enum class DemoEnum {
        A, B
    }
    
    override fun hashCode(): Int {
        return enum.name.hashCode()
    }

    ...
}

Workaround is ugly and error prone (if you add a field and forget to update the hashcode function, things can go wrong), however this is the best I came up with so far.

EDIT: Another possible workaround is to use sealed class instead of enum. It's more verbose, but at least it's safer:

data class DemoKey(val enum: DemoEnum): DefaultComposeKey {
    @Parcelize
    sealed class DemoEnum : Parcelable {
        @Parcelize
        data object A: DemoEnum() 
        @Parcelize
        data object B: DemoEnum() 
    }
    ...
}

This is not really a bug on our side (we can't fix it), but I just wanted to start a discussion on what (if anything) should we do about this. Maybe update the documentation?

[Zhuinden]

That's messed up... I could think of this in theory

data class EnumArgumentWrapper<E: Enum<E>>(val value: Enum<E>): Serializable {
    override fun hashCode(): Int {
        return value.name.hashCode()
    }

    override fun toString(): String {
        return value.name
    }

    override fun equals(other: Any?): Boolean {
        if (this === other) return true
        if (javaClass != other?.javaClass) return false

        other as EnumArgumentWrapper<*>

        if (value != other.value) return false

        return true
    }
}

But if you get classes from outside and want to use those, this doesn't really work. Also, who wants to remember doing this?

This is not an obvious case. I guess the solution is to use .name() and .valueOf() instead of the enum itself even in a key.

[Zhuinden]

Writing the readme, I said this:

data class DemoKey(private val enumName: String): DefaultComposeKey {
    constructor(enum: DemoEnum): this(enum.name)

    val enum: DemoEnum get() = DemoEnum.valueOf(enumName)
}

[Zhuinden]

Actually... you know, this is a bit of an eye-opener. You know how Navigation-Compose has a String representation of routes and arguments, right?

In a way, if ordering is guaranteed, which in their case you build the URL string in the same format each time; then the arguments become hash-stable.

As while sure, you can add some tricks to handle enum class in an argument when you use hashCode() as used by the Saver framework, but this won't handle nested cases.

We might need to either somehow get rid of reliance on hashCode() (how?) or build an internal String representation of a key that is hash-stable after all, which is potentially either bad for performance/memory usage or error-prone. 😒 Honestly i'm just brainstorming because this means all classes are unsafe when used with hashCode() and hashCode() is expected to be stable across process death.

[matejdro]

We could just use key.toString() which fixes this specific case, but reduces performance, because all keys must be serialized to strings and it just replaces reliance on hashcode() with reliance on toString().

[Zhuinden]

I wonder if it'd make sense to do the same thing as DefaultFragmentKey and depend on the hash code of the class of the key, rather than actually calculate it based on the full content. After all, if there's collision, it'd just invoke equals anyway, which is correct.

[matejdro]

I think this might break state saving for compose between different screens (they rely heavily on hashcodes as state saving keys), but I'm not 100% sure, we might be in the clear, because our state saving is overridden by SaveableStateProvider anyway.

[matejdro]

Another issue with the enumName approach that I just discovered: .copy() method still receives enum name. It's fairly easy to just add .name when copying, but it's another situation, where invalid value could creep in.