-
Notifications
You must be signed in to change notification settings - Fork 13
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
Making the use_async
a bit more ergonomic
#29
Comments
hi, the design of separate fields for loading/data/error is a bit like SWR lib of js world, data and error could even co-exist for cache/user-friendly reasons, maybe we could not reduce them to a enum like Result<Data, Error>. |
Right now evaluating the state looks a bit like this: #[function_component(EvalView)]
pub fn eval_view(props: &EvalViewProps) -> Html {
match &props.eval {
UseAsyncState { loading: true, .. } => {
html!("Loading...")
}
UseAsyncState {
error: Some(err), ..
} => {
html!(format!("Failed: {err}"))
}
UseAsyncState {
data: Some(result), ..
} => {
html!(
<ResultView result={result.clone()}/>
)
}
_ => html!(""),
}
} Which isn't optimal IMHO. I think this could be changed into something like: #[function_component(EvalView)]
pub fn eval_view(props: &EvalViewProps) -> Html {
match &props.eval {
UseAsyncState::Loading => {
html!("Loading...")
}
UseAsyncState::Err(err) => {
html!(format!("Failed: {err}"))
}
UseAsyncState::Ok(result) => {
html!(
<ResultView result={result.clone()}/>
)
}
}
} Which feels a bit more natural to me. I am not sure in which case one would have all three values together. Maybe I don't understand the idea having having both data and error at the same time. |
I went ahead, trying to create an example of what I had in mind, something like: #[derive(PartialEq, Eq)]
pub enum UseAsyncState<T, E> {
/// Operation has not been started yet
Pending,
/// Operation is running
Loading,
/// Operation is complete
Ready(Result<T, E>),
} While updating the code I think I ran into what you had in mind with "caching", when updating the example. When the state changes to "loading" again, the old data is still available. So data can be shown, while loading new. I personally am not a fan of this, as it shows stale data. Maybe the solution could be to create a second version of |
I really like this project, especially the
use_async
hook.However, evaluating the state when rendering feels a bit verbose. It feels to me as if this could be improved a bit, using an enum.
If you are interested, I would try to come up with a PR, as basis for further refining it. Trying to keep the default API intact.
The text was updated successfully, but these errors were encountered: