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

SSR Hydration #2453

Closed
wants to merge 42 commits into from
Closed

SSR Hydration #2453

wants to merge 42 commits into from

Conversation

futursolo
Copy link
Member

@futursolo futursolo commented Feb 12, 2022

Description

This pull request brings SSR Hydration to Yew.

Fixes #41
Fixes #2403

This pull request consists of the following changes:

  1. Fixed pr-flow on macOS, alongside with some tests that was skipped in the past.
  2. Added Renderer and Renderer::hydrate for Hydration.
  3. Added a function router example equivalent to router example.
  4. Added ssr router example that renders function router example on the server side and hydrates it on the client side.
  5. Scheduler now always renders parent component before child component
    (This is done to ensure NodeRef of parents can be fixed before child components, should also be beneficial in client-side rendering as well.)
  6. The application will now panic if you try to suspend from a fallback component. (This was previously an undefined behaviour where the last non-suspended rendered DOM of the fallback component will be shown.)

Suspense

Event replay is not implemented, this does not matter too much unless your Suspense is very slow.

This is kind of difficult with the current event system.
I guess it can be dealt with in a future PR.

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 Feb 12, 2022

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

https://yew-rs--pr2453-hydration-1ikzdrq3.web.app

(expires Sat, 26 Feb 2022 13:58:26 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

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.

Looks good to me, for the most part. Just a few comments. This is great addition, thank you for implementing it.

examples/simple_ssr/src/bin/simple_ssr_server.rs Outdated Show resolved Hide resolved
examples/ssr_router/src/lib.rs Show resolved Hide resolved
packages/yew/src/html/component/marker.rs Outdated Show resolved Hide resolved
packages/yew/src/html/component/scope.rs Outdated Show resolved Hide resolved
examples/function_router/src/app.rs Show resolved Hide resolved
packages/yew/src/scheduler.rs Show resolved Hide resolved
@ranile ranile requested a review from siku2 February 17, 2022 15:07
@futursolo
Copy link
Member Author

futursolo commented Feb 23, 2022

I haven't been following #2330 and this PR was done under the impression of #2330 wouldn't be landed at anytime soon.

The implementation will need some re-visitation if #2330 is merged before it.

If #2330 is merged before this pull request (which I am fine with and actually prefer), I will close this PR and split this into multiple smaller pull requests to avoid having to resolve conflicts on a big changeset repeatedly.

@WorldSEnder
Copy link
Member

WorldSEnder commented Feb 23, 2022

If you need a quick run down of specific changes in #2330 , feel free to ping me here or on discord. In general I'd say you want to hydrate bundle nodes instead of virtual nodes and that's all that needs to change (on the surface, anyway) and should cover most conflicts.

Also, thanks for the ssr examples, good to have them :)

@ranile
Copy link
Member

ranile commented Feb 25, 2022

@futursolo @WorldSEnder, we can merge the one which will cause less conflicts with the other. Which one should go first? I wanted to get #2330 in first because it's been sitting for way too long but if that will cause this PR issues and merging this PR will not make that one much harder to fix conflicts and merge, then I'd be happy with merging this first.

@futursolo
Copy link
Member Author

There's no easy way out of this situation. Whichever that is merged later is going to have to re-implement hydration on bundle nodes.
However, #2453 can be split into multiple smaller pull requests to avoid resolving conflicts repeatedly.

I guess #2330 should be given priority regardless of the difficulty to resolve conflicts for other PRs as it is taking too long to merge and any upcoming pull request needs to take consideration of it.

So I guess #2330 -> #2453 was the preferred order if nothing is merged in the meanwhile to make #2330 become unable to merge again.

@futursolo futursolo marked this pull request as draft March 6, 2022 05:11
futursolo added a commit to futursolo/yew that referenced this pull request Mar 6, 2022
ranile pushed a commit that referenced this pull request Mar 7, 2022
* Move example from #2453.

* Add implementation note.

* Update Implementation Note.
@WorldSEnder WorldSEnder added the A-yew Area: The main yew crate label Mar 18, 2022
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.

pr-flow is broken on macOS Proposal: Server-side rendering
3 participants