JetBrains / compose-multiplatform

Compose Multiplatform, a modern UI framework for Kotlin that makes building performant and beautiful user interfaces easy and enjoyable.
https://jetbrains.com/lp/compose-multiplatform
Apache License 2.0
16.01k stars 1.17k forks source link

`rememberSaveable` does not work - restore/save logic is never called #1932

Closed hakanai closed 6 months ago

hakanai commented 2 years ago

I currently have:

    val appState = remember { AppState() }

I thought I would try switching to rememberSaveable so that app settings would persist across app invocations.

So I changed it to:

    val appState = rememberSaveable(saver = AppState.Saver) { AppState() }

AppState.Saver is currently just:

    val Saver = object : Saver<AppState, Any> {
        override fun restore(value: Any): AppState? {
            TODO("Not yet implemented")
        }

        override fun SaverScope.save(value: AppState): Any? {
            TODO("Not yet implemented")
        }
    }

If I put a breakpoint on those TODO lines, I can see that they are never called. I'd expect a call to save during application exit, and a call to restore some time during startup.

I assume there's a way to call the right things programmatically to get this to happen as a workaround but haven't figured it out yet.

akurasov commented 2 years ago

On which OS do you use it?

hakanai commented 2 years ago

I'm on Windows 10.

hakanai commented 2 years ago

Initially, from what I gathered reading docs, it seemed like rememberSaveable was the way to remember state across runs of an application.

But the impression I get now is that perhaps rememberSaveable is just a hack to work around Android's terrible behaviour where it throws your app state away when you rotate the device, and on desktop I appear to be able to resize the window totally fine without this sort of hack, so perhaps rememberSaveable was never meant to work on desktop?

And worse - the impression I get from talking to people in a few places now is that both remember and rememberSaveable should be avoided as much as possible, and we should be initialising all our app state before even calling the first composable function.

So instead of doing this:

fun main() = singleWindowApplication {
    MaterialTheme {
        val state = remember { AppState() }
        MainUi(state)
    }
}

We should actually be doing this:

fun main() {
    val state = AppState()
    singleWindowApplication {
        MaterialTheme {
            MainUi(state)
        }
    }
}

Which is fine, but now I'm back to having no convenient way to store my app state at all, and I'm back to reinventing the wheel.

akurasov commented 2 years ago

I think saving state of App is not a functionality of UI framework... You just need to put your state to some object and serialize it to some persistant storage.

hakanai commented 2 years ago

I partially agree, except:

  1. It isn't clear that Compose is just a "UI framework". The docs really make it seem like an "app framework" instead - code examples show the main function directly calling their singleWindowApplication function, i.e. the very first line of code is Compose code.

  2. This means I have to reinvent the wheel even though surely I am not the only person trying to use this framework to make full applications.

So yeah, Compose may be a UI framework and not an app framework, but if that's the case, things like rememberSaveable are confusing at best, and it means we have to wait for someone to make an actual app framework which is built on Compose until we can really develop apps with ease, because "just putting state in some object and serialising it" is not trivial.

Burtan commented 2 years ago

Did you end up writing your own version of remember for things like windowState/appState which actually persists the state?

hakanai commented 2 years ago

No... I didn't have the slightest idea where to start.

syt0r commented 1 year ago

I did some investigation and looks like rememberSaveable actually works, but you need to implement additional thing to make it work, it's just that android's navigation library already has it implemented underneath. Also on desktop serialization is not really is necessary since everything is stored in memory

To make it work you need to create instance of SaveableStateHolder with rememberSaveableStateHolder() and wrap you navigation destinations with stateHolder.SaveableStateProvider(stringKey) { content }, it will assosiate your content with key and save data for specific destinations, then when removing destination you just need to invoke SaveableStateHolder.removeState(key)

Here is snippet of what I did in my project:

interface MultiplatformMainNavigationState : MainNavigationState {
    val currentDestination: State<MainDestination>
    val stateHolder: SaveableStateHolder
}

@Composable
fun MultiplatformMainNavigation(
    state: MainNavigationState
) {

    state as MultiplatformMainNavigationState

    val destination = state.currentDestination.value

    state.stateHolder.SaveableStateProvider(destination.toString()) {
        when (destination) {
            MainDestination.Home -> {
                val homeNavigationState = rememberHomeNavigationState()
                HomeScreen(
                    viewModel = getMultiplatformViewModel(),
                    mainNavigationState = state,
                    homeNavigationState = homeNavigationState
                )
            }
            MainDestination.About -> {
                AboutScreen(
                    mainNavigationState = state,
                    viewModel = getMultiplatformViewModel()
                )
            }
            // ...others
        }
    }

}

