Skip to content

Conversation

@futursolo
Copy link
Member

@futursolo futursolo commented Mar 6, 2022

Description

Originally part of #2453.

  1. Makes a yew::Renderer that supersedes start_app_*.
  2. Moves any common logic shared by both client-side rendering and server-side rendering out of dom_bundle.
  3. Added feature render allowing client-side rendering logic to be disabled for server-side rendering.

This pull request also includes changes from the following pull requests to avoid repeatedly resolving conflicts:

Caveats

This pull request inevitably introduces a lot of feature flags into the codebase.
However, I don't have a better way to handle this at the moment.

Checklist

  • I have run cargo make pr-flow
  • I have reviewed my own code
  • I have added tests

@github-actions
Copy link

github-actions bot commented Mar 6, 2022

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

https://yew-rs--pr2498-feat-render-lo1tccs5.web.app

(expires Sat, 26 Mar 2022 00:32:20 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

futursolo added 13 commits March 6, 2022 21:10
# Conflicts:
#	examples/function_router/Cargo.toml
#	examples/function_router/index.html
#	examples/function_router/src/main.rs
#	packages/yew/Cargo.toml
# Conflicts:
#	packages/yew/src/app_handle.rs
#	packages/yew/src/dom_bundle/bcomp.rs
#	packages/yew/src/html/component/scope.rs
#	packages/yew/src/lib.rs
#	packages/yew/src/virtual_dom/vcomp.rs
@futursolo futursolo marked this pull request as ready for review March 9, 2022 12:27
@ranile ranile self-requested a review March 13, 2022 17:00
@ranile
Copy link
Member

ranile commented Mar 13, 2022

Just requesting my own review so I don't forget to look at this

Copy link
Member

@ranile ranile left a comment

Choose a reason for hiding this comment

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

Implementation looks good. Just a few comments

github-actions[bot]
github-actions bot previously approved these changes Mar 18, 2022
@github-actions
Copy link

github-actions bot commented Mar 18, 2022

Size Comparison

examples master (KB) pull request (KB) diff
boids 304.520 304.690 +0.171
contexts 218.472 218.827 +0.355
counter 157.479 157.766 +0.287
dyn_create_destroy_apps 166.340 166.455 +0.115
file_upload 187.112 187.320 +0.208
function_memory_game 343.781 344.647 +0.866
function_router 688.486 22.254 -666.232
function_todomvc 319.987 320.487 +0.500
futures 355.671 355.903 +0.232
game_of_life 199.878 200.101 +0.223
inner_html 149.638 149.812 +0.175
js_callback 164.987 165.191 +0.204
keyed_list 322.397 322.457 +0.060
mount_point 156.952 157.106 +0.154
nested_list 218.629 218.762 +0.133
node_refs 164.232 164.352 +0.119
password_strength 1845.853 1845.999 +0.146
portals 171.420 171.551 +0.131
router 581.501 583.786 +2.285
suspense 205.727 206.096 +0.369
timer 163.372 163.583 +0.211
todomvc 263.759 263.862 +0.104
two_apps 159.051 159.250 +0.199
webgl 163.646 164.002 +0.356

@WorldSEnder
Copy link
Member

The measured drop in size for function_router looks a bit suspect. Any ideas?

@futursolo
Copy link
Member Author

function_router has feature render disabled by default because it's also used in ssr by the upcoming ssr_router example.

@WorldSEnder
Copy link
Member

I suspected as much. Then this is a perfect example of the benefit of having csr by default disabled; a drop of almost 300KB in size is great, even if it's "only" on SSR.

github-actions[bot]
github-actions bot previously approved these changes Mar 18, 2022
@futursolo futursolo requested review from ranile and voidpumpkin March 19, 2022 14:38
@futursolo
Copy link
Member Author

I will merge this pull request as I believe all items has been addressed.

@futursolo futursolo merged commit 8bc2212 into yewstack:master Mar 19, 2022
@futursolo futursolo deleted the feat-render branch March 26, 2022 09:02
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 A-yew-macro Area: The yew-macro crate A-yew-router Area: The yew-router crate breaking change documentation

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants