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

Dynamic Prop Labels #3509

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

kirillsemyonkin
Copy link
Contributor

@kirillsemyonkin kirillsemyonkin commented Nov 1, 2023

Description

Partially implements features from discussion #3477. This PR adds Dynamic Prop Labels, so that anyone can write their favorite attributes like HTMX's hx-on:click:

html! {
    <div "hx-on:click"="alert('Clicked!')">{ "Click" }</div>
    <div { "hx-on:click" }={ "alert('Clicked!')" }>{ "Click" }</div>
}

It works by using new PropLabel enum everywhere, which can be either a Static HtmlDashedName like before, or Dynamic Expr. When parsing, if it meets = token after the expression group, it will read a value following it instead of assuming that it is shorthand syntax.

Checklist

  • I have reviewed my own code - what does that mean? Is this always checked for everyone?
  • I have added tests

Copy link

github-actions bot commented Nov 1, 2023

Visit the preview URL for this PR (updated for commit 5fc9058):

https://yew-rs-api--pr3509-dyn-prop-0vcn3vrd.web.app

(expires Mon, 04 Dec 2023 08:06:38 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@kirillsemyonkin
Copy link
Contributor Author

kirillsemyonkin commented Nov 1, 2023

My local setup told me nothing about which Rust version is used, wow...

Fixing 1.64.0 compiler support shortly I guess, there is nothing special, just Result::is_ok_and.

Copy link

github-actions bot commented Nov 1, 2023

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 100.292 100.292 0 0.000%
boids 173.724 173.724 0 0.000%
communication_child_to_parent 92.738 92.738 0 0.000%
communication_grandchild_with_grandparent 105.761 105.761 0 0.000%
communication_grandparent_to_grandchild 101.012 101.012 0 0.000%
communication_parent_to_child 89.088 89.088 0 0.000%
contexts 106.004 106.004 0 0.000%
counter 86.120 86.120 0 0.000%
counter_functional 86.515 86.515 0 0.000%
dyn_create_destroy_apps 88.930 88.930 0 0.000%
file_upload 99.988 99.988 0 0.000%
function_memory_game 172.357 172.357 0 0.000%
function_router 348.660 348.660 0 0.000%
function_todomvc 161.180 161.180 0 0.000%
futures 229.034 229.034 0 0.000%
game_of_life 110.118 110.118 0 0.000%
immutable 185.424 185.424 0 0.000%
inner_html 79.896 79.896 0 0.000%
js_callback 109.511 109.511 0 0.000%
keyed_list 199.562 199.562 0 0.000%
mount_point 82.784 82.784 0 0.000%
nested_list 113.870 113.870 0 0.000%
node_refs 90.325 90.325 0 0.000%
password_strength 1750.046 1750.046 0 0.000%
portals 93.458 93.458 0 0.000%
router 317.565 317.565 0 0.000%
simple_ssr 140.313 140.313 0 0.000%
ssr_router 385.893 385.893 0 0.000%
suspense 115.705 115.705 0 0.000%
timer 88.555 88.555 0 0.000%
timer_functional 97.918 97.918 0 0.000%
todomvc 141.333 141.333 0 0.000%
two_apps 85.821 85.821 0 0.000%
web_worker_fib 134.756 134.756 0 0.000%
web_worker_prime 184.976 184.976 0 0.000%
webgl 82.500 82.500 0 0.000%

✅ None of the examples has changed their size significantly.

Copy link

github-actions bot commented Nov 1, 2023

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 289.358 295.919 290.165 2.026
Hello World 10 504.055 564.258 515.305 18.384
Function Router 10 1605.011 1623.532 1613.642 5.706
Concurrent Task 10 1005.584 1006.625 1006.134 0.373
Many Providers 10 1113.755 1164.546 1137.840 17.422

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 289.261 289.772 289.505 0.156
Hello World 10 476.348 495.162 479.349 5.678
Function Router 10 1626.899 1648.855 1634.601 8.137
Concurrent Task 10 1004.922 1006.348 1005.895 0.485
Many Providers 10 1124.838 1165.348 1143.790 12.075

@kirillsemyonkin
Copy link
Contributor Author

Uh sorry, will check what it fails on tomorrow

@kirillsemyonkin
Copy link
Contributor Author

Wow, I passed even the benchmark lol
I still need help with writing tests for this.

Allows not only literals, e.g. "hx-on:click", but all expressions
@ranile
Copy link
Member

ranile commented Nov 2, 2023

You need both passing and failing tests for it. You can copy a -fail.rs and -pass.rs from yew-macro/tests/html_macro and modify it as needed. To regenerate stderr files, you can run tests with TRYBUILD=overwrite environment variable

@kirillsemyonkin
Copy link
Contributor Author

If there are any // FIXMEs in my PR, I also need them reviewed

cecton
cecton previously approved these changes Nov 6, 2023
Copy link
Member

@cecton cecton left a comment

Choose a reason for hiding this comment

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

Awesome! I need this. At the moment I'm building the component linearGradient manually and it's such a pain

#[function_component(LinearGradient)]
pub fn linear_gradient(
    Props {
        id,
        gradient_transform,
        children,
    }: &Props,
) -> Html {
    let mut vtag = yew::virtual_dom::VTag::new("linearGradient");
    vtag.add_children(children.clone());
    vtag.add_attribute("id", id.clone());
    if let Some(x) = gradient_transform.clone() {
        vtag.add_attribute("gradientTransform", x);
    }
    vtag.into()
}

Now with autoprops + dyn prop labels, I can simplify this code a lot

packages/yew-macro/tests/html_macro/dyn-prop-fail.stderr Outdated Show resolved Hide resolved
@kirillsemyonkin
Copy link
Contributor Author

kirillsemyonkin commented Nov 6, 2023

I tried to add following to the dyn-prop-pass.rs test:

    // property literal
    _ = ::yew::html! { <span ~"hx-on:click"="alert('Clicked!')" /> };
    _ = ::yew::html! { <span ~"hx-on:click"="alert('Clicked!')" ~"hx-on:click"="alert('Clicked!')" /> };
    _ = ::yew::html! { <span ~{ "hx-on:click" }={ "alert('Clicked!')" } /> };
    _ = ::yew::html! { <span ~{ "hx-on:click" }={ "alert('Clicked!')" } ~{ "hx-on:click" }={ "alert('Clicked!')" } /> };

    // property expr
    _ = ::yew::html! { <span ~{ dyn_prop() }={ "alert('Clicked!')" } /> };
    _ = ::yew::html! { <span ~{ dyn_prop() }={ "alert('Clicked!')" } ~{ dyn_prop() }={ "alert('Clicked!')" } /> };

Tests (RUST_BACKTRACE=1 cargo +1.64.0 test -p yew-macro) greeted me with the following:

test tests\html_macro\dyn-prop-pass.rs [should pass] ... error
Test case failed at runtime.

STDERR:
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
thread 'main' panicked at 'function not implemented on non-wasm32 targets', D:\RustDownloads\cargo\registry\src\github.com-1ecc6299db9ec823\wasm-bindgen-0.2.87\src\lib.rs:996:1       
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library\std\src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library\core\src/panicking.rs:142:14
   2: core::panicking::panic
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library\core\src/panicking.rs:48:5
   3: wasm_bindgen::__wbindgen_string_new
             at D:\RustDownloads\cargo\registry\src\github.com-1ecc6299db9ec823\wasm-bindgen-0.2.87\src\lib.rs:41:17
   4: wasm_bindgen::JsValue::from_str
             at D:\RustDownloads\cargo\registry\src\github.com-1ecc6299db9ec823\wasm-bindgen-0.2.87\src\lib.rs:132:32
   5: <wasm_bindgen::JsValue as core::convert::From<&str>>::from
             at D:\RustDownloads\cargo\registry\src\github.com-1ecc6299db9ec823\wasm-bindgen-0.2.87\src\lib.rs:776:9
   6: <T as core::convert::Into<U>>::into
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52\library\core\src\convert/mod.rs:550:9
   7: trybuild005::main
             at D:\RustProjects\yew\packages\yew-macro\tests\html_macro\dyn-prop-pass.rs:58:30
   8: core::ops::function::FnOnce::call_once
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52\library\core\src\ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈

As I was trying to find out whether any tests would help me figure out how to test the feature, I performed a quick search on all tests, only to find out that nobody tested ~ properties. I will await on someone testing and fixing the feature before adding tests like this. (Edit: This is not blocking)

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.

3 participants