-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
SSR Hydration #2453
Conversation
…trailing elements.
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 🌎 |
There was a problem hiding this 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.
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. |
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 :) |
@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. |
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. 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. |
* Move example from #2453. * Add implementation note. * Update Implementation Note.
Description
This pull request brings SSR Hydration to Yew.
Fixes #41
Fixes #2403
This pull request consists of the following changes:
Renderer
andRenderer::hydrate
for Hydration.(This is done to ensure
NodeRef
of parents can be fixed before child components, should also be beneficial in client-side rendering as well.)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
cargo make pr-flow