From 3aff68a4195a608e92dc6299610a4b06e7bb96f1 Mon Sep 17 00:00:00 2001 From: Chris Kanich Date: Mon, 13 Jan 2025 03:24:30 -0800 Subject: [PATCH] Remove encryption of empty props to allow server island cacheability (#12956) * tests for cacheable server islands with no props * changeset * allow server islands to omit encrypted props when they do not have any props * prod and dev tests --- .changeset/bright-toes-wink.md | 5 ++ .../astro/src/core/server-islands/endpoint.ts | 2 +- .../runtime/server/render/server-islands.ts | 3 +- .../src/components/ComponentWithProps.astro | 11 ++++ .../src/pages/includeComponentWithProps.astro | 11 ++++ packages/astro/test/server-islands.test.js | 65 +++++++++++++++++++ 6 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 .changeset/bright-toes-wink.md create mode 100644 packages/astro/test/fixtures/server-islands/ssr/src/components/ComponentWithProps.astro create mode 100644 packages/astro/test/fixtures/server-islands/ssr/src/pages/includeComponentWithProps.astro diff --git a/.changeset/bright-toes-wink.md b/.changeset/bright-toes-wink.md new file mode 100644 index 000000000000..e83229bd83f9 --- /dev/null +++ b/.changeset/bright-toes-wink.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Removes encryption of empty props to allow server island cacheability diff --git a/packages/astro/src/core/server-islands/endpoint.ts b/packages/astro/src/core/server-islands/endpoint.ts index 97ea4ee23c9a..b152e0967f00 100644 --- a/packages/astro/src/core/server-islands/endpoint.ts +++ b/packages/astro/src/core/server-islands/endpoint.ts @@ -120,7 +120,7 @@ export function createEndpoint(manifest: SSRManifest) { const key = await manifest.key; const encryptedProps = data.encryptedProps; - const propString = await decryptString(key, encryptedProps); + const propString = encryptedProps === '' ? '{}' : await decryptString(key, encryptedProps); const props = JSON.parse(propString); const componentModule = await imp(); diff --git a/packages/astro/src/runtime/server/render/server-islands.ts b/packages/astro/src/runtime/server/render/server-islands.ts index 7059218287c8..e45b1e6d47d1 100644 --- a/packages/astro/src/runtime/server/render/server-islands.ts +++ b/packages/astro/src/runtime/server/render/server-islands.ts @@ -76,7 +76,8 @@ export function renderServerIsland( } const key = await result.key; - const propsEncrypted = await encryptString(key, JSON.stringify(props)); + const propsEncrypted = + Object.keys(props).length === 0 ? '' : await encryptString(key, JSON.stringify(props)); const hostId = crypto.randomUUID(); diff --git a/packages/astro/test/fixtures/server-islands/ssr/src/components/ComponentWithProps.astro b/packages/astro/test/fixtures/server-islands/ssr/src/components/ComponentWithProps.astro new file mode 100644 index 000000000000..b1eeff544622 --- /dev/null +++ b/packages/astro/test/fixtures/server-islands/ssr/src/components/ComponentWithProps.astro @@ -0,0 +1,11 @@ +--- +await new Promise(resolve => setTimeout(resolve, 1)); +Astro.response.headers.set('X-Works', 'true'); +export type Props = { + greeting?: string; +}; +const greeting = Astro.props?.greeting ? Astro.props.greeting : 'default greeting'; +--- +
+
{greeting}
+
diff --git a/packages/astro/test/fixtures/server-islands/ssr/src/pages/includeComponentWithProps.astro b/packages/astro/test/fixtures/server-islands/ssr/src/pages/includeComponentWithProps.astro new file mode 100644 index 000000000000..614f7f8b86a4 --- /dev/null +++ b/packages/astro/test/fixtures/server-islands/ssr/src/pages/includeComponentWithProps.astro @@ -0,0 +1,11 @@ +--- +import ComponentWithProps from '../components/ComponentWithProps.astro'; +--- + + + Testing + + + + + diff --git a/packages/astro/test/server-islands.test.js b/packages/astro/test/server-islands.test.js index e0a591588f63..96b58354e29a 100644 --- a/packages/astro/test/server-islands.test.js +++ b/packages/astro/test/server-islands.test.js @@ -61,6 +61,36 @@ describe('Server islands', () => { const works = res.headers.get('X-Works'); assert.equal(works, 'true', 'able to set header from server island'); }); + it('omits empty props from the query string', async () => { + const res = await fixture.fetch('/'); + assert.equal(res.status, 200); + const html = await res.text(); + const fetchMatch = html.match(/fetch\('\/_server-islands\/Island\?[^']*p=([^&']*)/s); + assert.equal(fetchMatch.length, 2, 'should include props in the query string'); + assert.equal(fetchMatch[1], '', 'should not include encrypted empty props'); + }); + it('re-encrypts props on each request', async () => { + const res = await fixture.fetch('/includeComponentWithProps/'); + assert.equal(res.status, 200); + const html = await res.text(); + const fetchMatch = html.match( + /fetch\('\/_server-islands\/ComponentWithProps\?[^']*p=([^&']*)/s, + ); + assert.equal(fetchMatch.length, 2, 'should include props in the query string'); + const firstProps = fetchMatch[1]; + const secondRes = await fixture.fetch('/includeComponentWithProps/'); + assert.equal(secondRes.status, 200); + const secondHtml = await secondRes.text(); + const secondFetchMatch = secondHtml.match( + /fetch\('\/_server-islands\/ComponentWithProps\?[^']*p=([^&']*)/s, + ); + assert.equal(secondFetchMatch.length, 2, 'should include props in the query string'); + assert.notEqual( + secondFetchMatch[1], + firstProps, + 'should re-encrypt props on each request with a different IV', + ); + }); }); describe('prod', () => { @@ -103,6 +133,41 @@ describe('Server islands', () => { const response = await app.render(request); assert.equal(response.headers.get('x-robots-tag'), 'noindex'); }); + it('omits empty props from the query string', async () => { + const app = await fixture.loadTestAdapterApp(); + const request = new Request('http://example.com/'); + const response = await app.render(request); + assert.equal(response.status, 200); + const html = await response.text(); + const fetchMatch = html.match(/fetch\('\/_server-islands\/Island\?[^']*p=([^&']*)/s); + assert.equal(fetchMatch.length, 2, 'should include props in the query string'); + assert.equal(fetchMatch[1], '', 'should not include encrypted empty props'); + }); + it('re-encrypts props on each request', async () => { + const app = await fixture.loadTestAdapterApp(); + const request = new Request('http://example.com/includeComponentWithProps/'); + const response = await app.render(request); + assert.equal(response.status, 200); + const html = await response.text(); + const fetchMatch = html.match( + /fetch\('\/_server-islands\/ComponentWithProps\?[^']*p=([^&']*)/s, + ); + assert.equal(fetchMatch.length, 2, 'should include props in the query string'); + const firstProps = fetchMatch[1]; + const secondRequest = new Request('http://example.com/includeComponentWithProps/'); + const secondResponse = await app.render(secondRequest); + assert.equal(secondResponse.status, 200); + const secondHtml = await secondResponse.text(); + const secondFetchMatch = secondHtml.match( + /fetch\('\/_server-islands\/ComponentWithProps\?[^']*p=([^&']*)/s, + ); + assert.equal(secondFetchMatch.length, 2, 'should include props in the query string'); + assert.notEqual( + secondFetchMatch[1], + firstProps, + 'should re-encrypt props on each request with a different IV', + ); + }); }); });