-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat: explain the limitations of serialization #10944
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for astro-docs-2 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Thanks for jumping on this issue, Brandon! 🙌 It's looking pretty good, but I think this is now verging into "not a caution" territory, so I'm going to think about about making it just regular body text or at most a "note." Just waiting for some other feedback on this, but very happy to have this!! |
Hey Sarah! I rewrote the middle sentence, and I moved it around a bit. Hopefully that seems to be a meaningful improvement! |
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.
Thank you for taking another pass at the older, existing content that I do think can be improved. It's very helpful!
So given what you proposed, I tried to dig even a little deeper. I do not know that what I wrote is correct, but I think it's the structure/level of detail I'd like to have here. So if you can help make the content actually correct, then I think we have a winner!
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
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.
The new text for client islands looks great and is technically correct. Good job @branberry !!
I left some comments about the server islands page, mostly it needs the same improvements as the other page.
@@ -33,6 +33,15 @@ const avatarURL = await getUserAvatar(userSession); | |||
--- | |||
<img alt="User avatar" src={avatarURL} /> | |||
``` | |||
:::caution[Server Island Prop Limitations] | |||
You can pass a function as a prop to a server island component, but this only works during server rendering. If you try to use the function in a hydrated component (for example, as an event handler), an error will occur. |
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.
You can pass a function as a prop to a server island component, but this only works during server rendering.
Server islands are always server rendered. That is their purpose, make something server rendered even on a statically pre-rendered page. So this seems a bit unhelpful.
Functions can be passed as props while on the server, they can't be passed to the components marked with server:defer
. Meaning you can pass functions around on a server island, but never to a server island.
@@ -33,6 +33,15 @@ const avatarURL = await getUserAvatar(userSession); | |||
--- | |||
<img alt="User avatar" src={avatarURL} /> | |||
``` | |||
:::caution[Server Island Prop Limitations] | |||
You can pass a function as a prop to a server island component, but this only works during server rendering. If you try to use the function in a hydrated component (for example, as an event handler), an error will occur. |
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.
If you try to use the function in a hydrated component (for example, as an event handler), an error will occur.
This example seems weird to me. Server islands are not hydrated (at least not in the sense we use anywhere else in the docs for that word, it doesn't add handlers and bindings for interactivity to existing DOM nodes). They get replaced with the content of a separate request when loaded.
The fallback/original content doesn't require any hydration and neither does the code after the swap. After the swap there isn't even any JS related to the server island in the document, contrary to hydration, which is also a win for server island, and might give people the idea that server islands are heavier on their site than it really is.
:::caution[Server Island Prop Limitations] | ||
You can pass a function as a prop to a server island component, but this only works during server rendering. If you try to use the function in a hydrated component (for example, as an event handler), an error will occur. | ||
|
||
This is because functions cannot be _serialized_ (transferred from the server to the client) by Astro. Objects with circular references are also not serializable. |
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.
I think this should have the same wording regarding serialization as the client islands. Even more so here to not say that the limitation is regarding sharing information with the client because that is an implementation detail. It could send the information between instances of the server (which might remove some other limitations we currently have) and it would have the same problem.
Thanks for this additional helpful context, Fryuni! (And nice job to get a "technically correct" out of Fryuni, @branberry ! 😄 Would you be willing to tackle the extra sections mentioned above?) |
Hey @sarah11918 and @Fryuni, thank you so much for the thorough code review, I really do appreciate it. It's been incredibly helpful, and I've learned a good deal. Working on those extra sections now! |
Description (required)
Documents the limitation of props for framework components, and server island components.
Related issues & labels (optional)