@Composable
fun rememberMultiplatformMainNavigationState(): MainNavigationState {
    val stack = remember { mutableStateOf<List<MainDestination>>(listOf(MainDestination.Home)) }
    val currentDestinationState = remember { derivedStateOf { stack.value.last() } }

    val stateHolder = rememberSaveableStateHolder()

    return remember {
        object : MultiplatformMainNavigationState {

            override val currentDestination = currentDestinationState
            override val stateHolder: SaveableStateHolder = stateHolder

            override fun navigateBack() {
                val lastItem = stack.value.last()
                stack.value = stack.value.dropLast(1)
                stateHolder.removeState(lastItem.toString())
            }

            override fun popUpToHome() {
                val itemsToRemove = stack.value.drop(1)
                stack.value = stack.value.take(1)
                itemsToRemove.forEach { stateHolder.removeState(it) }
            }

            override fun navigate(destination: MainDestination) {
                stack.value = stack.value.plus(destination)
            }

        }
    }
}
syt0r commented 1 year ago

Btw, don't misunderstand, rememberSaveable is to save data associated with certain position of backstack during navigation and restore it later when navigating back, it's not meant to persist data across multiple application sessions

hakanai commented 1 year ago

I disagree. If you don't need it to persist across application invocations, remember works perfectly fine.

The docs for rememberSaveable talk about the information being persisted, which in my head suggests that it should work across app invocations.

The actual wording in the docs is:

The rememberSaveable API behaves similarly to remember because it retains state across recompositions, and also across activity or process recreation using the saved instance state mechanism. For example, this happens, when the screen is rotated.

Please re-read this carefully, particularly this bit:

also across activity or process recreation

So now that you have read the docs, please explain the following:

  1. When you say this:

    Also on desktop serialization is not really is necessary since everything is stored in memory

    What do you mean exactly? If it's only stored in memory, how can it even be useful? The memory is blown away when you kill the app.

  2. What do you mean by this?

    don't misunderstand

    Because as far as I can tell, my view is in line with the docs. Do you have a source to cite which says that rememberSaveable doesn't use persistence? If so, feel free to provide it to the Compose team, so they can update the docs to reflect that.

syt0r commented 1 year ago

This description refers to android specific components so it's not completely relevant for multiplatform. And even in android case it doesn't persist data across application sessions, the only exception is the case when single session is temporary ̶k̶i̶l̶l̶e̶d̶ hibernated by system and then system restores it later, I don't think that any desktop OS has similar behavior

hakanai commented 1 year ago

Correct. Now you're up to date with what I said on Mar 16, 2022 above.

Although I never had an Android device handy to really test that behaviour. It always seemed to me that the user killing the process should be treated the same way.

So as far as I can tell, saying that rememberSaveable doesn't work for desktop is accurate. It's just that, due to what it was intended to be used for, it was never intended to work.

With regard to the question (paraphrased):

"Do any desktop OS have the ability to suspend and recreate apps which were previously running?"

The answer is yes, it happens on macOS. When you reboot, things like TextEdit have their state serialised and come back with the same state next session, including all unsaved data. So of course, I consider that sort of behaviour essential for a well-behaved desktop app, which is why I wanted this sort of state saving to work.

As far as I can tell, all you have to do to appear to be doing the same thing as TextEdit is, (1) save your state, and (2) terminate immediately when the OS asks you to terminate, don't pop up prompts to save documents, etc. Note that you don't have to save your state at the shutdown request - you could save state continuously, so that it's always safe to just terminate.

syt0r commented 1 year ago

Oh right, I remember that feature, mac os applications certainly might persist data between reboots, although I'm not sure whether their state is restored, I only remember it could reopen all applications automatically. This might be a good feature, but idk if any other desktop os doing something similar. Also I don't think rememberSaveable will work like this right now.

Also here a better description from a SaveableStateHolder documentation:

Allows to save the state defined with rememberSaveable for the subtree before disposing it to make it possible to compose it back next time with the restored state. It allows different navigation patterns to keep the ui state like scroll position for the currently not composed screens from the backstack.

