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
withastro · Jan 13, 2025 · 3aff68a · 3aff68a
1 parent c7642fb
commit 3aff68a
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 2 deletions.
diff --git a/.changeset/bright-toes-wink.md 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
@@ -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
@@ -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
@@ -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';
+---
+<div id="islandContent">
+<div id="greeting">{greeting}</div>
+</div>
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
@@ -0,0 +1,11 @@
+---
+import ComponentWithProps from '../components/ComponentWithProps.astro';
+---
+<html>
+<head>
+  <title>Testing</title>
+</head>
+<body>
+  <ComponentWithProps server:defer greeting="Hello" />
+</body>
+</html>
diff --git 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',
+				);
+			});
 		});
 	});