fix: don't serialize undefined as null (#7531)

* fix: don't serialize `undefined` as `null` * test: include more types in the pass-js test
withastro · Jun 30, 2023 · 2172dd4 · 2172dd4
1 parent 4dd8849
commit 2172dd4
Show file tree

Hide file tree

Showing 6 changed files with 145 additions and 37 deletions.
diff --git a/.changeset/moody-singers-develop.md b/.changeset/moody-singers-develop.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix serialization of `undefined` in framework component props
diff --git a/packages/astro/e2e/fixtures/pass-js/src/components/React.tsx b/packages/astro/e2e/fixtures/pass-js/src/components/React.tsx
@@ -1,32 +1,45 @@
 import type { BigNestedObject } from '../types';
-import { useState } from 'react';
 
 interface Props {
-	obj: BigNestedObject;
-	num: bigint;
-	arr: any[];
+	undefined: undefined;
+	null: null;
+	boolean: boolean;
+	number: number;
+	string: string;
+	bigint: bigint;
+	object: BigNestedObject;
+	array: any[];
 	map: Map<string, string>;
 	set: Set<string>;
 }
 
 const isNode = typeof process !== 'undefined' && Object.prototype.toString.call(process) === '[object process]';
 
 /** a counter written in React */
-export default function Component({ obj, num, arr, map, set }: Props) {
+export default function Component({ undefined: undefinedProp, null: nullProp, boolean, number, string, bigint, object, array, map, set }: Props) {
 	// We are testing hydration, so don't return anything in the server.
 	if(isNode) {
 		return <div></div>
 	}
 
 	return (
 		<div>
-			<span id="nested-date">{obj.nested.date.toUTCString()}</span>
-			<span id="regexp-type">{Object.prototype.toString.call(obj.more.another.exp)}</span>
-			<span id="regexp-value">{obj.more.another.exp.source}</span>
-			<span id="bigint-type">{Object.prototype.toString.call(num)}</span>
-			<span id="bigint-value">{num.toString()}</span>
-			<span id="arr-type">{Object.prototype.toString.call(arr)}</span>
-			<span id="arr-value">{arr.join(',')}</span>
+			<span id="undefined-type">{Object.prototype.toString.call(undefinedProp)}</span>
+			<span id="null-type">{Object.prototype.toString.call(nullProp)}</span>
+			<span id="boolean-type">{Object.prototype.toString.call(boolean)}</span>
+			<span id="boolean-value">{boolean.toString()}</span>
+			<span id="number-type">{Object.prototype.toString.call(number)}</span>
+			<span id="number-value">{number.toString()}</span>
+			<span id="string-type">{Object.prototype.toString.call(string)}</span>
+			<span id="string-value">{string}</span>
+			<span id="bigint-type">{Object.prototype.toString.call(bigint)}</span>
+			<span id="bigint-value">{bigint.toString()}</span>
+			<span id="date-type">{Object.prototype.toString.call(object.nested.date)}</span>
+			<span id="date-value">{object.nested.date.toUTCString()}</span>
+			<span id="regexp-type">{Object.prototype.toString.call(object.more.another.exp)}</span>
+			<span id="regexp-value">{object.more.another.exp.source}</span>
+			<span id="array-type">{Object.prototype.toString.call(array)}</span>
+			<span id="array-value">{array.join(',')}</span>
 			<span id="map-type">{Object.prototype.toString.call(map)}</span>
 			<ul id="map-items">{Array.from(map).map(([key, value]) => (
 				<li>{key}: {value}</li>

diff --git a/packages/astro/e2e/fixtures/pass-js/src/pages/index.astro b/packages/astro/e2e/fixtures/pass-js/src/pages/index.astro
@@ -1,8 +1,8 @@
 ---
+import type { BigNestedObject } from '../types';
 import Component from '../components/React';
-import { BigNestedObject } from '../types';
 
-const obj: BigNestedObject = {
+const object: BigNestedObject = {
 	nested: {
 		date: new Date('Thu, 09 Jun 2022 14:18:27 GMT')
 	},
@@ -30,7 +30,19 @@ set.add('test2');
 	</head>
 	<body>
 		<main>
-			<Component client:load obj={obj} num={11n} arr={[0, "foo"]} map={map} set={set} />
+			<Component
+				client:load
+				undefined={undefined}
+				null={null}
+				boolean={true}
+				number={16}
+				string={"abc"}
+				bigint={11n}
+				object={object}
+				array={[0, "foo"]}
+				map={map}
+				set={set}
+			/>
 		</main>
 	</body>
 </html>
diff --git a/packages/astro/e2e/pass-js.test.js b/packages/astro/e2e/pass-js.test.js
@@ -16,49 +16,97 @@ test.afterAll(async () => {
 });
 
 test.describe('Passing JS into client components', () => {
-	test('Complex nested objects', async ({ astro, page }) => {
+	test('Primitive values', async ({ astro, page }) => {
 		await page.goto(astro.resolveUrl('/'));
 
-		const nestedDate = await page.locator('#nested-date');
-		await expect(nestedDate, 'component is visible').toBeVisible();
-		await expect(nestedDate).toHaveText('Thu, 09 Jun 2022 14:18:27 GMT');
-
-		const regeExpType = await page.locator('#regexp-type');
-		await expect(regeExpType, 'is visible').toBeVisible();
-		await expect(regeExpType).toHaveText('[object RegExp]');
-
-		const regExpValue = await page.locator('#regexp-value');
-		await expect(regExpValue, 'is visible').toBeVisible();
-		await expect(regExpValue).toHaveText('ok');
+		// undefined
+		const undefinedType = page.locator('#undefined-type');
+		await expect(undefinedType, 'is visible').toBeVisible();
+		await expect(undefinedType).toHaveText('[object Undefined]');
+
+		// null
+		const nullType = page.locator('#null-type');
+		await expect(nullType, 'is visible').toBeVisible();
+		await expect(nullType).toHaveText('[object Null]');
+
+		// boolean
+		const booleanType = page.locator('#boolean-type');
+		await expect(booleanType, 'is visible').toBeVisible();
+		await expect(booleanType).toHaveText('[object Boolean]');
+
+		const booleanValue = page.locator('#boolean-value');
+		await expect(booleanValue, 'is visible').toBeVisible();
+		await expect(booleanValue).toHaveText('true');
+
+		// number
+		const numberType = page.locator('#number-type');
+		await expect(numberType, 'is visible').toBeVisible();
+		await expect(numberType).toHaveText('[object Number]');
+
+		const numberValue = page.locator('#number-value');
+		await expect(numberValue, 'is visible').toBeVisible();
+		await expect(numberValue).toHaveText('16');
+
+		// string
+		const stringType = page.locator('#string-type');
+		await expect(stringType, 'is visible').toBeVisible();
+		await expect(stringType).toHaveText('[object String]');
+
+		const stringValue = page.locator('#string-value');
+		await expect(stringValue, 'is visible').toBeVisible();
+		await expect(stringValue).toHaveText('abc');
 	});
 
 	test('BigInts', async ({ astro, page }) => {
 		await page.goto(astro.resolveUrl('/'));
 
-		const bigIntType = await page.locator('#bigint-type');
+		const bigIntType = page.locator('#bigint-type');
 		await expect(bigIntType, 'is visible').toBeVisible();
 		await expect(bigIntType).toHaveText('[object BigInt]');
 
-		const bigIntValue = await page.locator('#bigint-value');
+		const bigIntValue = page.locator('#bigint-value');
 		await expect(bigIntValue, 'is visible').toBeVisible();
 		await expect(bigIntValue).toHaveText('11');
 	});
 
+	test('Complex nested objects', async ({ astro, page }) => {
+		await page.goto(astro.resolveUrl('/'));
+
+		// Date
+		const dateType = page.locator('#date-type');
+		await expect(dateType, 'is visible').toBeVisible();
+		await expect(dateType).toHaveText('[object Date]');
+
+		const dateValue = page.locator('#date-value');
+		await expect(dateValue, 'is visible').toBeVisible();
+		await expect(dateValue).toHaveText('Thu, 09 Jun 2022 14:18:27 GMT');
+
+		// RegExp
+		const regExpType = page.locator('#regexp-type');
+		await expect(regExpType, 'is visible').toBeVisible();
+		await expect(regExpType).toHaveText('[object RegExp]');
+
+		const regExpValue = page.locator('#regexp-value');
+		await expect(regExpValue, 'is visible').toBeVisible();
+		await expect(regExpValue).toHaveText('ok');
+	});
+
 	test('Arrays that look like the serialization format', async ({ astro, page }) => {
 		await page.goto(astro.resolveUrl('/'));
 
-		const arrType = await page.locator('#arr-type');
-		await expect(arrType, 'is visible').toBeVisible();
-		await expect(arrType).toHaveText('[object Array]');
+		const arrayType = page.locator('#array-type');
+		await expect(arrayType, 'is visible').toBeVisible();
+		await expect(arrayType).toHaveText('[object Array]');
 
-		const arrValue = await page.locator('#arr-value');
-		await expect(arrValue, 'is visible').toBeVisible();
-		await expect(arrValue).toHaveText('0,foo');
+		const arrayValue = page.locator('#array-value');
+		await expect(arrayValue, 'is visible').toBeVisible();
+		await expect(arrayValue).toHaveText('0,foo');
 	});
 
 	test('Maps and Sets', async ({ astro, page }) => {
 		await page.goto(astro.resolveUrl('/'));
 
+		// Map
 		const mapType = page.locator('#map-type');
 		await expect(mapType, 'is visible').toBeVisible();
 		await expect(mapType).toHaveText('[object Map]');
@@ -69,10 +117,13 @@ test.describe('Passing JS into client components', () => {
 		const texts = await mapValues.allTextContents();
 		expect(texts).toEqual(['test1: test2', 'test3: test4']);
 
+		// Set
 		const setType = page.locator('#set-type');
 		await expect(setType, 'is visible').toBeVisible();
+		await expect(setType).toHaveText('[object Set]');
 
 		const setValue = page.locator('#set-value');
+		await expect(setValue, 'is visible').toBeVisible();
 		await expect(setValue).toHaveText('test1,test2');
 	});
 });
diff --git a/packages/astro/src/runtime/server/serialize.ts b/packages/astro/src/runtime/server/serialize.ts
@@ -58,7 +58,7 @@ function convertToSerializedForm(
 	value: any,
 	metadata: AstroComponentMetadata | Record<string, any> = {},
 	parents = new WeakSet<any>()
-): [ValueOf<typeof PROP_TYPE>, any] {
+): [ValueOf<typeof PROP_TYPE>, any] | [ValueOf<typeof PROP_TYPE>] {
 	const tag = Object.prototype.toString.call(value);
 	switch (tag) {
 		case '[object Date]': {
@@ -100,6 +100,8 @@ function convertToSerializedForm(
 		default: {
 			if (value !== null && typeof value === 'object') {
 				return [PROP_TYPE.Value, serializeObject(value, metadata, parents)];
+			} else if (value === undefined) {
+				return [PROP_TYPE.Value];
 			} else {
 				return [PROP_TYPE.Value, value];
 			}

diff --git a/packages/astro/test/serialize.test.js b/packages/astro/test/serialize.test.js
@@ -2,11 +2,36 @@ import { expect } from 'chai';
 import { serializeProps } from '../dist/runtime/server/serialize.js';
 
 describe('serialize', () => {
-	it('serializes a plain value', () => {
+	it('serializes undefined', () => {
+		const input = { a: undefined };
+		const output = `{"a":[0]}`;
+		expect(serializeProps(input)).to.equal(output);
+	});
+	it('serializes null', () => {
+		const input = { a: null };
+		const output = `{"a":[0,null]}`;
+		expect(serializeProps(input)).to.equal(output);
+	});
+	it('serializes a boolean', () => {
+		const input = { a: false };
+		const output = `{"a":[0,false]}`;
+		expect(serializeProps(input)).to.equal(output);
+	});
+	it('serializes a number', () => {
 		const input = { a: 1 };
 		const output = `{"a":[0,1]}`;
 		expect(serializeProps(input)).to.equal(output);
 	});
+	it('serializes a string', () => {
+		const input = { a: 'b' };
+		const output = `{"a":[0,"b"]}`;
+		expect(serializeProps(input)).to.equal(output);
+	});
+	it('serializes an object', () => {
+		const input = { a: { b: 'c' } };
+		const output = `{"a":[0,{"b":[0,"c"]}]}`;
+		expect(serializeProps(input)).to.equal(output);
+	});
 	it('serializes an array', () => {
 		const input = { a: [0] };
 		const output = `{"a":[1,"[[0,0]]"]}`;