[Proposal] Simplify StoreResponse by Replacing Loading and NoNewData with a isFetcherActive flag in Data

[eyalgu]

API update proposal:

Some of the alpha feedback we've gotten is that there are too many states to handle under StoreResponse.

We currently have the following
Loading
NoNewData
Data
Error.Message
Error.Exception

I propose we drop Loading and NewNewData and instead add a isFetcherActive flag to Data (and maybe also Error) that can be used by the users to know whether to show a spinner (or whatever they are using the loading flag for right now).

The isFetcherActive will only mirror the state of the original fetch associated with the stream and will not flip back and forth if subsequent steam calls are made that re-trigger the fetcher (thus preserving the existing Loading/NoNewData behavior)

As a bonus this will also eliminate the 2 states of StoreResponse that are only valid for origin=fetcher.

[ferinagy]

While we are removing states, why do we need 2 error states? I think after quick look, I did not even find Error.Message used anywhere. IMO one error state with a throwable would suffice.

[eyalgu]

While we are removing states, why do we need 2 error states? I think after quick look, I did not even find Error.Message used anywhere. IMO one error state with a throwable would suffice.

I've open #231 for discussing the error states. we do need to handle non throwable error propagation because we don't want to force our users to have to use exceptions to propagate errors.

[ferinagy]

and maybe also Error

Is there a case when a first response in the stream would be an Error and there would be still fetch happening? I thought that would not be possible?

The isFetcherActive will only mirror the state of the original fetch associated with the stream and will not flip back and forth...

To avoid this confusion, maybe we could, instead of adding a flag, add a new state DataWhileLoading (help with better name appreciated), that would be only emitted as a first state. That way, we don't get tempted with checking isFetcherActive on the subsequent Data states.

[yigit]

this feels like making stream not a stream of events anymore. Feels very similar to what we wanted to do very early on and we decided not to since this information can be kept with a simple stream fold operation.
My first instinct is to implement the helper to do this and then the helper can have the logic.

something like.

data class UIState<T>(
    val loading:Boolean,
    val data: T?,
    val error:Throwable
)

The helper would handle restart of the stream (using the trigger mentioned in #124 ) but also keep previous data.

Usage would be like:

helper.flow.collect { uiState : UIState ->
}

refreshButton.onClick {
    helper.refresh()
}

[yigit]

here is a quick and dirty prototype:

5bd4e27

[digitalbuddha]

I'm a fan of externalizing the UIState controller similar to what Yigit proposed above. Closing for now