From 35af73aace97a7cc898b9aa5040db8bc2ac62687 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 15 Aug 2024 08:04:02 -0400 Subject: [PATCH] Prevent errant HTML from crashing server islands (#11692) --- .changeset/hot-dodos-whisper.md | 7 +++++++ .../server-islands/src/components/HTMLError.astro | 1 + .../fixtures/server-islands/src/pages/index.astro | 12 ++++++++++++ packages/astro/e2e/server-islands.test.js | 6 ++++++ .../src/runtime/server/render/server-islands.ts | 7 ++++--- 5 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 .changeset/hot-dodos-whisper.md create mode 100644 packages/astro/e2e/fixtures/server-islands/src/components/HTMLError.astro diff --git a/.changeset/hot-dodos-whisper.md b/.changeset/hot-dodos-whisper.md new file mode 100644 index 0000000000..07398058f2 --- /dev/null +++ b/.changeset/hot-dodos-whisper.md @@ -0,0 +1,7 @@ +--- +'astro': patch +--- + +Prevent errant HTML from crashing server islands + +When an HTML minifier strips away the server island comment, the script can't correctly know where the end of the fallback content is. This makes it so that it simply doesn't remove any DOM in that scenario. This means the fallback isn't removed, but it also doesn't crash the browser. diff --git a/packages/astro/e2e/fixtures/server-islands/src/components/HTMLError.astro b/packages/astro/e2e/fixtures/server-islands/src/components/HTMLError.astro new file mode 100644 index 0000000000..91b1946539 --- /dev/null +++ b/packages/astro/e2e/fixtures/server-islands/src/components/HTMLError.astro @@ -0,0 +1 @@ +
diff --git a/packages/astro/e2e/fixtures/server-islands/src/pages/index.astro b/packages/astro/e2e/fixtures/server-islands/src/pages/index.astro index de9a6c456f..eff5df25e9 100644 --- a/packages/astro/e2e/fixtures/server-islands/src/pages/index.astro +++ b/packages/astro/e2e/fixtures/server-islands/src/pages/index.astro @@ -1,6 +1,7 @@ --- import Island from '../components/Island.astro'; import Self from '../components/Self.astro'; +import HTMLError from '../components/HTMLError.astro'; --- @@ -12,5 +13,16 @@ import Self from '../components/Self.astro';

children

+ +
+ + + +
diff --git a/packages/astro/e2e/server-islands.test.js b/packages/astro/e2e/server-islands.test.js index 2e68937b98..1c4e2a7e80 100644 --- a/packages/astro/e2e/server-islands.test.js +++ b/packages/astro/e2e/server-islands.test.js @@ -50,6 +50,12 @@ test.describe('Server islands', () => { await expect(el).toHaveCount(2); }); + + test('Missing server island start comment doesn\'t cause browser to lock up', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/base/')); + let el = page.locator('#first'); + await expect(el).toHaveCount(1); + }); }); test.describe('Development - trailingSlash: ignore', () => { diff --git a/packages/astro/src/runtime/server/render/server-islands.ts b/packages/astro/src/runtime/server/render/server-islands.ts index 153c01c656..0a07211344 100644 --- a/packages/astro/src/runtime/server/render/server-islands.ts +++ b/packages/astro/src/runtime/server/render/server-islands.ts @@ -86,9 +86,10 @@ if(response.status === 200 && response.headers.get('content-type') === 'text/htm let html = await response.text(); // Swap! - while(script.previousSibling?.nodeType !== 8 && - script.previousSibling?.data !== 'server-island-start') { - script.previousSibling?.remove(); + while(script.previousSibling && + script.previousSibling.nodeType !== 8 && + script.previousSibling.data !== 'server-island-start') { + script.previousSibling.remove(); } script.previousSibling?.remove();