Skip to content

Commit

Permalink
Fixes some potential bug with mapping lenses (#910)
Browse files Browse the repository at this point in the history
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.

Co-authored-by: christian.hausknecht <christian.hausknecht@oeffentliche.de>
  • Loading branch information
Lysander and christian.hausknecht authored Oct 21, 2024
1 parent 2f1868c commit fc099ae
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
5 changes: 4 additions & 1 deletion core/src/jsMain/kotlin/dev/fritz2/core/tags.kt
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,10 @@ open class HtmlTag<out E : Element>(
}

override fun className(value: Flow<String>, 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())
Expand Down
45 changes: 45 additions & 0 deletions core/src/jsTest/kotlin/dev/fritz2/core/tags.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> { 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())
}
}

0 comments on commit fc099ae

Please sign in to comment.