Stringify shouldn't throw on user object during rendering (#8127)

* fix(#7923): do not throw on user { type } object

* chore: remove unused type export

* chore: guess it wasn't unused

Author: natemoo-re

.changeset/brave-cheetahs-float.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Do not throw Error when users pass an object with a "type" property

packages/astro/src/runtime/server/render/common.ts

@@ -1,6 +1,7 @@
 import type { SSRResult } from '../../../@types/astro';
-import type { RenderInstruction } from './types.js';
+import type { RenderInstruction } from './instruction.js';
 
+import { isRenderInstruction } from './instruction.js';
 import { HTMLBytes, HTMLString, markHTMLString } from '../escape.js';
 import {
 	determineIfNeedsHydrationScript,
@@ -52,8 +53,8 @@ function stringifyChunk(
 	result: SSRResult,
 	chunk: string | HTMLString | SlotString | RenderInstruction
 ): string {
-	if (typeof (chunk as any).type === 'string') {
-		const instruction = chunk as RenderInstruction;
+	if (isRenderInstruction(chunk)) {
+		const instruction = chunk;
 		switch (instruction.type) {
 			case 'directive': {
 				const { hydration } = instruction;
@@ -64,8 +65,8 @@ function stringifyChunk(
 				let prescriptType: PrescriptType = needsHydrationScript
 					? 'both'
 					: needsDirectiveScript
-					? 'directive'
-					: null;
+						? 'directive'
+						: null;
 				if (prescriptType) {
 					let prescripts = getPrescripts(result, prescriptType, hydration.directive);
 					return markHTMLString(prescripts);
@@ -86,27 +87,24 @@ function stringifyChunk(
 				return renderAllHeadContent(result);
 			}
 			default: {
-				if (chunk instanceof Response) {
-					return '';
-				}
 				throw new Error(`Unknown chunk type: ${(chunk as any).type}`);
 			}
 		}
-	} else {
-		if (isSlotString(chunk as string)) {
-			let out = '';
-			const c = chunk as SlotString;
-			if (c.instructions) {
-				for (const instr of c.instructions) {
-					out += stringifyChunk(result, instr);
-				}
+	} else if (chunk instanceof Response) {
+		return '';
+	} else if (isSlotString(chunk as string)) {
+		let out = '';
+		const c = chunk as SlotString;
+		if (c.instructions) {
+			for (const instr of c.instructions) {
+				out += stringifyChunk(result, instr);
 			}
-			out += chunk.toString();
-			return out;
 		}
-
-		return chunk.toString();
+		out += chunk.toString();
+		return out;
 	}
+
+	return chunk.toString();
 }
 
 export function chunkToString(result: SSRResult, chunk: Exclude<RenderDestinationChunk, Response>) {

packages/astro/src/runtime/server/render/component.ts

@@ -4,7 +4,7 @@ import type {
 	SSRLoadedRenderer,
 	SSRResult,
 } from '../../../@types/astro';
-import type { RenderInstruction } from './types.js';
+import { createRenderInstruction, type RenderInstruction } from './instruction.js';
 
 import { AstroError, AstroErrorData } from '../../../core/errors/index.js';
 import { HTMLBytes, markHTMLString } from '../escape.js';
@@ -370,7 +370,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 					destination.write(instruction);
 				}
 			}
-			destination.write({ type: 'directive', hydration });
+			destination.write(createRenderInstruction({ type: 'directive', hydration }));
 			destination.write(markHTMLString(renderElement('astro-island', island, false)));
 		},
 	};

packages/astro/src/runtime/server/render/head.ts

@@ -1,7 +1,8 @@
 import type { SSRResult } from '../../../@types/astro';
 
 import { markHTMLString } from '../escape.js';
-import type { MaybeRenderHeadInstruction, RenderHeadInstruction } from './types';
+import { createRenderInstruction } from './instruction.js';
+import type { MaybeRenderHeadInstruction, RenderHeadInstruction } from './instruction.js';
 import { renderElement } from './util.js';
 
 // Filter out duplicate elements in our set
@@ -45,7 +46,7 @@ export function renderAllHeadContent(result: SSRResult) {
 }
 
 export function* renderHead(): Generator<RenderHeadInstruction> {
-	yield { type: 'head' };
+	yield createRenderInstruction({ type: 'head' });
 }
 
 // This function is called by Astro components that do not contain a <head> component
@@ -55,5 +56,5 @@ export function* renderHead(): Generator<RenderHeadInstruction> {
 export function* maybeRenderHead(): Generator<MaybeRenderHeadInstruction> {
 	// This is an instruction informing the page rendering that head might need rendering.
 	// This allows the page to deduplicate head injections.
-	yield { type: 'maybe-head' };
+	yield createRenderInstruction({ type: 'maybe-head' });
 }

packages/astro/src/runtime/server/render/index.ts

@@ -1,4 +1,5 @@
 export type { AstroComponentFactory, AstroComponentInstance } from './astro/index';
+export type { RenderInstruction } from './instruction';
 export { createHeadAndContent, renderTemplate, renderToString } from './astro/index.js';
 export { Fragment, Renderer, chunkToByteArray, chunkToString } from './common.js';
 export { renderComponent, renderComponentToString } from './component.js';
@@ -7,5 +8,4 @@ export { maybeRenderHead, renderHead } from './head.js';
 export { renderPage } from './page.js';
 export { renderSlot, renderSlotToString, type ComponentSlots } from './slot.js';
 export { renderScriptElement, renderUniqueStylesheet } from './tags.js';
-export type { RenderInstruction } from './types';
 export { addAttribute, defineScriptVars, voidElementNames } from './util.js';

packages/astro/src/runtime/server/render/instruction.ts

@@ -0,0 +1,32 @@
+import type { HydrationMetadata } from '../hydration.js';
+
+const RenderInstructionSymbol = Symbol.for('astro:render');
+
+export type RenderDirectiveInstruction = {
+	type: 'directive';
+	hydration: HydrationMetadata;
+};
+
+export type RenderHeadInstruction = {
+	type: 'head';
+};
+
+export type MaybeRenderHeadInstruction = {
+	type: 'maybe-head';
+};
+
+export type RenderInstruction =
+	| RenderDirectiveInstruction
+	| RenderHeadInstruction
+	| MaybeRenderHeadInstruction;
+
+export function createRenderInstruction(instruction: RenderDirectiveInstruction): RenderDirectiveInstruction;
+export function createRenderInstruction(instruction: RenderHeadInstruction): RenderHeadInstruction;
+export function createRenderInstruction(instruction: MaybeRenderHeadInstruction): MaybeRenderHeadInstruction;
+export function createRenderInstruction(instruction: { type: string }): RenderInstruction {
+    return Object.defineProperty(instruction as RenderInstruction, RenderInstructionSymbol, { value: true });
+}
+
+export function isRenderInstruction(chunk: any): chunk is RenderInstruction {
+    return chunk && typeof chunk === 'object' && chunk[RenderInstructionSymbol];
+}

packages/astro/src/runtime/server/render/slot.ts

@@ -1,6 +1,6 @@
 import type { SSRResult } from '../../../@types/astro.js';
 import type { renderTemplate } from './astro/render-template.js';
-import type { RenderInstruction } from './types.js';
+import type { RenderInstruction } from './instruction.js';
 
 import { HTMLString, markHTMLString } from '../escape.js';
 import { renderChild } from './any.js';

packages/astro/src/runtime/server/render/types.ts

@@ -0,0 +1,52 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+import { fileURLToPath } from 'node:url';
+import { createFs, createRequestAndResponse, runInContainer } from '../test-utils.js';
+
+const root = new URL('../../fixtures/alias/', import.meta.url);
+
+describe('core/render chunk', () => {
+	it('does not throw on user object with type', async () => {
+		const fs = createFs(
+			{
+				'/src/pages/index.astro': `
+				---
+				const value = { type: 'foobar' }
+				---
+				<div id="chunk">{value}</div>
+			`,
+			},
+			root
+		);
+
+		await runInContainer(
+			{
+				fs,
+				inlineConfig: {
+					root: fileURLToPath(root),
+					logLevel: 'silent',
+					integrations: [],
+				},
+			},
+			async (container) => {
+				const { req, res, done, text } = createRequestAndResponse({
+					method: 'GET',
+					url: '/',
+				});
+				container.handle(req, res);
+
+				await done;
+				try {
+					const html = await text();
+					const $ = cheerio.load(html);
+					const target = $('#chunk');
+
+					expect(target).not.to.be.undefined;
+					expect(target.text()).to.equal('[object Object]');
+				} catch (e) {
+					expect(false).to.be.ok;
+				}
+			}
+		);
+	});
+});