From f54742c0a9264342e0053a020cce49225078d8c0 Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Mon, 4 Mar 2024 23:02:02 +0000 Subject: [PATCH 1/7] fix(rendering): prevent removal of necessary `` elements --- .../astro/src/runtime/server/render/common.ts | 4 ++- .../src/runtime/server/render/component.ts | 26 ++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/runtime/server/render/common.ts b/packages/astro/src/runtime/server/render/common.ts index 780e41463918..6cf309ef31fa 100644 --- a/packages/astro/src/runtime/server/render/common.ts +++ b/packages/astro/src/runtime/server/render/common.ts @@ -12,6 +12,7 @@ import { import { renderAllHeadContent } from './head.js'; import { isRenderInstruction } from './instruction.js'; import { type SlotString, isSlotString } from './slot.js'; +import { restoreHydratableAstroSlot } from './component.js'; /** * Possible chunk types to be written to the destination, and it'll @@ -137,7 +138,8 @@ export function chunkToByteArray( } else { // `stringifyChunk` might return a HTMLString, call `.toString()` to really ensure it's a string const stringified = stringifyChunk(result, chunk); - return encoder.encode(stringified.toString()); + const restoredMarkup = restoreHydratableAstroSlot(stringified) + return encoder.encode(restoredMarkup.toString()); } } diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 5d3435126f23..1d1c59055959 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -66,12 +66,34 @@ function isHTMLComponent(Component: unknown) { } const ASTRO_SLOT_EXP = /<\/?astro-slot\b[^>]*>/g; +// same as ASTRO_SLOT_EXP, but only includes the tag name in the match while requiring that it still be surrounded by "<" and ">" +const ASTRO_SLOT_TAGNAME_EXP = /(?<=<\/?)astro-slot(?=\b[^>]*>)/g; +// used to match temporary tags that will be replaced back with astro-slot +const ASTRO_PRESERVED_SLOT_TAGNAME_EXP = /(?<=<\/?)astro-preserved-slot(?=\b[^>]*>)/g; + const ASTRO_STATIC_SLOT_EXP = /<\/?astro-static-slot\b[^>]*>/g; +// same as ASTRO_STATIC_SLOT_EXP, but only includes the tag name in the match while requiring that it still be surrounded by "<" and ">" +const ASTRO_STATIC_SLOT_TAGNAME_EXP = /(?<=<\/?)astro-static-slot(?=\b[^>]*>)/g; +// used to match temporary tags that will be replaced back with astro-static-slot +const ASTRO_PRESERVED_STATIC_SLOT_TAGNAME_EXP = /(?<=<\/?)astro-preserved-static-slot(?=\b[^>]*>)/g; + function removeStaticAstroSlot(html: string, supportsAstroStaticSlot: boolean) { const exp = supportsAstroStaticSlot ? ASTRO_STATIC_SLOT_EXP : ASTRO_SLOT_EXP; return html.replace(exp, ''); } +// An HTML string may be processed by the parent of a parent, and if it isn't to be hydrated, astro-slot tags will be incorrectly removed. +// We rename them here so that the regex doesn't match. +function preserveHydratableAstroSlot(html: string) { + return html.replaceAll(ASTRO_STATIC_SLOT_TAGNAME_EXP, 'astro-preserved-static-slot') + .replaceAll(ASTRO_SLOT_TAGNAME_EXP, 'astro-preserved-slot'); +} + +export function restoreHydratableAstroSlot(html: string) { + return html.replaceAll(ASTRO_PRESERVED_STATIC_SLOT_TAGNAME_EXP, 'astro-static-slot') + .replaceAll(ASTRO_PRESERVED_SLOT_TAGNAME_EXP, 'astro-slot'); +} + async function renderFrameworkComponent( result: SSRResult, displayName: string, @@ -391,7 +413,9 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr }) ); } - destination.write(markHTMLString(renderElement('astro-island', island, false))); + const renderedElement = renderElement('astro-island', island, false); + const hydratableMarkup = preserveHydratableAstroSlot(renderedElement); + destination.write(markHTMLString(hydratableMarkup)); }, }; } From 338d950942e28d92b6e89c866e2bee14f1d1be78 Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Tue, 5 Mar 2024 00:08:40 +0000 Subject: [PATCH 2/7] add test --- .../src/components/Poly.astro | 6 +++++ .../src/components/Stuff.svelte | 4 ++++ .../src/pages/with-slots.astro | 23 +++++++++++++++++++ packages/astro/e2e/svelte-component.test.js | 11 +++++++++ 4 files changed, 44 insertions(+) create mode 100644 packages/astro/e2e/fixtures/svelte-component/src/components/Poly.astro create mode 100644 packages/astro/e2e/fixtures/svelte-component/src/components/Stuff.svelte create mode 100644 packages/astro/e2e/fixtures/svelte-component/src/pages/with-slots.astro diff --git a/packages/astro/e2e/fixtures/svelte-component/src/components/Poly.astro b/packages/astro/e2e/fixtures/svelte-component/src/components/Poly.astro new file mode 100644 index 000000000000..9f6a0cf11b23 --- /dev/null +++ b/packages/astro/e2e/fixtures/svelte-component/src/components/Poly.astro @@ -0,0 +1,6 @@ +--- +const Tag = 'div'; +--- + + + diff --git a/packages/astro/e2e/fixtures/svelte-component/src/components/Stuff.svelte b/packages/astro/e2e/fixtures/svelte-component/src/components/Stuff.svelte new file mode 100644 index 000000000000..9d7f1d4e0971 --- /dev/null +++ b/packages/astro/e2e/fixtures/svelte-component/src/components/Stuff.svelte @@ -0,0 +1,4 @@ + +
Slot goes here:
diff --git a/packages/astro/e2e/fixtures/svelte-component/src/pages/with-slots.astro b/packages/astro/e2e/fixtures/svelte-component/src/pages/with-slots.astro new file mode 100644 index 000000000000..43ce8a63aea4 --- /dev/null +++ b/packages/astro/e2e/fixtures/svelte-component/src/pages/with-slots.astro @@ -0,0 +1,23 @@ +--- +import Poly from '../components/Poly.astro'; +import Stuff from '../components/Stuff.svelte'; +--- + + + + + + Astro + + +

Astro

+ + + poo + + + + bar + + + diff --git a/packages/astro/e2e/svelte-component.test.js b/packages/astro/e2e/svelte-component.test.js index f0c9aa9bc155..67e032474a70 100644 --- a/packages/astro/e2e/svelte-component.test.js +++ b/packages/astro/e2e/svelte-component.test.js @@ -1,5 +1,6 @@ import { expect } from '@playwright/test'; import { prepareTestFactory } from './shared-component-tests.js'; +import { waitForHydrate } from './test-utils.js'; const { test, createTests } = prepareTestFactory({ root: './fixtures/svelte-component/' }); @@ -35,3 +36,13 @@ test.describe('Svelte components lifecycle', () => { expect((await toggle.textContent()).trim()).toBe('open'); }); }); + + +test.describe('Slotting content into svelte components', () => { + test('should stay after hydration', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/with-slots')); + const hydratableElement = page.locator('#hydratable'); + await waitForHydrate(page, hydratableElement); + await expect(hydratableElement).toHaveText("Slot goes here:poo"); + }); +}); From c107600d8aa501b9c0eb3aa0dfcdf1d0ca744eaf Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Tue, 5 Mar 2024 00:13:37 +0000 Subject: [PATCH 3/7] add changeset --- .changeset/thick-geckos-design.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/thick-geckos-design.md diff --git a/.changeset/thick-geckos-design.md b/.changeset/thick-geckos-design.md new file mode 100644 index 000000000000..28568f01e66d --- /dev/null +++ b/.changeset/thick-geckos-design.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes an issue where elements slotted within interactive framework components disappeared after hydration. From af2a50312ae5569135de81c86ef6e1233e61a007 Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Tue, 5 Mar 2024 00:45:19 +0000 Subject: [PATCH 4/7] missed a spot --- packages/astro/src/runtime/server/render/page.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/runtime/server/render/page.ts b/packages/astro/src/runtime/server/render/page.ts index c27c11e6d3ac..4236e0ed6b30 100644 --- a/packages/astro/src/runtime/server/render/page.ts +++ b/packages/astro/src/runtime/server/render/page.ts @@ -1,5 +1,5 @@ import type { RouteData, SSRResult } from '../../../@types/astro.js'; -import { type NonAstroPageComponent, renderComponentToString } from './component.js'; +import { type NonAstroPageComponent, renderComponentToString, restoreHydratableAstroSlot } from './component.js'; import type { AstroComponentFactory } from './index.js'; import { isAstroComponentFactory } from './astro/index.js'; @@ -31,7 +31,7 @@ export async function renderPage( route ); - const bytes = encoder.encode(str); + const bytes = encoder.encode(restoreHydratableAstroSlot(str)); return new Response(bytes, { headers: new Headers([ From aedcf9268de023636991e2995cbd9593e29330c1 Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Tue, 5 Mar 2024 00:45:53 +0000 Subject: [PATCH 5/7] adjust test --- packages/astro/test/astro-slot-with-client.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/astro/test/astro-slot-with-client.test.js b/packages/astro/test/astro-slot-with-client.test.js index a52d9b907964..8f34b8fc9158 100644 --- a/packages/astro/test/astro-slot-with-client.test.js +++ b/packages/astro/test/astro-slot-with-client.test.js @@ -18,9 +18,9 @@ describe('Slots with client: directives', () => { assert.equal($('script').length, 1); }); - it('Astro slot tags are cleaned', async () => { + it('Astro slot tags are kept', async () => { const html = await fixture.readFile('/index.html'); const $ = cheerio.load(html); - assert.equal($('astro-slot').length, 0); + assert.equal($('astro-slot').length, 1); }); }); From d2e1e2af8d6f7366f8e5fa32f58c5ef4053b98cf Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Thu, 7 Mar 2024 15:52:34 +0000 Subject: [PATCH 6/7] assume `supportsAstroStaticSlot` --- .../astro/src/runtime/server/render/common.ts | 4 +-- .../src/runtime/server/render/component.ts | 28 ++----------------- .../astro/src/runtime/server/render/page.ts | 4 +-- 3 files changed, 6 insertions(+), 30 deletions(-) diff --git a/packages/astro/src/runtime/server/render/common.ts b/packages/astro/src/runtime/server/render/common.ts index 6cf309ef31fa..780e41463918 100644 --- a/packages/astro/src/runtime/server/render/common.ts +++ b/packages/astro/src/runtime/server/render/common.ts @@ -12,7 +12,6 @@ import { import { renderAllHeadContent } from './head.js'; import { isRenderInstruction } from './instruction.js'; import { type SlotString, isSlotString } from './slot.js'; -import { restoreHydratableAstroSlot } from './component.js'; /** * Possible chunk types to be written to the destination, and it'll @@ -138,8 +137,7 @@ export function chunkToByteArray( } else { // `stringifyChunk` might return a HTMLString, call `.toString()` to really ensure it's a string const stringified = stringifyChunk(result, chunk); - const restoredMarkup = restoreHydratableAstroSlot(stringified) - return encoder.encode(restoredMarkup.toString()); + return encoder.encode(stringified.toString()); } } diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 1d1c59055959..9966d20b49ee 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -66,34 +66,13 @@ function isHTMLComponent(Component: unknown) { } const ASTRO_SLOT_EXP = /<\/?astro-slot\b[^>]*>/g; -// same as ASTRO_SLOT_EXP, but only includes the tag name in the match while requiring that it still be surrounded by "<" and ">" -const ASTRO_SLOT_TAGNAME_EXP = /(?<=<\/?)astro-slot(?=\b[^>]*>)/g; -// used to match temporary tags that will be replaced back with astro-slot -const ASTRO_PRESERVED_SLOT_TAGNAME_EXP = /(?<=<\/?)astro-preserved-slot(?=\b[^>]*>)/g; - const ASTRO_STATIC_SLOT_EXP = /<\/?astro-static-slot\b[^>]*>/g; -// same as ASTRO_STATIC_SLOT_EXP, but only includes the tag name in the match while requiring that it still be surrounded by "<" and ">" -const ASTRO_STATIC_SLOT_TAGNAME_EXP = /(?<=<\/?)astro-static-slot(?=\b[^>]*>)/g; -// used to match temporary tags that will be replaced back with astro-static-slot -const ASTRO_PRESERVED_STATIC_SLOT_TAGNAME_EXP = /(?<=<\/?)astro-preserved-static-slot(?=\b[^>]*>)/g; -function removeStaticAstroSlot(html: string, supportsAstroStaticSlot: boolean) { +function removeStaticAstroSlot(html: string, supportsAstroStaticSlot = true) { const exp = supportsAstroStaticSlot ? ASTRO_STATIC_SLOT_EXP : ASTRO_SLOT_EXP; return html.replace(exp, ''); } -// An HTML string may be processed by the parent of a parent, and if it isn't to be hydrated, astro-slot tags will be incorrectly removed. -// We rename them here so that the regex doesn't match. -function preserveHydratableAstroSlot(html: string) { - return html.replaceAll(ASTRO_STATIC_SLOT_TAGNAME_EXP, 'astro-preserved-static-slot') - .replaceAll(ASTRO_SLOT_TAGNAME_EXP, 'astro-preserved-slot'); -} - -export function restoreHydratableAstroSlot(html: string) { - return html.replaceAll(ASTRO_PRESERVED_STATIC_SLOT_TAGNAME_EXP, 'astro-static-slot') - .replaceAll(ASTRO_PRESERVED_SLOT_TAGNAME_EXP, 'astro-slot'); -} - async function renderFrameworkComponent( result: SSRResult, displayName: string, @@ -332,7 +311,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr } else if (html && html.length > 0) { destination.write( markHTMLString( - removeStaticAstroSlot(html, renderer?.ssr?.supportsAstroStaticSlot ?? false) + removeStaticAstroSlot(html, renderer?.ssr?.supportsAstroStaticSlot) ) ); } @@ -414,8 +393,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr ); } const renderedElement = renderElement('astro-island', island, false); - const hydratableMarkup = preserveHydratableAstroSlot(renderedElement); - destination.write(markHTMLString(hydratableMarkup)); + destination.write(renderedElement); }, }; } diff --git a/packages/astro/src/runtime/server/render/page.ts b/packages/astro/src/runtime/server/render/page.ts index 4236e0ed6b30..c27c11e6d3ac 100644 --- a/packages/astro/src/runtime/server/render/page.ts +++ b/packages/astro/src/runtime/server/render/page.ts @@ -1,5 +1,5 @@ import type { RouteData, SSRResult } from '../../../@types/astro.js'; -import { type NonAstroPageComponent, renderComponentToString, restoreHydratableAstroSlot } from './component.js'; +import { type NonAstroPageComponent, renderComponentToString } from './component.js'; import type { AstroComponentFactory } from './index.js'; import { isAstroComponentFactory } from './astro/index.js'; @@ -31,7 +31,7 @@ export async function renderPage( route ); - const bytes = encoder.encode(restoreHydratableAstroSlot(str)); + const bytes = encoder.encode(str); return new Response(bytes, { headers: new Headers([ From 1ff765aec44c66f680a84320c31717ab822842ad Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Thu, 7 Mar 2024 16:14:40 +0000 Subject: [PATCH 7/7] bring back accidentally removed `markHTMLString` call --- packages/astro/src/runtime/server/render/component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 9966d20b49ee..1d6b6e6dd1b2 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -393,7 +393,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr ); } const renderedElement = renderElement('astro-island', island, false); - destination.write(renderedElement); + destination.write(markHTMLString(renderedElement)); }, }; }