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

#[cfg(feature = "render")] and yew::Renderer #2498

Merged
merged 35 commits into from Mar 19, 2022

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 🌎

# 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
@hamza1311 hamza1311 self-requested a review March 13, 2022 17:00
@hamza1311
Copy link
Member

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

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.

Implementation looks good. Just a few comments

packages/yew/src/dom_bundle/mod.rs Outdated Show resolved Hide resolved
packages/yew/src/dom_bundle/traits.rs Show resolved Hide resolved
packages/yew/src/html/component/scope.rs Show resolved Hide resolved
packages/yew/src/renderer.rs Show resolved Hide resolved
packages/yew/src/renderer.rs Show resolved Hide resolved
packages/yew/Cargo.toml Outdated Show resolved Hide resolved
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
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.

None yet

4 participants