-
Notifications
You must be signed in to change notification settings - Fork 154
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
Fallible components #528
Comments
As some kind of input from others, I'd like to throw in what leptos does; the following quotes from their docs of their /// When you render a `Result<_, _>` in your view, in the `Err` case it will
/// render nothing, and search up through the view tree for an `<ErrorBoundary/>`.
/// This component lets you define a fallback that should be rendered in that
/// error case, allowing you to handle errors within a section of the interface.
///
/// ```
/// # use leptos_reactive::*;
/// # use leptos_macro::*;
/// # use leptos_dom::*; use leptos::*;
/// # if false {
/// # run_scope(create_runtime(), |cx| {
/// let (value, set_value) = create_signal(cx, Ok(0));
/// let on_input = move |ev| set_value(event_target_value(&ev).parse::<i32>());
///
/// view! { cx,
/// <input type="text" on:input=on_input/>
/// <ErrorBoundary
/// fallback=move |_, _| view! { cx, <p class="error">"Enter a valid number."</p>}
/// >
/// <p>"Value is: " {value}</p>
/// </ErrorBoundary>
/// }
/// # });
/// ```
I do favor this approach instead of the one where I have to specify the error component in an attribute for two reasons:
One downside is the decision to simply not do anything at all on an error result if there is no error boundary. I don't really know what you meant by the following section of your proposal:
If the approach you are opposed to is similar to what leptos provides for bubbling things up, I would like to understand your concerns a bit more. Especially I don't seem to see how abstracting things into a separate function to match on is a natural way for error handling (I'd say bubbling up custom error types until there is a component that can properly handle the error seems more rusty).
This is essentially what leptos does, yes (mapping the component by their hydration key to an error if any). |
I think 2 error handling primitives should be added:
In more details:
view! {
ReturnsResult {}?
(/*also returns a Result*/)?
}
struct ErrorView<E: Error, F: Fn(E) -> View> {
inner: E,
view: F
}
view! {
ReturnsResult {}.(|it| it.unwrap())
} I think this should be enough for most use cases. Also it goes hand in hand with classic rust error handling rather then the leptos approach mentioned above. This would makes the components feel like rust rather then js like. I'm willing to make a pr for thislet me know if if this is good or you want a different design |
I have some doubts on having to manually bubble-up errors.
So although using the Leptos approach of introducing a |
Another requirement for fallible components should be good integration with resources and async APIs. I think this is probably the most likely scenario where one encounters errors, for instance, if the server returns something unexpected. So I think a context-based approach would work best here. I'm currently thinking of how we might want to do this. |
I should have been more specific with it i meant that a component would return `Result<View, ErrorView> and then it would be either bubbled or matched Err(e) => e.into() that way the parent component can decide if they want to handle the error in which case they would need the inner: E or just display it how the child component wants it |
Personally i think that both top level and bubbling are very useful for different use cases so maybe both should be added but i'm unsure how they would coexist |
I feel like explicitly bubbling errors should be done in data-fetching logic rather than in components. Looking at Leptos' approach, I think it's actually surprisingly clean: adding a I think the problem with manually propagating errors is that a lot of the times, you don't get errors during creation of the component but after some interaction has happened. One thing that could be nice, however, would be to have a way to keep the concrete error type instead of getting it erased into a trait object. I'm not sure if that's possible though. |
alright, u convinced me with this, I will take a look at this again after i'm done with query params in routes |
Actually this can be done just be matching on the signal like we would without error boundaries anyways so this is not too much of a problem. |
Right now, error handling in Sycamore is difficult and fairly unergonomic, since any errors have to be handled within the component in which they occur, since there is no concept yet of a component that can return an error. I would propose the following pattern:
Central to this is a
#[fallible_component]
macro or the like, which would desugar, for the above example, to something like this:More formally, that macro should annotate a function with the return type
Result<View<G>, E>
, where the argument to the macro is a Sycamore component that takes a single argument of typeE
(i.e. it takes the error to process it and display it). This would allow individual components to return errors and then have them automatically handled and displayed neatly to the user.This is just one approach, similar to the current suspense system (with its fallback view), with the obvious part lacking being allowing errors to bubble up. We could alternately allow callers to handle errors in components, though I personally don't favor this approach (I think anything that the caller should handle ought to be abstracted to a separate function that it
match
es on, as one would do already). That way, Sycamore provides a convenient extra rather than just formalizing the existing workaround. Notably, both approaches should be documented if the fallible components idea is accepted, since there will likely be a place for both approaches in apps.There is also the issue of allowing errors to bubble up completely, which could be achieved with a special context element for handling errors in components. Inside that, each component could have an entry, and then other components could check as necessary if something else has failed. This would be clearly useful for parents to check if their children have failed, while sibling checks would probably only be regarding whether or not some other part of context is accessible, in which case it should probably just be in a
Result
itself, external to any systems Sycamore might provide.Pending input from others, I would be very happy to work on this!
The text was updated successfully, but these errors were encountered: