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

Use a cloneable type for ActionForm's error prop instead of Box<dyn Error> #2174

Closed
SleeplessOne1917 opened this issue Jan 9, 2024 · 2 comments

Comments

@SleeplessOne1917
Copy link
Contributor

SleeplessOne1917 commented Jan 9, 2024

Describe the bug
While working on this PR, I realized I couldn't get the error from the ActionForm's error prop. To give a small snippet:

#[component]
fn Foo() -> impl IntoView {
    let error = RwSignal::new(None);

    Effect::new(move |_| {
        logging::log!("{:?}", error.get()); // This line will error because get cannot be called for signals with types that don't implement Clone
    });

    view!{
        <ActionForm action=some_action error=error>
               {/* Rest of form */}
        </ActionForm>
    }
}

The type of the error prop is Option<RwSignal<Option<Box<dyn Error>>>>. Since Error isn't constrained by Clone, the compiler will give an error when trying to call get on that RwSignal. Should signals even be able to be set with non-cloneable types in the first place? The Clone constraint exists on SignalGet, but not SignalSet, making it possible to set a value that cannot be gotten.

@gbj
Copy link
Collaborator

gbj commented Jan 9, 2024

.with() can be called on any signal whose value isn't Clone, i.e.,
error.with(|value| logging::log!("{value:?}"));

We are probably slightly working across each other at this point, as the error signal is being removed on ActionForm in favor of simply using the action's .value() signal, which it currently duplicates (i.e., the Err condition in that signal) #2158

I need to reply to your comments in your PR and will do that soon.

@SleeplessOne1917
Copy link
Contributor Author

Using value instead of error should simplify things for me quite a bit. Thanks for letting me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants