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

Ergonomic pattern for passing along on* callbacks to children #1533

Open
1 of 3 tasks
thedodd opened this issue Aug 29, 2020 · 18 comments
Open
1 of 3 tasks

Ergonomic pattern for passing along on* callbacks to children #1533

thedodd opened this issue Aug 29, 2020 · 18 comments
Labels
A-yew-macro Area: The yew-macro crate feature-request A feature request

Comments

@thedodd
Copy link
Contributor

thedodd commented Aug 29, 2020

Describe the feature you'd like
I am in the process of creating a Bulma component library for Yew. For many of the components, users will need to attached callback handlers to respond to events. There are A LOT of different event types, and I'm looking for a way to be able to collect any on* event handlers attached to the component, and pass them down to the component's concrete HTML elements (where the DOM events actually take place).

Is your feature request related to a problem? Please describe. (Optional)
The problem is that I would have to create a very large amount of code on almost all of the components to be able to pass along these on* event handlers. Not impossible ... but definitely seems like an anti-pattern.

Describe alternatives you've considered (Optional)
I've looked into using https://github.com/deniskolodin/mixin, but the need for a generic type parameter seems to be causing issues with that approach.

Questionnaire

  • I'm interested in implementing this myself but don't know where to start
  • I would like to add this feature
  • I don't have time to add this right now, but maybe later

On that last note, if there is no straightforward way to handle this in Yew right now ... I'll probably just postpone the development of the component library :). It is primarily for a personal project, so I can move forward without finishing things.

Either way, I would love to implement this capability in Yew, just need to hear from those of you that know the code base the best on what design options we have here.

@thedodd thedodd added the feature-request A feature request label Aug 29, 2020
@siku2
Copy link
Member

siku2 commented Aug 29, 2020

That's something I've wanted for a really long time now. Not only for callbacks but for all kinds of props.
I've been thinking about using an approach similar to the serde(flatten) attribute which would allow something like this:

#[derive(Clone, Properties)]
struct ThemeProps {
  #[prop_or_default]
  pub text_color: Option<String>,
  #[prop_or_default]
  pub background_color: Option<String>,
}

#[derive(Clone, Properties)]
struct MyListComponentProps {
  pub children: Children,
  #[props_inherit]
  pub theme: ThemeProps,
}

html! {
  <MyListComponent text_color="maybe green or something idk">
    <span>{ "Hello World" }</span>
  </MyListComponent>
}

This way we can adhere to "composition over inheritance" but still have some syntactic sugar.
What do you think?

@thedodd
Copy link
Contributor Author

thedodd commented Aug 29, 2020

@siku2 nice, I like that. Yea, the composition approach would be great. I suppose with an approach like this, it would mostly be modifications to the Properties derive macro? I've done a fair bit of proc-macro/derive macro work, so I'm definitely happy to help out!

@siku2
Copy link
Member

siku2 commented Aug 29, 2020

Yes, it needs a clever idea to make it work with the html! macro though, because it is completely unaware of the actual Properties struct. One idea I have is to use DerefMut on the builder for this.

Feel free to give this a shot. The relevant code is in the yew-macro directory. The derive macro for Properties can be found in the derive_props module and the part of the html! macro that handles components is in the html_components.rs file.

@jon-zu
Copy link

jon-zu commented Oct 23, 2020

I'm working on a bootstrap library for Yew right now and stumbled over the same issue. My idea to solve this would be to use - as a way to express the flattening which works kinda well because - is never allowed as Identifier. Each flattened props would require an unique prefix. It would look like this:

#[derive(Clone, Properties)]
struct SizingProps {
  #[prop_or_default]
  pub sm: Option<usize>,
}

#[derive(Clone, Properties)]
struct ThemeProps {
  #[prop_or_default]
  pub text_color: Option<String>,
  #[prop_or_default]
  pub background_color: Option<String>,
}

#[derive(Clone, Properties)]
struct MyListComponentProps {
  pub children: Children,
  #[flatten("tp")]
  pub theme: ThemeProps,
  #[flatten]
  pub sizing: SizingProps,
}

html! {
  <MyListComponent -sm=100 tp-text_color="maybe green or something idk">
    <span>{ "Hello World" }</span>
  </MyListComponent>
}

@Follpvosten
Copy link

For the original issue, the proposed syntax examples leave out an important detail: How would a MyListComponent implementation pass on all of the events to a child?
I'd guess something like with could be used:
<div with self.props.events />
But of course it would also need to support specifying other standard HTML attributes alongside that

<div
    class="something"
    with=self.props.events.clone()
    />

and that would also have to work for wrapped components, kinda like ref does.

Just throwing in some thoughts here. I'm currently cherry-picking just the events our internal project needs into yew-mdc, but that makes the list of events look a bit strange from the API perspective, so it would be great to have this at some point.

@lukechu10
Copy link
Contributor

I created a discussion to discuss potential solutions: #1779

@Follpvosten
Copy link

I've considered something like this, but why not for all kinds of attributes, not just event handlers?

I'm imagining a field similar to Children you can add to your properties - something like a rest: Attributes, which would catch all of the attributes that couldn't be mapped into the Properties, and you could then pass on to children using struct update syntax, like: <Child something="x" ..rest />

Would that be a possibility?

@LLBlumire
Copy link
Contributor

Having a rest: Attributes is a very desirable use case for writing thin wrappers where you want a component that simply sets some properties and otherwise forwards everything else to the element its constructing. I'm running into this here https://github.com/rpghorizon/yew-aria-widgets/blob/ccfefafa99b25a7623ce429f23bde881ce0b0e31/src/alert.rs#L40-L42 where currently I'm forwarding id and class but in an ideal world would be forwarding all attributes.

