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

Hydration for VRaw #3245

Merged
merged 1 commit into from Apr 25, 2023
Merged

Conversation

dmeijboom
Copy link
Contributor

@dmeijboom dmeijboom commented Apr 21, 2023

Description

Implemented hydration for VRaw by re-using the Collectable workaround that the Suspense component also uses. Also used the DOMParser for parsing raw HTML into nodes.

As I don't really know what I'm doing I'd like for someone to verify whether this is correct. It does work for our application but that doesn't say much. Also, I can imagine that we want tests for this as well. Any ideas on that?

Fixes #2969

Checklist

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

@github-actions
Copy link

github-actions bot commented Apr 21, 2023

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

https://yew-rs-api--pr3245-feature-vraw-hydrati-m3nfegni.web.app

(expires Sun, 30 Apr 2023 08:33:36 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Apr 21, 2023

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 103.348 103.348 0 0.000%
boids 173.660 173.660 0 0.000%
communication_child_to_parent 95.847 95.847 0 0.000%
communication_grandchild_with_grandparent 107.813 107.813 0 0.000%
communication_grandparent_to_grandchild 104.125 104.125 0 0.000%
communication_parent_to_child 92.919 92.919 0 0.000%
contexts 108.926 108.926 0 0.000%
counter 89.885 89.885 0 0.000%
counter_functional 90.072 90.072 0 0.000%
dyn_create_destroy_apps 92.397 92.397 0 0.000%
file_upload 103.513 103.513 0 0.000%
function_memory_game 167.787 167.787 0 0.000%
function_router 336.407 336.407 0 0.000%
function_todomvc 162.301 162.301 0 0.000%
futures 226.587 226.587 0 0.000%
game_of_life 109.615 109.615 0 0.000%
immutable 187.178 187.178 0 0.000%
inner_html 86.262 86.262 0 0.000%
js_callback 113.247 113.247 0 0.000%
keyed_list 200.438 200.438 0 0.000%
mount_point 89.388 89.388 0 0.000%
nested_list 113.749 113.749 0 0.000%
node_refs 96.616 96.616 0 0.000%
password_strength 1543.108 1543.108 0 0.000%
portals 97.940 97.940 0 0.000%
router 306.976 306.976 0 0.000%
simple_ssr 143.343 143.671 +0.328 +0.229%
ssr_router 373.151 373.476 +0.324 +0.087%
suspense 110.367 110.367 0 0.000%
timer 92.363 92.363 0 0.000%
timer_functional 100.405 100.405 0 0.000%
todomvc 143.751 143.751 0 0.000%
two_apps 90.267 90.267 0 0.000%
web_worker_fib 154.030 154.030 0 0.000%
webgl 88.837 88.837 0 0.000%

✅ None of the examples has changed their size significantly.

@github-actions
Copy link

github-actions bot commented Apr 21, 2023

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 343.756 344.740 344.059 0.301
Hello World 10 647.496 648.114 647.886 0.208
Function Router 10 2147.225 2158.966 2155.055 3.775
Concurrent Task 10 1007.644 1009.122 1008.379 0.408
Many Providers 10 1691.304 1706.346 1696.642 4.849

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 343.762 344.652 343.935 0.258
Hello World 10 627.274 629.665 628.037 0.764
Function Router 10 2117.921 2145.786 2127.628 8.224
Concurrent Task 10 1007.181 1009.434 1008.355 0.691
Many Providers 10 1646.026 1674.901 1658.078 10.590

@futursolo futursolo added the A-yew Area: The main yew crate label Apr 21, 2023
Copy link
Member

@futursolo futursolo left a comment

Choose a reason for hiding this comment

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

Thank you for this awesome pull request!
I was wondering about how to hydrate the vraw with the current design and is very glad that someone can come up with a working way to hydrate them.

You can refer to the existing hydration tests to create tests for raw hydration.

(The clippy error is probably not related to this one so I will fix it in a separate pull request.)

packages/yew/src/dom_bundle/braw.rs Outdated Show resolved Hide resolved
packages/yew/src/dom_bundle/braw.rs Outdated Show resolved Hide resolved
packages/yew/src/dom_bundle/braw.rs Outdated Show resolved Hide resolved
packages/yew/src/dom_bundle/braw.rs Outdated Show resolved Hide resolved
Copy link
Member

@futursolo futursolo left a comment

Choose a reason for hiding this comment

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

This implementation looks good to me.

However, before we merge this, could you please add a couple test cases?

@dmeijboom
Copy link
Contributor Author

This implementation looks good to me.

However, before we merge this, could you please add a couple test cases?

Yes, will do! Hopefully today :)

@dmeijboom dmeijboom force-pushed the feature/vraw-hydration branch 2 times, most recently from cc8eac0 to 666bb16 Compare April 22, 2023 18:23
Copy link
Member

@hamza1311 hamza1311 left a comment

Choose a reason for hiding this comment

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

This was originally an oversight on my part when I initially added this. Thanks for fixing it, really appreciate it!

@hamza1311 hamza1311 merged commit d0205a8 into yewstack:master Apr 25, 2023
20 of 21 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VRaw is not hydratable
3 participants