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

For the Show component, make the fallback property or default to an empty view #1760

Closed
svieujot opened this issue Sep 20, 2023 · 5 comments · Fixed by #1817
Closed

For the Show component, make the fallback property or default to an empty view #1760

svieujot opened this issue Sep 20, 2023 · 5 comments · Fixed by #1817

Comments

@svieujot
Copy link

Very often, we have:

<Show when=... fallback=|| view! { "" }>
    ...
</Show>

It would be nice to make this attribute the default one.

@gbj
Copy link
Collaborator

gbj commented Sep 20, 2023

  1. || () is the shortest form, a bit better than || view! { "" }
  2. I don't think it's possible to make this an optional prop without adding some overhead (making it a boxed function) or with some other proposal like FromFn trait to deal with prop conversion #1759. If you can find a way to implement it, I'm happy to take a PR.

@rambip
Copy link
Contributor

rambip commented Sep 23, 2023

  1. || () is the shortest form, a bit better than || view! { "" }
  2. I don't think it's possible to make this an optional prop without adding some overhead (making it a boxed function) or with some other proposal like FromFn trait to deal with prop conversion #1759. If you can find a way to implement it, I'm happy to take a PR.

What happens if you do prop(default=|| ()) ?
I don't see what error you would get ...

And I think if you use fn_into, you will have to box the function anyway so ...

@gbj
Copy link
Collaborator

gbj commented Sep 23, 2023

What happens if you do prop(default=|| ()) ? I don't see what error you would get ...

If you do that, and then provide some other closure as the fallback in an example, you get the error

error[E0308]: mismatched types
  --> /Users/gjohnston/Documents/Projects/leptos-main/leptos/leptos/src/show.rs:36:1
   |
36 | #[component]
   | ^^^^^^^^^^^^ expected type parameter `F`, found closure
37 | pub fn Show<F, W, IV>(
   |             - this type parameter
   |
   = note: expected type parameter `F`
                     found closure `[closure@/.../leptos/src/show.rs:36:1: 36:13]`
   = help: every closure has a distinct type and so could not always match the caller-chosen type of parameter `F`
   = note: this error originates in the derive macro `::leptos::typed_builder_macro::TypedBuilder` (in Nightly builds, run with -Z macro-backtrace for more info)

In other words, it causes a type error because || "oops" and || () are not the same type.

@maccesch
Copy link
Contributor

maccesch commented Sep 30, 2023

I played around with this a bit because it was bothering me way more than is reasonable. I think it can be solved with sth like Rc<dyn Fn()>. This of course comes at the cost of a tiny bit of performance but I suspect that compared the DOM manipulation this is negligible.

@gbj If this sounds acceptable to you I can create a PR

I believe with a bit of new-typery and a #[prop(into)] this could be a non-breaking change.

@gbj
Copy link
Collaborator

gbj commented Sep 30, 2023

@maccesch That's fine with me.

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

Successfully merging a pull request may close this issue.

4 participants