fix: include slot scripts in server island response#15633
fix: include slot scripts in server island response#15633matthewp merged 9 commits intowithastro:mainfrom
Conversation
Components passed as slots to server:defer components had their scripts silently dropped because renderSlotToString separates HTML content from render instructions, and only the HTML was serialized into the island payload via content.toString(). This appends script instructions to the slot HTML before encryption, so the island response includes the scripts needed for interactivity. This may result in duplicate script injection when the same component is also used statically on the page. Deduplication is not feasible here due to the concurrent BufferedRenderer model — renderedScripts is not yet populated at the time island payloads are constructed.
🦋 Changeset detectedLatest commit: 3a5d9a3 The changes in this PR will be included in the next version bump. 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
bot
added
the
pkg: astro
|
Failing check seems to be unrelated and flaky. |
florian-lefebvre
left a comment
florian-lefebvre
left a comment
There was a problem hiding this comment.
Can you add a test for it? We probably have a file called server-islands.test.js and an associated fixture
|
Sure thing @florian-lefebvre ! I have added the example from my minimal repro. I verified that the test fails without the fix. |
Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
| // to server:defer components retain their scripts in the island response. | ||
| // renderSlotToString returns a SlotString (typed as string) that carries | ||
| // render instructions stripped from the HTML content. | ||
| const slotContent = content as unknown as import('./slot.js').SlotString; |
There was a problem hiding this comment.
can you use ar regular top-level type import here?
There was a problem hiding this comment.
sure, that should work. This was an oversight and there no particular reason for this kind of import.
3 participants
Fixes: #15622 Components passed as slots to
server:defercomponents had their<script>tags silently dropped.renderSlotToStringseparates HTML from render instructions, and only the HTML survived serialization viacontent.toString().This appends script instructions to the slot HTML before encryption, so the island response includes the scripts needed for interactivity.
Changes
type === 'script'render instructions to slot HTML inServerIslandComponent.getIslandContent()before the payload is encryptedTesting
Minimal reproduction: https://github.com/jwoyo/astro-server-island-bug
<script>passed as slot toserver:deferwrapper<script>→ no interactivityDocs
No docs needed — this is a bug fix restoring expected behavior.
One could argue that may result in duplicate script injection when the same component is also used statically on the page. But deduplication is not feasible at this point in the render pipeline due to concurrent buffering - the need to build idempotent scripts (that don't hurt when executed twice) could be part of the docs.