render-rs / render.rs

🔏 A safe and simple template engine with the ergonomics of JSX
https://docs.rs/render
MIT License
238 stars 23 forks source link

Use Cow for element attribute values #21

Closed vpzomtrrfrt closed 4 years ago

vpzomtrrfrt commented 4 years ago

I think this should help with using owned values (e.g. from format!) without requiring unnecessary clones

daaku commented 4 years ago

This is good, and probably could use the same for children too? Would be good to add some test cases:

#[test]
fn owned_string() {
    use pretty_assertions::assert_eq;
    use render::{component, html, rsx};

    #[component]
    fn Welcome<'kind, 'name>(kind: &'kind str, name: &'name str) {
        rsx! {
          <h1 class={format!("{}-title", kind)}>
            {format!("Hello, {}", name)}
          </h1>
        }
    }

    assert_eq!(
        html! { <Welcome kind={"alien"} name={"Yoda"} /> },
        r#"<h1 class="alien-title">Hello, Yoda</h1>"#
    );
}

Maybe split the support for children into another commit if it makes sense.

Schniz commented 4 years ago

I'm not familiar with Cow enough. But like @daaku said, let's add a test to verify that behavior? 😃

vpzomtrrfrt commented 4 years ago

Added test from comment

daaku commented 4 years ago

Not sure why github actions are not testing it, but I was expecting the test to not pass as-is since children support isn't built yet.. Does it work for you?

If it doesn't, I suggest removing the part of the test that tests for Cow children and merge this for attributes to start with.

vpzomtrrfrt commented 4 years ago

The test passes, so I guess children already work