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

Ensure html_nested returns the inner type #3493

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

Conversation

ranile
Copy link
Member

@ranile ranile commented Oct 27, 2023

Description

At some point in time, html_nested broke and started returning VNode instead of inner types. This PR fixes that

Checklist

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

@ranile ranile added bug breaking change A-yew-macro Area: The yew-macro crate labels Oct 27, 2023
@ranile ranile requested review from cecton and futursolo October 27, 2023 16:26
github-actions[bot]
github-actions bot previously approved these changes Oct 27, 2023
github-actions[bot]
github-actions bot previously approved these changes Oct 27, 2023
@github-actions
Copy link

github-actions bot commented Oct 27, 2023

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

https://yew-rs-api--pr3493-html-nested-types-6gu7o8xf.web.app

(expires Sat, 04 Nov 2023 14:53:46 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Oct 27, 2023

Benchmark - core

Yew Master

vnode           fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ vnode_clone  4.972 ns      │ 5.837 ns      │ 4.992 ns      │ 5.011 ns      │ 100     │ 1000000000

Pull Request

vnode           fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ vnode_clone  4.567 ns      │ 4.683 ns      │ 4.601 ns      │ 4.61 ns       │ 100     │ 1000000000

@ranile
Copy link
Member Author

ranile commented Oct 27, 2023

Compile failures will be resolved by #3453

@github-actions
Copy link

github-actions bot commented Oct 27, 2023

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 103.360 103.330 -0.030 -0.029%
boids 176.817 176.971 +0.153 +0.087%
communication_child_to_parent 95.831 95.892 +0.061 +0.063%
communication_grandchild_with_grandparent 109.030 109.072 +0.042 +0.039%
communication_grandparent_to_grandchild 105.435 105.405 -0.029 -0.028%
communication_parent_to_child 93.314 93.312 -0.002 -0.002%
contexts 112.714 112.714 0 0.000%
counter 90.002 90.148 +0.146 +0.163%
counter_functional 90.663 90.706 +0.043 +0.047%
dyn_create_destroy_apps 93.225 93.164 -0.061 -0.065%
file_upload 104.261 104.185 -0.076 -0.073%
function_memory_game 175.886 175.994 +0.108 +0.062%
function_router 353.806 355.347 +1.541 +0.436%
function_todomvc 164.230 164.266 +0.035 +0.021%
futures 232.174 232.199 +0.025 +0.011%
game_of_life 113.148 113.432 +0.283 +0.250%
immutable 190.273 190.730 +0.457 +0.240%
inner_html 86.320 86.320 0 0.000%
js_callback 113.781 113.809 +0.027 +0.024%
keyed_list 202.673 202.959 +0.286 +0.141%
mount_point 89.213 89.251 +0.038 +0.043%
nested_list 117.399 117.490 +0.091 +0.077%
node_refs 96.923 96.962 +0.039 +0.040%
password_strength 1753.925 1754.027 +0.103 +0.006%
portals 98.070 98.211 +0.141 +0.143%
router 322.340 322.961 +0.621 +0.193%
simple_ssr 144.692 144.714 +0.021 +0.015%
ssr_router 391.806 393.347 +1.541 +0.393%
suspense 120.140 120.423 +0.283 +0.236%
timer 92.549 92.706 +0.157 +0.170%
timer_functional 101.335 101.444 +0.109 +0.108%
todomvc 144.317 144.335 +0.018 +0.012%
two_apps 90.062 90.207 +0.145 +0.160%
web_worker_fib 138.945 139.095 +0.149 +0.108%
web_worker_prime 189.679 189.580 -0.099 -0.052%
webgl 88.930 88.930 0 0.000%

✅ None of the examples has changed their size significantly.

github-actions[bot]
github-actions bot previously approved these changes Oct 28, 2023
github-actions[bot]
github-actions bot previously approved these changes Oct 28, 2023
@github-actions
Copy link

github-actions bot commented Oct 28, 2023

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 357.159 359.485 357.664 0.924
Hello World 10 625.149 628.190 626.199 1.065
Function Router 10 2090.153 2105.024 2095.063 4.812
Concurrent Task 10 1007.005 1008.658 1007.782 0.472
Many Providers 10 1421.930 1440.086 1428.159 5.442

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 319.190 319.451 319.300 0.084
Hello World 10 619.606 620.755 620.001 0.355
Function Router 10 2077.102 2084.615 2081.952 2.199
Concurrent Task 10 1006.411 1009.354 1007.848 0.846
Many Providers 10 1435.146 1465.728 1446.473 10.042

@futursolo
Copy link
Member

I feel this is an expected behaviour.

See: https://docs.rs/yew/latest/yew/macro.html_nested.html

This macro is similar to html!, but preserves the component type instead of wrapping it in Html.

The documentation suggested that only <Comp /> is preserved as VChild<Comp>.

Returning other types would encourage inspection of VNode, which I am not in favour of.

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 breaking change bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants