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

fix: Nested hydration with Solid #3003

Merged
merged 2 commits into from
Apr 6, 2022

Conversation

ryansolid
Copy link
Contributor

Changes

Updated Solid's client renderer and integration to fix issue with nested hydration, like you'd find in the recursive comments in Hackernews.

Previous behavior recreated static children as DOM elements and inserted them during hydration. This reset hydration state messing with Solid's hydration when nested.

Solid treats any undefined value inserted as skip during hydration assuming client will match server eventually, so I just removed the children and it will skip hydration of static children.

This also appears to improve the Solid integration's hydration performance by about 200%.

Testing

Docs

@changeset-bot
Copy link

changeset-bot bot commented Apr 6, 2022

🦋 Changeset detected

Latest commit: a8daed3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@astrojs/solid-js Patch
@astrojs/renderer-solid Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: solid Related to Solid (scope) pkg: integration Related to any renderer integration (scope) labels Apr 6, 2022
@ryansolid
Copy link
Contributor Author

ryansolid commented Apr 6, 2022

Woke up and realized I missed a case. I need to update this for when Solid dynamically renders the static children outside of hydration. I will update this PR in a bit.

@ryansolid ryansolid marked this pull request as draft April 6, 2022 14:40
@ryansolid ryansolid marked this pull request as ready for review April 6, 2022 17:50
@ryansolid
Copy link
Contributor Author

Ok I've made it better now. Took some thinking since we don't really ever want to be creating these nodes on the client if possible. I've tested a number of scenarios. Including children initially being present or not, and eager versus lazy evaluation of children.

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

This looks amazing, thank you so much @ryansolid! Excited to see what folks can build with this now!

@natemoo-re natemoo-re merged commit 13b782f into withastro:main Apr 6, 2022
This was referenced Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope) pkg: solid Related to Solid (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants