Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the Store expose a StateFlow instead of a Flow #46

Open
thuutin opened this issue Apr 11, 2023 · 2 comments
Open

Make the Store expose a StateFlow instead of a Flow #46

thuutin opened this issue Apr 11, 2023 · 2 comments
Labels
enhancement New feature or request plumbing

Comments

@thuutin
Copy link
Contributor

thuutin commented Apr 11, 2023

Description

Make the Store expose a StateFlow instead of a Flow so it's easier to use in Compose. With StateFlow we can just use collectAsState() without providing an initial value and reducing recompositions.

@thuutin thuutin added enhancement New feature or request plumbing labels Apr 11, 2023
@sveltema
Copy link

sveltema commented Apr 11, 2023

Pretty sure that to change store state to val state: StateFlow<State>, the signature of fun optionalView would have to change to something like:

    fun <ViewState : Any, ViewAction : Any> optionalView(
        mapToLocalState: (State) -> ViewState?,
        mapToGlobalAction: (ViewAction) -> Action?
    ): Store<ViewState?, ViewAction>

or:

    fun <ViewState : Any, ViewAction : Any> optionalView(
        initialState: ViewState,
        mapToLocalState: (State) -> ViewState?,
        mapToGlobalAction: (ViewAction) -> Action?
    ): Store<ViewState, ViewAction>

When the mapToLocalState lambda returns an optional, there is no good way (?) to provide a non-optional initial value for a stateIn() function. For example from MutableStateFlowStore the required mapToLocalState(state.value)!! would cause crashes when the mapToLocalState lambda returns null.

For example:

    override fun <ViewState : Any, ViewAction : Any> optionalView(
        mapToLocalState: (State) -> ViewState?,
        mapToGlobalAction: (ViewAction) -> Action?
    ): Store<ViewState, ViewAction> = MutableStateFlowStore(
       state = state.mapNotNull { mapToLocalState(it) }
         .stateIn(storeScope, SharingStarted.WhileSubscribed(), mapToLocalState(state.value)!!),
        sendFn = { actions ->
            val globalActions = actions.mapNotNull(mapToGlobalAction)
            sendFn(globalActions)
        }
    )

@heytherewill
Copy link
Contributor

What sveltema said above is the reason we didn't implement this change while I was at toggl. We couldn't agree on what the API for optionalView should look like 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request plumbing
Projects
None yet
Development

No branches or pull requests

3 participants