This is the only feature from androidx.compose.runtime.saveable

hakanai commented 8 months ago

Worth noting that the current version of Notepad on Windows now behaves like this as well. You can kill the application and the next time it will come back with the window in the same place and your text still there.

MatkovIvan commented 8 months ago

Let me try to summarize this thread.

Android Jetpack Compose behavior

rememberSaveable relies (via a few abstraction layers) on system's Saved instance state mechanism. From the documentation:

Saved instance state
Storage location in memory
Survives configuration change Yes
Survives system-initiated process death Yes
Survives user complete activity dismissal No
Read/write time slow (requires serialization/deserialization)

So, it's NOT stored on disk, it does NOT survive when the user closes the application.

Compose Multiplatform behavior

Currently save/restore aren't invoked out of the box because SaveableStateRegistry is not provided by default (yet). We do not plan to provide saving it to disk out of the box since it's not the initial purpose of this API. Since the lifecycle/scope of stored data is outside of Compose itself and relies on navigation, currently we cannot provide expected/correct implementation out of the box, but it's on our radar as an integration point with the future navigation solution.

For now you can provide a custom implementation of SaveableStateRegistry (see below).

Providing custom savable logic

If you want to provide custom storing logic, you can do it via providing LocalSaveableStateRegistry:

CompositionLocalProvider(
    LocalSaveableStateRegistry provides saveableStateRegistry,
) {
    // your compose content
}

Example of simpleSaveableStateRegistry that stores values as-is inside process memory (might be useful if you recreate Compose during the run and want to restore the state):

class GlobalSaveableStateRegistry(
    val saveableId: String,
) : SaveableStateRegistry by SaveableStateRegistry(
    restoredValues = map[saveableId],
    canBeSaved = { true }
) {
    fun save() { map[saveableId] = performSave() } // Should be called manually before Compose's dispose
    companion object {
        private val map = mutableMapOf<String, SaveableStateData>()
    }
}

Based on this you can implement saving the state on the disk if you want to.

hakanai commented 8 months ago

I assume the missing piece here is something along these lines...

@Composable
fun rememberGlobalSaveableStateRegistry(saveableId: String): SaveableStateRegistry {
    val saveableStateRegistry = remember {
        GlobalSaveableStateRegistry(saveableId)
    }
    DisposableEffect(Unit) {
        onDispose {
            saveableStateRegistry.save()
        }
    }
    return saveableStateRegistry
}

But yeah, it really just sounds like this was a misunderstanding on my part, caused by the wording in the docs.

The docs say, "and also across activity or process recreation", which leads one to be optimistic about the situations in which it should work. The docs do follow that with, "using the saved instance state mechanism", but I didn't come from Android dev, so I read that as buzz, rather than a description of a limitation. There's no hyperlink from that paragraph out to anywhere describing what that mechanism means, which reinforces my interpretation of it as buzz.

I maintain that being able to remember state across process recreation, even if it was an OS crash which caused it, is useful behaviour, though. It just sounds like implementing that is being pushed back on the app developer. Maybe some day there will be a framework where the batteries are included, because I for one am sick of building batteries.

MatkovIvan commented 8 months ago

I intentionally avoided DisposableEffect usage here because it might be called after disposing rememberSaveable itself (it removes it from saving list). By "manually" I meant calling it outside of compose before disposing Compose/platform view (ComposePanel) with it

hakanai commented 7 months ago

Buuuuut, how do you know when you should call it then? DisposableEffect is the only obvious API I was able to think of. Is there some other way to find out that the current context is about to be disposed? I certainly have no idea from my own code.

When I wrote that code, I think I was assuming that disposals are called in the opposite order from the order they are initialised. Which is usually a fairly sane assumption to make, but I don't know how Compose has actually implemented it. (Maybe what I should have done was had the DisposableEffect also create the thing which actually does the saving? Then I'd have it in scope and would know that it hasn't been disposed by anyone else.)

What I wish Compose had was something like a rememberDisposable which would allow me to remember a thing but also have a hook that would be called to dispose it.

MatkovIvan commented 7 months ago

how do you know when you should call it then?

I thought about a general case with ComposePanel when you have direct control of disposing it/Compose.

When I wrote that code, I think I was assuming that disposals are called in the opposite order from the order they are initialised.

It's true, the order is opposite and consistent. But you need to save it BEFORE disposing the content that you will initialize later

