diff --git a/koma-compose/src/commonMain/kotlin/io/github/komakt/koma/compose/ViewStore.kt b/koma-compose/src/commonMain/kotlin/io/github/komakt/koma/compose/ViewStore.kt index ca709e6..54fffd5 100644 --- a/koma-compose/src/commonMain/kotlin/io/github/komakt/koma/compose/ViewStore.kt +++ b/koma-compose/src/commonMain/kotlin/io/github/komakt/koma/compose/ViewStore.kt @@ -121,6 +121,9 @@ class ViewStore internal constructor( * For a given remembered Store instance, this function returns the same [ViewStore] instance across * recompositions. A new [ViewStore] is created only when a new Store instance is remembered for * [key]. + * The [autoClose] value is fixed when that remembered Store instance is adopted. Recomposition + * alone does not change close ownership for the same Store, but when [key] causes a different + * Store instance to be remembered, the current [autoClose] value is used for that new Store. * * @param key Key used to remember and retain the Store instance * @param autoClose Whether to close the Store when the composable leaves the composition @@ -130,20 +133,19 @@ class ViewStore internal constructor( @Suppress("unused") @Composable fun rememberViewStore(key: Any? = null, autoClose: Boolean = false, store: @Composable () -> Store): ViewStore { - val fixedAutoClose = remember { autoClose } - val holder = remember(key) { object { var value: Store? = null } } val rememberedStore = holder.value ?: store().also { holder.value = it } + val closeStoreOnDispose = remember(rememberedStore) { autoClose } val state = rememberedStore.state.collectAsState() DisposableEffect(rememberedStore) { onDispose { - if (fixedAutoClose) { + if (closeStoreOnDispose) { rememberedStore.close() } } diff --git a/koma-compose/src/jvmTest/kotlin/io/github/komakt/koma/compose/ViewStoreJvmTest.kt b/koma-compose/src/jvmTest/kotlin/io/github/komakt/koma/compose/ViewStoreJvmTest.kt index df0ab16..3e89483 100644 --- a/koma-compose/src/jvmTest/kotlin/io/github/komakt/koma/compose/ViewStoreJvmTest.kt +++ b/koma-compose/src/jvmTest/kotlin/io/github/komakt/koma/compose/ViewStoreJvmTest.kt @@ -334,7 +334,7 @@ class ViewStoreJvmTest { } @Test - fun rememberViewStore_closeBehaviorUsesInitialAutoCloseValue() = runTest(testDispatcher) { + fun rememberViewStore_closeBehaviorIsFixedPerRememberedStore() = runTest(testDispatcher) { val neverCloseStore = TestStore(UiState.Ready(0)) var autoClose = mutableStateOf(false) @@ -349,14 +349,67 @@ class ViewStoreJvmTest { assertEquals(0, neverCloseStore.closeCount) - val closeStore = TestStore(UiState.Ready(0)) + val alwaysCloseStore = TestStore(UiState.Ready(0)) + autoClose.value = true withComposition( content = { - rememberViewStore(autoClose = true) { closeStore } + rememberViewStore(autoClose = autoClose.value) { alwaysCloseStore } + }, + afterSetContent = { + autoClose.value = false }, ) - assertEquals(1, closeStore.closeCount) + assertEquals(1, alwaysCloseStore.closeCount) + } + + @Test + fun rememberViewStore_keyChangeAppliesAutoCloseToNewRememberedStore() = runTest(testDispatcher) { + val firstStore = TestStore(UiState.Ready(0)) + val secondStore = TestStore(UiState.Ready(0)) + val frameClock = BroadcastFrameClock() + var frameTimeNanos = 0L + var key = "first" + var autoClose = false + + suspend fun pumpFrame() { + testScheduler.runCurrent() + frameTimeNanos += 16_000_000L + frameClock.sendFrame(frameTimeNanos) + testScheduler.runCurrent() + } + + withContext(frameClock) { + withRunningRecomposer { recomposer -> + val composition = Composition(NoOpApplier(), recomposer) + try { + composition.setContent { + rememberViewStore(key = key, autoClose = autoClose) { + if (key == "first") firstStore else secondStore + } + } + repeat(2) { pumpFrame() } + + autoClose = true + key = "second" + composition.setContent { + rememberViewStore(key = key, autoClose = autoClose) { + if (key == "first") firstStore else secondStore + } + } + repeat(2) { pumpFrame() } + + assertEquals(0, firstStore.closeCount) + assertEquals(0, secondStore.closeCount) + } finally { + composition.dispose() + pumpFrame() + } + } + } + + assertEquals(0, firstStore.closeCount) + assertEquals(1, secondStore.closeCount) } @Test