leptos icon indicating copy to clipboard operation
leptos copied to clipboard

Support non-literal defaults with `#[prop(default=...)]`

Open ModProg opened this issue 2 years ago • 7 comments

After some investigation, it turned out that this is in fact a missing feature of the attribute parsing, we currently do not support non-literal defaults due to only supporting meta attributes.

struct A;

#[component]
fn CustomButton(
  cx: Scope,
  #[prop(default=A)] non_default: A,
  #[prop(default=true)] default: bool) -> impl IntoView {}

non_default will not be optional even though a default is provided, while default will be.

It T implements Default should not be relevant here.

ModProg avatar Feb 04 '23 13:02 ModProg

I think this is at least in part an XY problem. Are you actually trying to do this with NonZeroUsize or another type?

There are two issues in the example:

  1. The macro is choking on default=8.into() because it takes a single value, not an expression. default = 8 parses fine.
  2. NonZeroUsize doesn't implement From<{integer}>, so you can't actually call 8.into() and get a NonZeroUsize, or NonZeroUsize::from(8).

gbj avatar Feb 04 '23 13:02 gbj

Oh well then it is at least a dx problem, we should error when failing to parse the expression given to default.

And on top of that, how would you provide a non-literal value to default?

default not supporting expressions actually "solves" this issue, because every type with a literal implements Default.

Speaking of, this is also not ideal error messaging:

error: only `optional`, `optional_no_strip`, `strip_option`, `default` and `into` are allowed as arguments to `#[prop()]`
  --> src/main.rs:35:10
   |
35 |   #[prop(default(NonZeroUsize))] non_default: std::num::NonZeroUsize,
   |

because the used attribute is default it is just invoked incorrectly. But that would probably be a seperate issue.

ModProg avatar Feb 04 '23 14:02 ModProg

I updated the example to not use NonZeroUsize but as the default attribute just silently does nothing on non literals, this actually doesn't work either.

ModProg avatar Feb 04 '23 14:02 ModProg

Maybe we could use a crate for attribute parsing, that allows easy parsing of non-meta attributes? Such as (self plug, but there are also others) https://docs.rs/attribute-derive.

But we can also do it without adding a new dependency.

ModProg avatar Feb 04 '23 14:02 ModProg

I'd guess, based on that crate, that you know much more about attribute macros than I do. (I didn't write the current implementation of the component macro, or the default = ... implementation) A PR would be welcome to handle expressions, rather than only literals, in default = ...

Here are the relevant pieces...

Definition of PropOpt https://github.com/leptos-rs/leptos/blob/4034aa9c112ec036d332d40ec420c6169c0fd75f/leptos_macro/src/component.rs#L394-L401

Parsing https://github.com/leptos-rs/leptos/blob/4034aa9c112ec036d332d40ec420c6169c0fd75f/leptos_macro/src/component.rs#L435-L449

This will need to stringify the expression and use TypedBuilder default_code instead of default https://github.com/leptos-rs/leptos/blob/4034aa9c112ec036d332d40ec420c6169c0fd75f/leptos_macro/src/component.rs#L489-L490

gbj avatar Feb 04 '23 15:02 gbj

P.S. I'm fine with you pulling in your other crate to do it, as it's a compile-time dependency without runtime implications.

gbj avatar Feb 04 '23 15:02 gbj

I'll have a look later. One short win would be https://github.com/leptos-rs/leptos/pull/469 as that would add an error on invalid syntax rather than staying silent.

ModProg avatar Feb 04 '23 15:02 ModProg

@ModProg are you still interested in working on this?

gbj avatar Feb 24 '23 00:02 gbj

yes, I should get on it next week

ModProg avatar Feb 24 '23 01:02 ModProg

create a pr, but it has a random "out of disc space" error which I'm not sure how related to my changes

ModProg avatar Mar 02 '23 18:03 ModProg