You just need to make sure you call DisposableEffect in the right place:

setContent {
    val saveableStateRegistry = remember { GlobalSaveableStateRegistry("KEY") }
    CompositionLocalProvider(LocalSaveableStateRegistry provides saveableStateRegistry) {
        ComposeContent()
    }
    DisposableEffect(Unit) {
        onDispose { saveableStateRegistry.save() }
    }
}

Previously, since I had full control over Compose lifecycle, I thought it would be just easier not to rely on that order. But if it done correctly, it will work with DisposableEffect too

pablichjenkov commented 7 months ago

It has to be in the root Composable right Ivan? On another hand I really think bringing process-death/activity-resrorarion recovery concepts to compose is adding unnecessary complexity. That should be part of a state management library like decompose or whatever out there. Take for example the Platform Lifecycle library, which is independent from compose. Integrates with it but is not required. Just an opinion. Traditionally in Android this process death api brings Development slowness. If my screen data/state is already backed up by a database why do I need to save it twice. Things of that nature. I side with user hakanai, all we need is an event for when is it gonna happen. Again , my humble opinion.

MatkovIvan commented 7 months ago

It has to be in the root Composable right Ivan?

No, it's regular compose local and might be applied/replaced only to part of your application. I didn't test it personally, but I don't see any reason why it wouldn't work

Take for example the Platform Lifecycle library, which is independent from compose. Integrates with it but is not required.

I'm currently working on porting lifecycle, things a bit more complicated there. On Android lifecycle library is independent from Compose, but vice versa - Compose has it in the dependencies to find "lifecycle owner" from View tree and provide as composition local. However, there is no such "lifecycle owner" like activity in multiplatform, and Compose does window management related things itself. So, as the first step Compose will be "lifecycle owner". With ViewModels and "state registry owner" the situation is similar - there is no API at the moment to hold it outside of Compose. I wrote about it above:

Since the lifecycle/scope of stored data is outside of Compose itself and relies on navigation, currently we cannot provide expected/correct implementation out of the box

But it's another topic.

If my screen data/state is already backed up by a database why do I need to save it twice

Usually current visual state is not stored in database. I mean for example collapsed spoiler or selected text range. rememberSaveable and saving state on config changes in general is about such cases.

all we need is an event for when is it gonna happen

What happen? The last example is about custom registry with custom logic. It's up to you when you're going to save the current state. Maybe you want to save it every minute? The saving just before disposing just allows you to save the state for next run - it's what @hakanai wanted. My example shows how to achieve that. Anyway, "event" is out of scope of the original issue. Please follow https://github.com/JetBrains/compose-multiplatform/issues/2915 for lifecycle events. But I doubt that it's directly related.

pablichjenkov commented 7 months ago

I understand, what happens is sometimes rememberSaveable is misused / overused, saving state that perhaps don't belong there. But definitely the data or ui-state that goes into the database shouldn't know about process death at all. Thanks for the DisposableEffect clarification.

MatkovIvan commented 6 months ago

Compose Navigation provides LocalSaveableStateRegistry inside NavHost, so rememberSaveable restores its state after navigating back to the previous screen. It's available in the latest dev, and I'm considering it as a main use case, so closing this issue.

BTW rememberSaveableStateHolder + SaveableStateHolder.SaveableStateProvider(content) does the same as the code snippet above but easier

eygraber commented 4 months ago

@MatkovIvan I would think that rememberSaveable in the browser would use something like the browser's session storage API so the state is saved across tab refreshes, browser restarts, etc...

Is there any plan for that, or would I need to roll my own with LocalSaveableStateRegistry?

MatkovIvan commented 4 months ago

While I agree that sessionStorage is much closer to original meaning than the Desktop case from this issue, but as I wrote above, rememberSaveable is not supposed to restore state after explicit "close" action from user. So for web page refresh shouldn't restore the previous value - I doubt that you're expecting to save scroll position after page refresh. It might be a valid cases like restoring after closing by memory saver, but I doubt that we can distinguish such cases. There is no short term plan to do that yet, so custom registry is a way to go. PS could you please create separate issue for web case with session storage API? It's better not to mix that with Desktop case.

eygraber commented 4 months ago

https://github.com/JetBrains/compose-multiplatform/issues/4896

I addressed some of your comments in the description there.

okushnikov commented 2 months ago

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.