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

feat: Support generic components in rsx!() macro #385

Merged
merged 10 commits into from
Jun 11, 2022

Conversation

MuhannadAlrusayni
Copy link

No description provided.

@jkelleyrtp
Copy link
Member

jkelleyrtp commented May 2, 2022

This is awesome - I'm excited to see it!

However I can't seem to get it working properly. :(

Also, I would imagine that the correct syntax would be Component::<Generics> since we're working with function calls.

For reference, the two ways of specifying explicit type parameters in Rust are:

// for functions
let _ = eat::<MyThing>();

// for structs
let _ = MyStruct::<MyThing> { _p: PhantomData };

Can you add an example to the list of examples that showcases how this is supposed to work?

use dioxus::prelude::*;

fn main() {}

fn app(cx: Scope) -> Element {
    cx.render(rsx! {
        div {
            // I tried this
            Child<MyThing> {}

            // I also tried this
            Child::<MyThing> {}
        }
    })
}

trait Message {
    const MSG: &'static str;
}

struct MyThing {}
impl Message for MyThing {
    const MSG: &'static str = "hello";
}

#[inline_props]
fn Child<T: Message>(cx: Scope) -> Element {
    let me = <T as Message>::MSG;
    cx.render(rsx! {
        div {
            p {
                "Hello {me}"
            }
        }
    })
}

I get these errors:

error[E0658]: associated type bounds are unstable
  --> examples/generic_components.rs:23:10
   |
23 | fn Child<T: Message>(cx: Scope) -> Element {
   |          ^^^^^^^^^^
   |
   = note: see issue #52662 <https://github.com/rust-lang/rust/issues/52662> for more information

error[E0107]: this struct takes 1 generic argument but 0 generic arguments were supplied
  --> examples/generic_components.rs:22:1
   |
22 | #[inline_props]
   | ^^^^^^^^^^^^^^^ expected 1 generic argument
   |
note: struct defined here, with 1 generic parameter: `T`
  --> examples/generic_components.rs:22:1
   |
22 | #[inline_props]
   | ^^^^^^^^^^^^^^^
23 | fn Child<T: Message>(cx: Scope) -> Element {
   |          -
   = note: this error originates in the attribute macro `inline_props` (in Nightly builds, run with -Z macro-backtrace for more info)
help: add missing generic argument
   |
23 | fn Child<T, T: Message>(cx: Scope) -> Element {
   |          ++

error[E0229]: associated type bindings are not allowed here
  --> examples/generic_components.rs:23:10
   |
23 | fn Child<T: Message>(cx: Scope) -> Element {
   |          ^^^^^^^^^^ associated type not allowed here

Some errors have detailed explanations: E0107, E0229, E0658.
For more information about an error, try `rustc --explain E0107`.
error: could not compile `dioxus` due to 3 previous errors
warning: build failed, waiting for other jobs to finish...
warning: field is never read: `text`
   --> examples/rsx_usage.rs:225:35
    |
225 | fn with_inline<'a>(cx: Scope<'a>, text: &'a str) -> Element {
    |                                   ^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default

warning: `dioxus` (example "rsx_usage") generated 1 warning

@MuhannadAlrusayni
Copy link
Author

MuhannadAlrusayni commented May 4, 2022

Ops looks like I forget to do it for #[inline_props], I'll update my PR to add support generic there, thinking about it.

Now when I think about it, it will won't be funny task to do it for #[inline_props] because I need to PhantomData for generic types that is not used by the function parameters..


Anyway I was testing with Props struct style, here is a working example:

// src/testing.rs
use core::fmt;
use std::{marker::PhantomData, str::FromStr};

use css_style::style;
use dioxus::prelude::*;
use dioxus_layout::Column;

#[derive(Props)]
pub struct Props<T: 'static> {
    #[props(default)]
    phantom: PhantomData<T>,
}

#[allow(non_snake_case)]
pub fn MyInput<'a, T>(cx: Scope<'a, Props<T>>) -> Element
where
    T: FromStr + fmt::Display + Clone + 'static,
    <T as FromStr>::Err: std::fmt::Display,
{
    // states
    let is_hover = use_state(&cx, || false);
    let is_focus = use_state(&cx, || false);
    let error: &UseState<Option<T::Err>> = use_state(&cx, || None);

    // styles
    let inner = style();
    let container = style();
    let error_style = style();

    // elements
    let input = rsx!(input {
        style: "{inner}",
        oninput: move |event| {
            match T::from_str(&event.value) {
                Ok(_) => error.set(None),
                Err(err) => error.set(Some(err)),
            }
        },
    });

    cx.render(rsx!(
        Column {
            div {
                style: "{container}",
                onfocusin: move |_| is_focus.set(true),
                onfocusout: move |_| is_focus.set(false),
                onmouseover: move |_| is_hover.set(true),
                onmouseout: move |_| is_hover.set(false),

                input
            }
            error.get().as_ref().map(|err| {
                rsx!(p {
                    style: "{error_style}",
                    "{err}"
                })
            })
        }
    ))
}

and here is how how to use it:

    rsx! {
        MyInput<testing::Props<Url>> {}
    };

Also, I would imagine that the correct syntax would be Component:: since we're working with function calls.

True, but for some reason I used the style when we crate types:

struct Input<T> { .. }
       ^^^^^^^^

I'll change it to be rsx! { Input::<T> {} }


Can you add an example to the list of examples that showcases how this is supposed to work?

Sure, will do once we got stable API/Design.

@MuhannadAlrusayni
Copy link
Author

now generic components is supported in #[inline_props], @jkelleyrtp can you test it and see if there is any missing things ?

the only left parts are docs, unit tests and examples, they will follow once you approve last changes, I also left some messages on Discord regarding these things, would you take look on them.

@jkelleyrtp
Copy link
Member

I can add the docs and examples added - looks good to merge now. I'm going to run through a few tests and then merge it if all looks good.

@MuhannadAlrusayni
Copy link
Author

Sorry I'm kinda slow doing this PR, I try to work on it when i have free time :D.

I have added a test in example/rsx_usage.rs, you can merge this PR and add the docs if you like.

I just want to mention that it would be great if we make the docs part of our tests, that would make the docs up-to-date, correct and we cover more test.

@MuhannadAlrusayni
Copy link
Author

I have reused example/rsx_usage.rs as docs for rsx! macro, if that's okay with you you can merge it.

@MuhannadAlrusayni
Copy link
Author

Hi @jkelleyrtp, I consider this PR done from my side, if you have comment on this or we can merge ?

Muhannad Alrusayni added 10 commits June 4, 2022 13:40
before that, we accepted this style `Input<..>` but to be consistent
with Rust syntax this is dropped now.

> For reference, the two ways of specifying explicit type parameters in
> Rust are:
> // for functions
> let _ = foo::<MyType>();
>
> // for structs
> let _ = Foo::<MyType> { ... };
>
> by @jkelleyrtp
previously I from some reason I thought this not allowed syntax. Some test
failed because of my misunderstood, so now I fix this :D
Copy link
Member

@jkelleyrtp jkelleyrtp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exciting! Thank you! Sorry for the wait - I left for vacation. When checks pass again I will merge :)

@jkelleyrtp jkelleyrtp merged commit 03973f6 into DioxusLabs:master Jun 11, 2022
@jkelleyrtp jkelleyrtp mentioned this pull request Jun 11, 2022
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

Successfully merging this pull request may close these issues.

2 participants