@futursolo
Copy link
Member

I think the issues with props spreading(..props) are:

  • requires a serde::Deserialize-style API, which can be difficult to implement and heavy on code generation.
  • spreaded fields are unknown at compile time, hence requires runtime panicking.

I think there're 2 ways to avoid runtime panicking:

  1. Limit spreadable to props that are #[prop_or_default] (so any uncovered props can be initialised with default).
  2. Add attribute to help developers to implement props for html elements and traits to convert to underlying props for elements.

Snippet for the second way:

#[derive(Properties, PartialEq, Debug)]
#[props_extend(button)] // now ButtonProps also implements all fields of `HtmlButtonElementProps`.
pub struct ButtonProps {
    // other props.
}

#[automatically_derived] // by #[props_extend(button)]
impl IntoElementProps<HtmlButtonElementProps> for ButtonProps {
    fn into_element_props() -> HtmlButtonElementProps { .. }
 }

#[function_component(Button)]
fn button(props: &ButtonProps) -> Html {
    // can call `.clone().into_element_props()` here if props need to be modified.
    html! { <button {..props} /> } // can only spread IntoElementProps<HtmlButtonElementProps> here.
}

@ranile
Copy link
Member

ranile commented Jan 5, 2022

I've been thinking about this and it seems that we would need to have a Default requirement for anything that can be extended.

#[props_extend(button)] // now ButtonProps also implements all fields of `HtmlButtonElementProps`.

The problem here is that the macro can not know the fields of HtmlButtonElementProps so we can't implement these. There has to be a way to initialize defaults and then modify. We could pull Deref hack similar to web-sys/js-sys to make the process easier

@futursolo
Copy link
Member

The problem here is that the macro can not know the fields of HtmlButtonElementProps so we can't implement these.

I think it can know the fields of HtmlButtonElementProps as it is props of the button element, not an arbitrary component. So it can be predefined in the macro itself.

There has to be a way to initialize defaults and then modify.

All attributes on elements should be optional hence a default value of None can be assumed.

@voidpumpkin
Copy link
Member

voidpumpkin commented Jan 19, 2022

After going through #2369 I really feel inheritance for props should not be considered for yew for now.
Maybe in coming years rust will get more features that allow to implement without that many drawbacks.

My view is that inheritance is a nice to have feature. We can launch 1.0 without it, we just need an alternative.
Thus a more practical solution for today is to make the following points more documented:

  • Yew users already can add props struct as a field in other component props, so that works:
#[derive(Clone, PartialEq, Properties)]
pub struct FormProps {
    pub label_props: LabelProps,
}
#[derive(Clone, PartialEq, Properties)]
pub struct LabelProps {
    pub color: String,
}
// pseudo Label component
html!{<div><label style={format!("color: {}",color)}>Cool input</label></div>}
  • If a lower component provides some modified props, we can use a callback:
#[derive(Clone, PartialEq, Properties)]
pub struct LabelProps {
    #[prop_or_else(create_label_default_html_prop)]
    pub create_label_html: Callback<(String,String),Html>,
}
fn create_label_default_html_prop() {
    Callback::from(|(color, text ):(String, String )| {
        html!{<label style={format!("color: {}",color)}>{text}</label>
    }
}

// pseudo Label component
// This way the user can later completely replace the whole html and lets say provide a custom yew label component
let label_html = create_label_html.emit(("red".to_string(), "Cool input".to_string() ));
html!{<div>{label_html}</div>}
  • And then we need way to pass dynamic props:
#[derive(Clone, PartialEq, Properties)]
pub struct LabelProps {
    #[prop_or(Attributes::Static(&[["style", "color: red"]]))]
    pub inner_label_props: Attributes,
}
// pseudo Label component
// we could use a more ergonomic solution to this
let mut label_vtag = VTag::new("label");
label_vtag.attributes = inner_label_props;
label_vtag.add_child(VText::new("Cool input").into());

html!{<div>{label_vtag}</div>}

So we would need to solve only the third one, and it should suffice.

@Follpvosten
Copy link

I personally don't think inheritance is what's wanted here, or that it's a solution to the problem at all.

I'm more wondering: If it's possible to have both a catch-all children type and a typed children type, why is it impossible to have a catch-all "rest of the attributes" type?

@voidpumpkin
Copy link
Member

@Follpvosten I see, sorry it seems I'm mixing up issues.

@Follpvosten
Copy link

@voidpumpkin No, sorry - I was answering to many comments before yours, not just yours. I was mostly saying that I think inheriting props from other components is not a solution to this problem, and there must be a simpler way imo.

@ranile
Copy link
Member

ranile commented Jan 19, 2022

@Follpvosten, after implementing inheritance, I agree with you.

The problem I see with catch-all prop (a special rest prop?) is that you lose type checking at compile time. The html! macro does not know if rest prop exists. The only way out that I see if we were to do the checking at runtime and panic! if in case of a mismatch.

@toadslop
Copy link

toadslop commented Aug 4, 2022

I have a solution to this problem that I'm working on. The proof of concept is done. The basic idea is that I use enums with tuple varients to define the different options for various HTML attributes and event listeners. These are then managed by structs. The user can add as many attributes and listeners to the structs as they want (they're collected in hashmaps), and then they're injected into the DOM in the rendered function. It kind of works outside of the core of Yew, but it gets the job done and allows users to pass down whatever they want dynamically, but you still get the benefits of typechecking. I'll get a working example ready for you guys if you want to take a look at it.

@filipdutescu
Copy link

@toadslop was actually looking for something just like this. Did you happen to finish that example you were talking about by any chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew-macro Area: The yew-macro crate feature-request A feature request
Projects
None yet
Development

No branches or pull requests