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

Add VNode::from_html_unchecked #2842

Merged
merged 30 commits into from Nov 8, 2022
Merged

Conversation

hamza1311
Copy link
Member

@hamza1311 hamza1311 commented Aug 24, 2022

Description

Add method:

impl VNode {
     fn from_html_unchecked(html: &str) -> Self { ... }
}

Fixes #2841

Checklist

  • I have reviewed my own code
  • I have added tests

@hamza1311 hamza1311 added the A-yew Area: The main yew crate label Aug 24, 2022
@hamza1311 hamza1311 added this to the v0.20 milestone Aug 24, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 24, 2022
@hamza1311
Copy link
Member Author

I can't get any tests for this to pass:

#[cfg(all(test, not(target_arch = "wasm32")))]
mod tests {
    use super::*;
    use crate::html;

    #[test]
    fn from_raw_html_works() {
        let vnode = html! {
            <><div>{"c"}</div></>
        };

        eprintln!("{:#?}", vnode);

        let from_raw = VNode::from_raw_html("<div>c</div>");
        eprintln!();
        eprintln!("{:#?}", from_raw);

        assert_eq!(vnode, from_raw);
    }
}
Test Output
running 1 test
test virtual_dom::vnode::tests::from_raw_html_works ... FAILED

failures:

---- virtual_dom::vnode::tests::from_raw_html_works stdout ----
VList {
    children: [
        VTag {
            inner: Other {
                tag: "div",
                children: VList {
                    children: [
                        VText { text: "c" },
                    ],
                    fully_keyed: KnownMissingKeys,
                    key: None,
                },
            },
            listeners: None,
            node_ref: NodeRef { references: None },
            attributes: Static(
                [],
            ),
            key: None,
        },
    ],
    fully_keyed: KnownMissingKeys,
    key: None,
}

VList {
    children: [
        VTag {
            inner: Other {
                tag: "div",
                children: VList {
                    children: [
                        VText { text: "c" },
                    ],
                    fully_keyed: KnownMissingKeys,
                    key: None,
                },
            },
            listeners: None,
            node_ref: NodeRef { references: None },
            attributes: Static(
                [],
            ),
            key: None,
        },
    ],
    fully_keyed: KnownMissingKeys,
    key: None,
}
thread 'virtual_dom::vnode::tests::from_raw_html_works' panicked at 'assertion failed: `(left == right)`
  left: `VList { children: [VTag { inner: Other { tag: "div", children: VList { children: [VText { text: "c" }], fully_keyed: KnownMissingKeys, key: None } }, listeners: None, node_ref: NodeRef { references: None }, attributes: Static([]), key: None }], fully_keyed: KnownMissingKeys, key: None }`,
 right: `VList { children: [VTag { inner: Other { tag: "div", children: VList { children: [VText { text: "c" }], fully_keyed: KnownMissingKeys, key: None } }, listeners: None, node_ref: NodeRef { references: None }, attributes: Static([]), key: None }], fully_keyed: KnownMissingKeys, key: None }`', packages/yew/src/virtual_dom/vnode.rs:288:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I don't understand why exactly this test fails

@github-actions
Copy link

github-actions bot commented Aug 24, 2022

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 371.549 372.557 371.751 0.292
Hello World 10 707.307 716.183 709.373 2.887
Function Router 10 2344.654 2380.579 2361.433 9.339
Concurrent Task 10 1007.340 1009.305 1008.532 0.636

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 393.117 393.747 393.393 0.204
Hello World 10 709.091 722.206 714.874 4.073
Function Router 10 2332.463 2365.355 2349.607 9.417
Concurrent Task 10 1007.499 1009.495 1008.602 0.549

@github-actions
Copy link

github-actions bot commented Aug 24, 2022

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 106.139 108.607 +2.469 +2.326%
boids 170.366 172.852 +2.485 +1.459%
communication_child_to_parent 90.027 92.507 +2.479 +2.754%
communication_grandchild_with_grandparent 104.879 107.219 +2.340 +2.231%
communication_grandparent_to_grandchild 100.744 103.092 +2.348 +2.330%
communication_parent_to_child 87.134 89.614 +2.480 +2.847%
contexts 107.358 109.881 +2.522 +2.350%
counter 85.050 87.520 +2.470 +2.904%
counter_functional 85.522 88.000 +2.478 +2.897%
dyn_create_destroy_apps 87.882 90.355 +2.474 +2.815%
file_upload 99.645 102.067 +2.423 +2.431%
function_memory_game 162.865 165.333 +2.468 +1.515%
function_router 347.254 349.764 +2.510 +0.723%
function_todomvc 157.658 160.150 +2.492 +1.581%
futures 221.943 224.211 +2.268 +1.022%
game_of_life 104.550 107.018 +2.468 +2.360%
immutable 180.990 184.614 +3.624 +2.002%
inner_html 81.913 84.126 +2.213 +2.702%
js_callback 110.877 113.428 +2.551 +2.301%
keyed_list 195.523 198.045 +2.521 +1.290%
mount_point 84.658 87.130 +2.472 +2.920%
nested_list 111.624 114.168 +2.544 +2.279%
node_refs 92.534 95.009 +2.475 +2.674%
password_strength 1546.897 1549.191 +2.294 +0.148%
portals 95.945 98.518 +2.572 +2.681%
router 317.068 319.561 +2.492 +0.786%
simple_ssr 151.578 154.110 +2.532 +1.671%
ssr_router 392.338 394.761 +2.423 +0.618%
suspense 108.617 111.172 +2.555 +2.352%
timer 87.913 90.381 +2.468 +2.807%
todomvc 138.961 141.444 +2.483 +1.787%
two_apps 85.714 88.190 +2.477 +2.889%
web_worker_fib 152.032 154.515 +2.482 +1.633%
webgl 84.350 86.846 +2.496 +2.959%

