From e8705119dbcdd7d41feeb2bc8f3fdd115f550654 Mon Sep 17 00:00:00 2001 From: "christian.hausknecht" Date: Mon, 21 Oct 2024 11:55:37 +0200 Subject: [PATCH] Fixes some potential bug with mapping lenses The pimped `className`-function (introduced by PR #823 in `1.0-RC15` release) did not apply exception handling. That could lead to errors, if there is code that removes some element: The independent reactive renderings could simply lead to corner cases, where a lens will try to access its mapped object (e.g. by `mapByElement`) that is no longer part of the stored model. So a `CollectionLensGetException` will be thrown and must be handled by the consuming code. This PR adds such exception handling code and a unit test to verify the new, correct behaviour. --- .../src/jsMain/kotlin/dev/fritz2/core/tags.kt | 5 ++- .../src/jsTest/kotlin/dev/fritz2/core/tags.kt | 45 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/core/src/jsMain/kotlin/dev/fritz2/core/tags.kt b/core/src/jsMain/kotlin/dev/fritz2/core/tags.kt index 9587e22ca..cc66f5ceb 100644 --- a/core/src/jsMain/kotlin/dev/fritz2/core/tags.kt +++ b/core/src/jsMain/kotlin/dev/fritz2/core/tags.kt @@ -361,7 +361,10 @@ open class HtmlTag( } override fun className(value: Flow, initial: String) { - classesStateFlow.value += value.stateIn(MainScope() + job, SharingStarted.Eagerly, initial) + classesStateFlow.value += value + .catch { printErrorIgnoreLensException(it) } + .stateIn(MainScope() + job, SharingStarted.Eagerly, initial) + // this ensures that the set state is applied *immediately* without `Flow`-"delay". // in this case, the `initial` value gets applied as "promised". attr("class", buildClasses()) diff --git a/core/src/jsTest/kotlin/dev/fritz2/core/tags.kt b/core/src/jsTest/kotlin/dev/fritz2/core/tags.kt index d816e8cc3..ae8de5455 100644 --- a/core/src/jsTest/kotlin/dev/fritz2/core/tags.kt +++ b/core/src/jsTest/kotlin/dev/fritz2/core/tags.kt @@ -543,4 +543,49 @@ class TagTests { assertEquals("fourth", getAttribute()) } + @Test + fun testClassNameWithFlowInsideReactiveScopeBasedUponListMapping_WontThrowUnhandledCollectionLensGetException() = + runTest { + val items = listOf("red", "green", "blue") + + fun extractClasses() = items.mapNotNull { item -> document.getElementById(item) } + .joinToString(",") { it.className } + + val storedItems = storeOf(items, Job()) + val removeItem = storedItems.handle { allItems, toRemove -> + allItems.filterNot { it == toRemove } + } + + render { + storedItems.data.renderEach { item -> + val storedItem = storedItems.mapByElement(item) { it } + div(id = item) { + // access to mapped item will throw `CollectionLensGetException` after deletion + // -> must be catched internally like handlers do! + className(storedItem.data) + + // we give some time to the `className`-function to try to access a mapped lens for an object + // that might not exist anymore. + beforeUnmount { _, _ -> delay(100) } + } + } + } + + delay(100) + assertEquals("red,green,blue", extractClasses()) + + // this is the source of possible problem: By removing mapped lenses won't find their targeting object + // anymore. This will lead to `CollectionLensGetException`, that must be handled internally! + removeItem("green") + + delay(200) + assertEquals("red,blue", extractClasses()) + + removeItem("blue") + + // if not exception handling on the `classes`-Flow is done, rendering will freeze and the node for green + // will remain. + delay(200) + assertEquals("red", extractClasses()) + } } \ No newline at end of file