leonard-palm / compose-state-events Goto Github PK

Hi. Thank you for this library! Noticed a possible bug where the action for an EventEffect or NavigationEventEffect is called twice when triggered. This happens when the onConsumed parameter is set with a lambda. I think this is a valid way to use the effects, but I'm not sure. Can you let me know your thoughts? Thanks again

val screenState by viewModel.screenState.collectAsStateWithLifecycle()

// The action for this effect is called twice per trigger.
NavigationEventEffect(
    event = screenState.closeEvent,
    onConsumed = { viewModel.onConsumeCloseEvent() }
) {
    navController.popBackStack()
}

// The action for this version is called just once as expected.
NavigationEventEffect(
    event = screenState.closeEvent,
    onConsumed = viewModel::onConsumeCloseEvent
) {
    navController.popBackStack()
}

// View Model
private val _screenState = MutableStateFlow(initialState)
val screenState = _screenState.asStateFlow()

...

fun onCloseClick() {
    _screenState.update { it.copy(closeEvent = triggered) }
}

fun onConsumeCloseEvent() {
    _screenState.update { it.copy(closeEvent = consumed) }
}

A possible fix for this is to remove the onConsumed parameter from the keys on the LaunchedEffect within NavigationEventEffect and EventEffect.

Hi, thanks for your inspiring work!

One question; Why is StateEventWithContent implemented as an abstract class instead of sealed interface/class?

I have a Room DAO that has the following query. My ViewModel then defines the Pager instance I use inside my ViewModel as shown below:

/// DAO
@Query("SELECT * FROM <table> ORDER BY createdAt DESC")
fun getAllPaged(): PagingSource<Int, Model>

/// ViewModel
class MyViewModel: ViewModel() {
    val allItems = Pager(PagingConfig(pageSize = 10)) {
        myDao.getAllPaged()
    }.flow.cachedIn(viewModelScope)
}

/// UI
@Composable
fun MyContent(viewModel: MyViewModel = hiltViewModel()) {
    val items = viewModel.allItems.collectAsLazyPagingItems()
}

How do I keep the Pager lazy when adding it to the ViewState data class? I don't know if this is possible and by checking online I don't think it is.

Hi,

I'm wondering what your thoughts are about marking the StateEvent / StateEventWithContent and its implementation as either Stable or Immutable. I think both are applicable (though Immutable should be more fitting) and would enable the Composables where those types are used to be skippable.

Any thoughts on this?

As reference: Jetpack Compose Stability Explained