⚠️ The following examples have changed their size significantly:

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 106.139 108.607 +2.469 +2.326%
boids 170.366 172.852 +2.485 +1.459%
communication_child_to_parent 90.027 92.507 +2.479 +2.754%
communication_grandchild_with_grandparent 104.879 107.219 +2.340 +2.231%
communication_grandparent_to_grandchild 100.744 103.092 +2.348 +2.330%
communication_parent_to_child 87.134 89.614 +2.480 +2.847%
contexts 107.358 109.881 +2.522 +2.350%
counter 85.050 87.520 +2.470 +2.904%
counter_functional 85.522 88.000 +2.478 +2.897%
dyn_create_destroy_apps 87.882 90.355 +2.474 +2.815%
file_upload 99.645 102.067 +2.423 +2.431%
function_memory_game 162.865 165.333 +2.468 +1.515%
function_todomvc 157.658 160.150 +2.492 +1.581%
futures 221.943 224.211 +2.268 +1.022%
game_of_life 104.550 107.018 +2.468 +2.360%
immutable 180.990 184.614 +3.624 +2.002%
inner_html 81.913 84.126 +2.213 +2.702%
js_callback 110.877 113.428 +2.551 +2.301%
keyed_list 195.523 198.045 +2.521 +1.290%
mount_point 84.658 87.130 +2.472 +2.920%
nested_list 111.624 114.168 +2.544 +2.279%
node_refs 92.534 95.009 +2.475 +2.674%
portals 95.945 98.518 +2.572 +2.681%
simple_ssr 151.578 154.110 +2.532 +1.671%
suspense 108.617 111.172 +2.555 +2.352%
timer 87.913 90.381 +2.468 +2.807%
todomvc 138.961 141.444 +2.483 +1.787%
two_apps 85.714 88.190 +2.477 +2.889%
web_worker_fib 152.032 154.515 +2.482 +1.633%
webgl 84.350 86.846 +2.496 +2.959%

github-actions[bot]
github-actions bot previously approved these changes Aug 24, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 24, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 24, 2022
packages/yew/src/virtual_dom/vnode.rs Outdated Show resolved Hide resolved
packages/yew/Cargo.toml Outdated Show resolved Hide resolved
github-actions[bot]
github-actions bot previously approved these changes Aug 27, 2022
@github-actions
Copy link

github-actions bot commented Aug 27, 2022

Visit the preview URL for this PR (updated for commit 4850d52):

https://yew-rs-api--pr2842-html-from-raw-v3ysmy42.web.app

(expires Sun, 13 Nov 2022 16:20:14 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

github-actions[bot]
github-actions bot previously approved these changes Aug 27, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 27, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 28, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 28, 2022
Copy link
Member

@WorldSEnder WorldSEnder left a comment

Choose a reason for hiding this comment

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

I would like to see an explanation for the increase in bundle size for the examples not using inner html. Other than that, I think reusing the vlist/blist implementation could save a bit of duplicated code.

packages/yew/src/dom_bundle/braw.rs Show resolved Hide resolved
packages/yew/src/dom_bundle/braw.rs Outdated Show resolved Hide resolved
@hamza1311 hamza1311 changed the title Add VNode::html_from_raw Add VNode::from_html_unchecked Nov 6, 2022
github-actions[bot]
github-actions bot previously approved these changes Nov 6, 2022
github-actions[bot]
github-actions bot previously approved these changes Nov 6, 2022
Copy link
Member

@WorldSEnder WorldSEnder left a comment

Choose a reason for hiding this comment

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

Test coverage seems good, and I don't see any more issues with the code.

Figuring out why this increases the bundle size is not blocking imo, even if it would be nice not to pay the additional 2kB.

@hamza1311
Copy link
Member Author

I couldn't figure out anything regarding size increase. If I had to guess, maybe it's because yew now enables more web-sys features? Not quite sure. 2KB isn't much, especially with how .wasm files compress.

I'm going to go ahead and merge this

@hamza1311 hamza1311 merged commit 90c7ff1 into yewstack:master Nov 8, 2022
@dmeijboom dmeijboom mentioned this pull request Nov 17, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew Area: The main yew crate breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

raw HTML in a component with SSR
2 participants